Updating versions of redis & async and issue with HAMulti erroring #22
Conversation
The versions of redis and async are really out of date. Updating.
HAMulti was referencing a non-existing property "client" instead of "_client" of the extended Redis Multi class
|
Looks promising, but to upgrade to node_redis 0.10 I need to be sure haredis passes the 0.10 tests. test.js from the node_redis project has traditionally been copied over into haredis, with some modifications to change createClient() to run in clustered mode. I started to upgrade to node_redis 0.8 a while ago: #10 , but it didn't go smoothly. If you can migrate the 0.10 tests over and they pass, I would gladly accept this PR :) Can you summarize what issues you had with the older version of node_redis? What version of redis -server do you run? |
|
Sure, no problem. I was noticing some performance and connectivity issues. I can't say for certain what they were, but when I forked and changed the version to use 0.10.0 the errors stopped. Redis server version 2.4.10 (00000000:0) I actually just ran $ sudo npm test
> [email protected] test /home/cw/htdocs/galactica/node_modules/haredis
> make test
failover
✓ create client (510ms)
✓ increment counter
✓ wait a bit for replication (1502ms)
✓ check counter on nodes
✓ kill master (3014ms)
✓ check counter
✓ client had failed over
✓ increment counter again
✓ restart old master
✓ old master is made into slave (2005ms)
✓ cluster in reasonable state
✓ wait a bit for replication (1502ms)
✓ get counter
✓ quit
node_redis tests
✓ original node_redis test (4677ms)
✓ node_redis test in cluster mode (6012ms)
quit
clustered
✓ create client
✓ kill master (2001ms)
✓ quit (627ms)
✓ ended (1003ms)
single
✓ create client
✓ quit
✓ ended (1003ms)
end
clustered
✓ create client
✓ kill master (2001ms)
✓ end
single
✓ create client
✓ quit
28 tests complete (26 seconds) |
|
Part of the haredis tests are borrowed from node_redis. What you ran was the node_redis 0.7 tests using node_redis 0.10 code, which doesn't indicate much :) To assure "drop-in" status, haredis must pass the node_redis 0.10 tests using node_redis 0.10. Both these Files must be updated from this file. Before that is done, I can't merge this. Also your redis-server version is very, very old (2.8.5 is the current version). I would recommend going with at least 2.6.x, and that could solve some of your performance/connectivity issues. I tried to make haredis pass tests on redis 2.4 but it's really aimed at 2.6 and higher. |
|
I would like to upgrade the version of redis, but we're limited by what's available in RHEL. I'll update the files and re-run the tests. |
|
Hi carlos8f. So, I've updated the tests, but there seem to be some issues with the actual node_redis tests. To be sure I ran the tests on node_redis itself and I'm getting errors on their own tests. So far, only 2 of the original tests don't run/pass. $ sudo npm test
> [email protected] test /home/cw/htdocs/haredis/node_modules/redis
> node ./test.js
Connected to 127.0.0.1:6379, Redis server version 2.4.10
Using reply parser javascript
- flushdb: 5 ms
- incr: 0 ms
- multi_1:client: AssertionError: MULTI_1 err is null, but an error is expected here.
at Array.2 (/home/cw/htdocs/haredis/node_modules/redis/test.js:83:16)
at /home/cw/htdocs/haredis/node_modules/redis/index.js:1128:42
at try_callback (/home/cw/htdocs/haredis/node_modules/redis/index.js:571:9)
at RedisClient.return_reply (/home/cw/htdocs/haredis/node_modules/redis/index.js:659:13)
at ReplyParser.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:309:14)
at ReplyParser.EventEmitter.emit (events.js:95:17)
at ReplyParser.send_reply (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:300:10)
at ReplyParser.execute (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:211:22)
at RedisClient.on_data (/home/cw/htdocs/haredis/node_modules/redis/index.js:532:27)
at Socket.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:91:14)
client: AssertionError: true == false
at process.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/test.js:2230:12)
at process.EventEmitter.emit (events.js:95:17)
at process.exit (node.js:707:17)
at RedisClient.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/test.js:2204:13)
at RedisClient.EventEmitter.emit (events.js:95:17)
at try_callback (/home/cw/htdocs/haredis/node_modules/redis/index.js:577:20)
at RedisClient.return_reply (/home/cw/htdocs/haredis/node_modules/redis/index.js:659:13)
at ReplyParser.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:309:14)
at ReplyParser.EventEmitter.emit (events.js:95:17)
at ReplyParser.send_reply (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:300:10)Result after updating the tests: $ sudo npm test
> [email protected] test /home/cw/htdocs/haredis
> make test
failover
✓ create client (516ms)
✓ increment counter
✓ wait a bit for replication (1500ms)
✓ check counter on nodes
✓ kill master (3002ms)
✓ check counter
✓ client had failed over
✓ increment counter again
✓ restart old master (39ms)
✓ old master is made into slave (2004ms)
✓ cluster in reasonable state
✓ wait a bit for replication (1502ms)
✓ get counter
✓ quit
node_redis tests
◦ original node_redis test: client: AssertionError: MULTI_8 err is null, but an error is expected here.
at Array.2 (/home/cw/htdocs/haredis/test-singlemode.js:82:16)
at /home/cw/htdocs/haredis/node_modules/redis/index.js:1128:42
at try_callback (/home/cw/htdocs/haredis/node_modules/redis/index.js:571:9)
at RedisClient.return_reply (/home/cw/htdocs/haredis/node_modules/redis/index.js:659:13)
at ReplyParser.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:309:14)
at ReplyParser.EventEmitter.emit (events.js:95:17)
at ReplyParser.send_reply (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:300:10)
at ReplyParser.execute (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:211:22)
at RedisClient.on_data (/home/cw/htdocs/haredis/node_modules/redis/index.js:532:27)
at Socket.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:91:14)
client: AssertionError: true == false
at process.<anonymous> (/home/cw/htdocs/haredis/test-singlemode.js:2230:12)
at process.EventEmitter.emit (events.js:95:17)
at process.exit (node.js:707:17)
at RedisHAClient.<anonymous> (/home/cw/htdocs/haredis/test-singlemode.js:2204:13)
at RedisHAClient.EventEmitter.emit (events.js:95:17)
at Node.<anonymous> (/home/cw/htdocs/haredis/index.js:525:14)
at Node.EventEmitter.emit (events.js:95:17)
at RedisClient.<anonymous> (/home/cw/htdocs/haredis/lib/node.js:72:10)
at RedisClient.EventEmitter.emit (events.js:95:17)
at try_callback (/home/cw/htdocs/haredis/node_modules/redis/index.js:577:20)
✓ original node_redis test (203ms)
◦ node_redis test in cluster mode: bclient: TypeError: Object GoC44pdt:1392071497716 has no method 'split'
at Array.node.client.MULTI.SETNX.GET.EXEC.was_error [as 2] (/home/cw/htdocs/haredis/index.js:716:25)
at /home/cw/htdocs/haredis/node_modules/redis/index.js:1128:42
at try_callback (/home/cw/htdocs/haredis/node_modules/redis/index.js:571:9)
at RedisClient.return_reply (/home/cw/htdocs/haredis/node_modules/redis/index.js:659:13)
at ReplyParser.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:309:14)
at ReplyParser.EventEmitter.emit (events.js:95:17)
at ReplyParser.send_reply (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:300:10)
at ReplyParser.execute (/home/cw/htdocs/haredis/node_modules/redis/lib/parser/javascript.js:211:22)
at RedisClient.on_data (/home/cw/htdocs/haredis/node_modules/redis/index.js:532:27)
at Socket.<anonymous> (/home/cw/htdocs/haredis/node_modules/redis/index.js:91:14)
bclient: AssertionError: true == false
at process.<anonymous> (/home/cw/htdocs/haredis/test.js:2232:12)
at process.EventEmitter.emit (events.js:95:17)
at process.exit (node.js:707:17)
at RedisHAClient.<anonymous> (/home/cw/htdocs/haredis/test.js:2219:13)
at RedisHAClient.EventEmitter.emit (events.js:95:17)
at Node.<anonymous> (/home/cw/htdocs/haredis/index.js:525:14)
at Node.EventEmitter.emit (events.js:95:17)
at RedisClient.<anonymous> (/home/cw/htdocs/haredis/lib/node.js:72:10)
at RedisClient.EventEmitter.emit (events.js:95:17)
at try_callback (/home/cw/htdocs/haredis/node_modules/redis/index.js:577:20)
✓ node_redis test in cluster mode (295ms)
quit
clustered
✓ create client
✓ kill master (2000ms)
✓ quit (607ms)
✓ ended (1003ms)
single
✓ create client
✓ quit
✓ ended (1000ms)
end
clustered
✓ create client
✓ kill master (2001ms)
✓ end
single
✓ create client
✓ quit
28 tests complete (16 seconds) |
|
My guess is the old redis-server version is causing failures (pretty sure the node_redis tests are written for 2.6 or later). You could try compiling redis 2.6 or 2.8 from source, or commit the tests to your fork and I could try running them. |
|
Sure. Committed to my fork. |
|
Thanks, I massaged it a bit and pushed node_redis_0.10 branch. There are some test failures now in |
I ran into a few issues using the older version of redis that haredis was previously using. As well as a lot of errors from the HAMulti class due to attempting to reference a non-exist property "client" of the extended Redis Multi class, it needed to reference "_client".