Updating versions of redis & async and issue with HAMulti erroring

[trickeyone]

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".

[carlos8f]

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?

[trickeyone]

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 npm test and I received the below result:

$ sudo npm test

> haredis@0.2.15 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)

[carlos8f]

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.

[trickeyone]

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.

[trickeyone]

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

> redis@0.10.0 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

> haredis@0.2.15 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)

[carlos8f]

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.

[trickeyone]

Sure. Committed to my fork.

[carlos8f]

Thanks, I massaged it a bit and pushed node_redis_0.10 branch. There are some test failures now in hmget and sub_unsub_sub that need to be worked out.