Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monitor listener not present after reconnect #52

Closed
smithandweb opened this issue May 29, 2015 · 4 comments
Closed

Monitor listener not present after reconnect #52

smithandweb opened this issue May 29, 2015 · 4 comments
Labels

Comments

@smithandweb
Copy link

I have my Redis master setup on a private DNS name with a TTL of 30. Average reconnection it obviously around that time once everything propagates.

One thing I've noticed is that any monitor that is running on the client is not present after a reconnect.

To see if I could recreate it on a reconnect I first called monitor.off('monitor'); and then attached the listener again once redis.status was ready.

It's not an important part of my server so I wasn't worried about it not working, but one thing that happened was this lovely error:

[2015-05-29 20:30:50.073] [ERROR] [Server]-> Caught exception: Error: Command queue state error. If you can reproduce this, please report it.
    at Redis.exports.returnReply (/home/compilr/node_modules/ioredis/lib/redis/prototype/parser.js:125:33)
    at HiredisReplyParser.<anonymous> (/home/compilr/node_modules/ioredis/lib/redis/prototype/parser.js:27:10)
    at HiredisReplyParser.emit (events.js:107:17)
    at HiredisReplyParser.execute (/home/compilr/node_modules/ioredis/lib/parsers/hiredis.js:42:12)
    at Socket.<anonymous> (/home/compilr/node_modules/ioredis/lib/redis/event_handler.js:86:22)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at Socket.Readable.push (_stream_readable.js:126:10)
    at TCP.onread (net.js:538:20)

So I'm reporting it just like you asked. Looks to be a hiredis issue and not yours perhaps.

If you need more info let me know!

@luin
Copy link
Collaborator

luin commented May 30, 2015

Thank you for reporting this. It's a bug that monitors don't work after reconnection and will be fixed in the next release. However I just did some tests but haven't found any way to reproduce the Command queue state error. Could you please show me the code causing the error? Here's mine:

var redis = new Redis(6999);

var listener = function (time, args) {
  console.log(time, args);
};

redis.monitor(function (err, monitor) {
  monitor.on('monitor', listener);
  redis.on('ready', function () {
    monitor.removeListener('monitor', listener);
    monitor.on('monitor', listener);
  });
});

@smithandweb
Copy link
Author

Thanks for the reply. I'll add code as soon as I can but I'll be away from this server for about a week on a work trip. I'll update you when I get back to it.

Thanks!

@luin luin added the bug label Jun 2, 2015
@luin luin closed this as completed in 109a6ec Jun 3, 2015
@smithandweb
Copy link
Author

Hey thanks for fixing this! I'll be sure to update.

For your curiosity this was the code that was giving me the Command queue error:

          //var monitored = false;

            _.client.on('error', function(err) {
                _.errorHandler(err);
            });

            _.client.on('reconnecting', function(delay){
                logger.debug('Retrying connection in: %s ms', delay);
            });

            // setInterval(function(){
            //     if (_.client.status !== 'ready' && monitored) {
            //         _.client.monitor(function(err, monitor) {
            //             monitor.off('monitor');
            //             monitored = false;
            //         });
            //     } else if (_.client.status === 'ready' && !monitored) {
            //         _.client.monitor(function (err, monitor) {
            //             monitor.on('monitor', function (time, args) {
            //                 if (args.indexOf('info') === -1) {
            //                     logger.log('Redis activity: Time: %s Args: %s', time, args[0]);
            //                 }
            //             });
            //         });

            //         monitored = true;
            //     }
            // },3000);

This was basically my fix for checking the ready state and if it was monitored. I tried both removeAllListeners and .off()

Thanks again for the quick fix!

@smithandweb
Copy link
Author

Just confirmed monitors are working as expected. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants