search

carlos8f / haredis

High-availability redis in Node.js.
https://npmjs.org/package/haredis
154 stars 21 forks source link

Upgrade to node_redis 0.8.x #10

Open carlos8f opened 11 years ago

carlos8f commented 11 years ago

Initiative started in #9 by @shamil, but after copying over test.js from node_redis, there were failures, so I started a new branch for it (https://github.com/carlos8f/haredis/tree/node_redis_0.8) and reverted master.

I also had failure running the node_redis 0.7.x test-singlemode.js under redis-server 2.6.12 (i was testing previously on 2.5.x) related to EXECABORT, but that might just be an issue with the older test code.

shamil commented 11 years ago

You seem to be running different tests, as mine passes all the tests once I fixed the EXECABORT issue.

carlos8f commented 11 years ago

Yes - if we upgrade node_redis to 0.8, we have to upgrade the tests too (which were copied from 0.7). I forgot to commit the upgraded test I ran -- but it is now in the node_redis_0.8 branch.

How did you fix the EXECABORT issue?

shamil commented 11 years ago

EXECABORT fired when one of the command in MULTI series is failed. so I just commented out the bad set (something like: multi1.set("foo2", require_error(name));)

carlos8f commented 11 years ago

ahh, I pushed the fix to both branches: 5850d3fd795e90ee9fb43a87d3a8aa0a967a2d15

now to work out the new failures... https://gist.github.com/carlos8f/5544218

nickminutello commented 11 years ago

I'm looking forward to the upgrade - I'm experiencing a bug in redis 0.7.3 : https://github.com/mranney/node_redis/issues/467

lcodes commented 10 years ago

I've been trying to get these tests to pass on my machine. We'd really like to use redis-0.8.x with haredis as we're hitting the same bug @nickminutello mentionned on every automatic maintenance of our azure servers. I used redis 0.9.1 for the tests (they had the same initial results you did with 0.8.x)

Here is what I got, for the test.js file:

I was able to fix the TypeError: Cannot read property 'name' of undefined in the incr test by changing the reference to reply_parser to be the following:

bclient.nodes[0].client.reply_parser.name

The multi_1 and multi_2 tests were both failing until I removed the redis version checks (I'm running 2.6.16)

There's also an error in the script_load test I fixed by replacing the call to .script by a call to .send_command, however I find it odd that the script function was missing.

Then the reconnect_select_db_after_pubsub test I fixed by setting a higher timeout for reconnection (it was working correctly but always timing out before having a chance to reconnect)

The subscribe tests were all failing until I changed the 3 replies to the 3 messages to be 0.

I had to bypass the enable_offline_queue_false and enable_offline_queue_true tests (I'm not running a redis on 9999 and it threw an error instead of skipping to the next test, cli.offline_queue is undefined).

I stopped at the slowlog test, the config function is not on the RedisHAClient object (just as was the case with the script function in the script_load test.

Also, the test refuses to boot if the redis instances aren't already properly configured with one master. The bclient in the test causes conflicts in the haredis code which doesn't handle Buffer replies.

I get the impression that these bugs are only present in the test script, but not sure enough to go with this branch on our live deployments yet.