support timeout option

[secmask]

Does vertx-redis-client support timeout? I need redis command to be finish in some milliseconds, how can I do it?

[pmlopes]

Just use a vertx timer for that, that would be the easiest.

[secmask]

So does the redis handler get call after that?, can I use redis-client for other command in timer handler context?

[pmlopes]

here some code:

      Handler<AsyncResult<String>> handler = res -> {
        // here your success handler as usual...
      };

      final long timerId = vertx.setTimer(30000l, t -> handler.handle(Future.failedFuture("timeout")));

      redis.get(key, res -> {
        if (vertx.cancelTimer(timerId)) {
          handler.handle(res);
        }
      });

So you declare the handler outside the redis command, and wrap it with a timer, in this case i set call failure if no reply arrives in 30 seconds. If a reply arrives before it cancels the timer and delegates to your handler, if a timer has already been triggered and a reply comes later the cancel timer returns false since the timer does not exist anymore and your handler is not called twice.

Probably you could wrap this in some helper method and make the code cleaner.

[secmask]

hello, I'm back, I still need to clear the state of RedisClient after the timeout fire.
for example, I have follow sequence:

browser request - > RedisClient -> send request -> redis_server busy for a moment -> client fire timedout handler ->
next browser request -> RedisClient -> send request -> redis_server response for the first request

Actually, we have a RPC service that use redis protocol as redis server side, sometime it take time for this server to response, I just want to ignore any slow response and continue other.
So, is there any risk that the second request will get the reply correspond to the first request?

[pmlopes]

for each request you specify a handler (callback if you prefer) this handler is specific to the command so ignoring it means that the handler if triggered but you then have some logic to ignore any further processing. This means that no handler will receive replies from other requests it is 1 to 1 mapping.

[secmask]

Yes, I see, but when the timeout fired, how RedisClient handle the late incoming reply (discard it? ), does RedisClient clear the receive buffer before send a command? should I close the RedisClient instance and create a new one for safe?
Thanks for your explain.

[pmlopes]

hi, there is no receive buffer, what happens is like this:

your code sends a command and passes a handler, the redis client has a FIFO where the handlers are added, once a reply arrives from the redis server the top of the FIFO is removed and called with the incoming message. Now the timer has to do the following:

if handler has not been called, then mark it to do nothing once it is called by the client, else do nothing.

Please see the code on the comment: #26 (comment)

for each command you send you create a timer, then the handler is:

cancel the timer, if you can cancel it means it has been called yet (no timeout yet) therefore call the real handler.

If a reply comes after the cancel was triggered the if statement will be false so the lower handler will not be triggered.

[secmask]

ok, I was forget that the first reply was ignore by this block

if (vertx.cancelTimer(timerId)) {
         handler.handle(res);
}

Thank you.

[ltamrazov]

Sorry, but I don't understand why this issue was closed. Has this been resolved? I think the vertx timer is simply a workaround. Its strange that a storage library like Redis would not natively support a timeout option...

[pmlopes]

@ltamrazov Redis does not support timeout see their docs: https://redis.io/commands

You can add it to your code as stated in these comments but that would not be in any way related to redis and it can introduce strange behaviour. For example if a DEL command takes too long you would trigger a timeout and assume it failed but since redis does not have the concept of timeout with enough time and if the server does not crash the key will be deleted.

So that was the reason this was closed as won't fix.

[ltamrazov]

Yes, agreed. Thanks for the clarification.