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

[Question]how to set redis get command timeout #789

Closed
cekimy opened this issue Jul 25, 2015 · 16 comments
Closed

[Question]how to set redis get command timeout #789

cekimy opened this issue Jul 25, 2015 · 16 comments

Comments

@cekimy
Copy link

cekimy commented Jul 25, 2015

I've meet a problem several times. redis.get cost much time, more than 1s (even more than 1 minute)
This is a terrible online disaster, so i want to know how to set timeout of get command.
Or what could cause redis cost so much time?

@stockholmux
Copy link
Contributor

Could you tell me more about your setup and the code that is causing this delay? Is the server remote or local? Is this your first call to Redis in your script after a restart?

This is very unusual - Redis usually takes ~1ms to do a simple operation (like get). I would be surprised if this is actually a node_redis problem and more likely something to do with your environment and how Node is communicating (or not) with your Redis server.

@cekimy
Copy link
Author

cekimy commented Jul 27, 2015

server is remote, but in the same IDC. Most of the time, it only takes ~1ms, but this >1s situation came about 1time/4-5months, and continue several seconds.

i have 4 redis servers, each key will hash to one of them, and then redis.get(This caused delay).
Our service has about 500,000,000+ requests one day, and each redis server has 10000+ operation per second on the rush hour.
The server is usually stable running, this problem appears suddenly, and disappears a few seconds later.

@stockholmux
Copy link
Contributor

It's hard to say, but I would venture it is actually not a node_redis issue from what you're telling me - I can't think of a thing in the project code that would cause a stable service to take that long. I'm guessing that it has to do with something busy on the server (redis or node).

As far as a timeout, I don't think there is anything in the project to set a specific timeout for a single command. It could be done in your app - perhaps with a closure and a setTimeout - run the setTimeout and the get at the same time, then use a variable to see which one has completed first. It's definitely a hack, but it could work.

@stockholmux
Copy link
Contributor

I got around to writing a gist on how to do a timeout for a redis command:

https://gist.github.com/stockholmux/3a4b2d1480f27df8be67#file-timelimitedredis-js

@cekimy Can we close this issue since it's seems more like a redis server stall issue and there is a non-library way of addressing the issue?

@cekimy
Copy link
Author

cekimy commented Aug 17, 2015

@stockholmux ok, thanks very much, I'll try this gist.

@cekimy cekimy closed this as completed Aug 17, 2015
@cekimy
Copy link
Author

cekimy commented Nov 4, 2015

i still have this problem, when the network between node server and redis server has problem, redis.get will cost hundreds seconds.

see our monitor: mostly it takes avg/1ms

fdac73cd-caa2-4979-a89f-06f9ccb3f51d

this is my simple code:

redis.js

var redis = require("redis"), h = null;
var client = redis.createClient(6379, '10.173.2.22', {enable_offline_queue: false, retry_max_delay: 100});

addListener(client, 'error', null);
addListener(client, 'connect', client);

function addListener(handler, evt, value) {
    handler.on(evt, function() {
        h = value;
    });
}

var puppet = {
    'get': function(key, callback) {
        if (h === null) {
            //reporting error
            callback("redis error", null);
        } else {
            h.get(key, callback);
        }
    },
    //other commands
};

module.exports = puppet;

app.js

var http = require('http');
var redis = require("redis.js");

http.createServer(function (req, res) {
    //...
    var t1 = Date.now();
    redis.get('key', function(err, reply) {
        var t2 = Date.now();
        monitor("time_redis", t2 - t1);     //this is reporting to above pic's monitor,calculate the average value
        //...
    });
    //...
}).listen(1337, "10.23.5.42");

@cekimy cekimy reopened this Nov 4, 2015
@BridgeAR
Copy link
Contributor

BridgeAR commented Nov 8, 2015

@cekimy what node_redis version and what parser do you use?

@cekimy
Copy link
Author

cekimy commented Nov 9, 2015

redis@0.12.1
hiredis@0.4.0
and use default parser:hiredis

@BridgeAR
Copy link
Contributor

Hm, this is weird. Since you do not use the offline queue. I can't think of a reason why this could happen. Are you certain that you only use .get? Or what commands do you use?

@cekimy
Copy link
Author

cekimy commented Nov 13, 2015

i use .get, .set, .incr, .hgetall...and so on, but i only monitor the cost of every .get command.

@BridgeAR
Copy link
Contributor

Hm, you said there were connection issues while the huge delay occured? I can't explain such a delay right now and wonder what happend on your side in this case.

And did you try the code from @stockholmux? By having a glimpse at it, it looks good to me for solveing your issue.

@cekimy
Copy link
Author

cekimy commented Nov 17, 2015

ok,thanks

@cekimy cekimy closed this as completed Nov 17, 2015
@jbergknoff
Copy link
Contributor

Having a command timeout would be a useful feature. I am experiencing an issue where the redis client hangs, after sending a command, for about 15 minutes before the socket emits an ETIMEDOUT error. The cause of this is most likely an OS or network issue, but I'd rather detect it within a few seconds rather than let my application hang for 15 minutes.

I am currently monkey-patching the behavior a la @stockholmux's solution. Ideally I would modify just RedisClient::send_command, but the input to that method varies wildly depending on circumstances, so it's untenable to pick out and change the callback. Alternately, the connection timeout could be implemented inside send_command. @BridgeAR what are your thoughts on that? Would you be open to a PR for this?

@shivoodles
Copy link

Hi,

Can you provide the code sample for lrange+ltrim in multi+exec.

Thanks in advance.

@kidkissed
Copy link

i change the file redis/index.js line 352
var queue_name = options.queues || ['command_queue','offline_queue']
to
var queue_name = ['command_queue','offline_queue']
so that i can get an error callback when the server is down
but i don't know wether this change will make negative affects to the other functions or not

@ali-essam
Copy link

ali-essam commented Jul 12, 2018

For those stumbling upon this thread, I ended up using promise-timeout along with the promisified api.

Example:

const { timeout: promiseTimeout } = require('promise-timeout')
const { promisifyAll } = require('bluebird')

const redis = require('redis')
promisifyAll(redis.RedisClient.prototype)
promisifyAll(redis.Multi.prototype)

const client = redis.createClient({
  url: process.env.REDIS_URL
})


const foo = async () => {
  const val = await promiseTimeout(client.getAsync(key), REDIS_TIMEOUT)
}

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

No branches or pull requests

7 participants