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

Node redis giving Error: Ready check failed: Redis connection gone from close event #725

Closed
PrakashJetty opened this issue Mar 15, 2015 · 12 comments
Labels

Comments

@PrakashJetty
Copy link

Hi,

The client started giving the error "Error: Ready check failed: Redis connection gone from close event"
, the same code is running from last june perfectly fine. Two days back due to networking issue redis server went down , then onward we got these messages.Then after sometime we got this message
"[Error: Ready check failed: ERR max number of clients reached]" continuously, when redis boxes were brought back.

We are using one connection per request and closing the connection.

Our view for the second messages continuously coming is that client tries to connect infinite times until get connected ( this I inferred from the documentation), It is strange to me. this is causing the pile up of connection requests overloading redis.

Reason why i am stating this issue is that , there is something wrong i believe the way connection issues are handles , they cannot simply try for infinite no of times effecting cpu.

Also i wanted help in finding out resolving the exact issue here.

@PrakashJetty
Copy link
Author

Hey,

Somebody , please respond.

Thanks,
Prakash

@brycebaril
Copy link
Contributor

Hi Prakash -- in terms of the cause of this error, it usually appears to be network related. See #247 and #601 for other people running into this.

We're still hoping someone finds a reliable reproduction case to help us troubleshoot how to handle this error when it does come up.

As for your conjecture about the client loading up Redis, I don't think that would be the case, as these clients shouldn't be staying connected if they fail.

What is your Redis configuration, especially with respect to the number of connections allowed?

What version of Redis server?

What version of node_redis?

One thing you can do when this happens is look at the output of the CLIENT LIST command to debug where your clients are from if you hit the max clients error.

@PrakashJetty
Copy link
Author

redis_version:2.8.18
Node_redis : 0.10.3

"As for your conjecture about the client loading up Redis, I don't think that would be the case, as these clients shouldn't be staying connected if they fail."

They are not stayed get connected , the issue is that once redis server comes back , with in few seconds we saw max connections reached.

Like i said we are using one connection per request. So these requests when redis goes down infinitely retrying and piling up connection requests, when redis server comes back they are all casuing reached maximum allowed connections.

This is a serious issue for us and serious for any scenario.

@PrakashJetty
Copy link
Author

Here are the files we were using to simulate the error

var redisLoad = function(styleId,cb) {
   var recoResponse = {};
               var redisService = require('./lib/recoservice/redis')();
    redisService.onConnect(function() {
    redisService.getStringValue("somekey" + styleId,
        function(err, results) {
            if (err) {
                console.error("Error while retrieving styles:" , err);
                console.trace(err);
                recoResponse.similar = [];
                redisService.endConnection();
                cb && cb(null, recoResponse);
                return;
            } else {
                console.log("connected to redis");
                var recos;
                // parse recos
                try {
                    if (results) {
                        recos = JSON.parse(results);
                    }
                } catch (err) {
                    console.error("Error parsing recos" + err);
                }
                if (recos && recos.length) {
                            recoResponse.similar = recos.length > 15 ? recos.slice(0, 15) : recos;
                            redisService.endConnection();
                            cb && cb(null, recoResponse);
                } else {
                    // handle no recos case as explained above
                    redisService.endConnection();
                    recoResponse.similar = [];
                    cb && cb(null, recoResponse);
                }
            }
        });
});
};
        
var i=0;
while(i<2000) {
    redisLoad(101617,function(err,results){
        console.log(results,err);
    });
    ++i;
}

@PrakashJetty
Copy link
Author

var i=0;
while(i<2000) {
    redisLoad(101617,function(err,results){
        console.log(results,err);
    });
    ++i;
}

@PrakashJetty
Copy link
Author

"use strict";
var redis = require('redis'),
    config = require('../recoconfig')['redis_connection_object'];
//redis.debug_mode=true;
/*
 * Wrapper object for redis core services
 * Defines functions 
 * persistStringValue(), endConnection() & retrieveStringValue()
 *
 */
var Redis = function () { 
    if (!(this instanceof Redis)) return new Redis();
    var client = this.client = redis.createClient("6379","localhost",{
            "connection_timeout": 1.0
        });
    client.on('error',function(err){
            if (err) {
                console.error('error connecting redis for recommendations: ', err);
                //client.quit();
                return;
            }
        });
    client.on('reconnecting',function(){
        console.log('reconnecting');
    })
};
Redis.prototype.endConnection = function () {
    this.client.quit();
};
Redis.prototype.persistStringValue = function(key,value,ex,cb) {
   this.client.set(key,value,"EX",ex,cb);
};
Redis.prototype.retrieveStringValue = function(key,cb) {
    var redisClient = this.client;
    (function(){
        var callbackCall = false;
        var timer = setTimeout(function(){
            callbackCall = true;
            cb(new Error('Redis is not responding'),null);
        },2000);
        redisClient.get(key,function(err,res){
            clearTimeout(timer);
            if(callbackCall){
                return;
            }
            cb(err,res);
        });
    })();
};
Redis.prototype.getStringValue = function(key,cb) {
   this.client.get(key,cb);
};
Redis.prototype.onConnect = function(cb) {
    this.client.on('connect',cb);
}
module.exports = Redis;

@PrakashJetty
Copy link
Author

By running the first file and retarting redis server in between .

Observed :

The redis does not connect at all after restarting , it contniuosly throws errors.

@brycebaril
Copy link
Contributor

@PrakashJetty thank you for the test scripts, I'll try it out

@PrakashJetty
Copy link
Author

@brycebaril , Did u get a chance to look into the scripts sahred

@carlosascari
Copy link

I found this error while creating a mocha test for a project i'm working on.

Heres the rundown:

50,000 commands async executed at different times (between 0ms-500ms)
The commands are arrays, with the last element being the callback, and are fed into redisClient.multi([command]).exec

To complete execution, it would take about 11 seconds, before completion, I would close the redis-server. And restart it all as well

Out of the 50K, 1 or 2 of those commands would return the Redis connection gone from error event. error, particularly, it seemed to affect the commands executed the closest to the time where I would close the redis-server

From this, I'm assuming the error is rare because it only occurs when the server takes a command from the client, is about to either execute it or return at which point the server, even for a tiny amount of time, disconnects or closes.

The window of time for this particular event to happen is tiny, so you would only see it in high volumes over large periods of time.

A Solution:

I queue commands on my own, upon executing these commands, i take the callback and wrap it inside a 'safety' function, this function tests for an error with the message Redis connection gone from error event., if found, it re-sends the command, otherwise the original callback is executed.

This is what it looks like:

// Get command, from queue 
var command = queue[0]

// Get the command's callback 
var command_callback = command[command.length-1]

// This is the safety function to secure all commands are executed
function safetyCallbackWrapper(err){
    if (err && err.message === 'Redis connection gone from error event.') 
    {
        // Darn it! execute this command again
        return queue.push(command)
    }
    command_callback.apply(this, arguments) 
}; 

// The callback is now inside our safetyCallbackWrapper
command[command.length-1] = safetyCallbackWrapper

// Execute
redisClient.multi([command]).exec ...

Just to re-iterate: The error occurs at a midpoint, where the client has successfully sent a command, at which point the redis-server disconnects or closes. This particular error is rare so the safetyWrapperCallback is used to test for it specifically, if found the command is re-executed, otherwise, the original callback is executed.

@BridgeAR
Copy link
Contributor

@PrakashJetty @carlosascari could you check if the error persists in v.2? That would be really nice :)

@BridgeAR
Copy link
Contributor

I tried your test code and it reconnected fine with v.2.0. If this is not fixed by v.2 feel free to reopen the issue.

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

4 participants