BRPOP... 0 (and similar) now timeout after connection timeout rather than block

[shuckc]

I have a c program that does a blocking pop from a redis list to retrieve work. In order to prevent long connection timeouts, I set a connection timeout.

redisContext *rc = NULL;
struct timeval redis_timeout = { 1, 500000 }; // 1.5 seconds
redisReply *reply;
... 
rc = redisConnectWithTimeout(optarg, 6379, redis_timeout);
if (rc == NULL || rc->err) {
  if (rc) {
    printf("Redis connection error: %s\n", rc->errstr);
    redisFree(rc);
  } else {
    printf("Redis connection error: can't allocate redis context\n");
  }
  exit(1);
}
...
reply = redisCommand(rc, "BRPOP tokens 0");
if (rc->err) {
  printf("Connection error on wakeup: %s\n", rc->errstr);
  exit(1);
}
freeReplyObject(reply);

Using hiredis master, the Connection error on wakeup fires after 1.5s.
If I wanted an operation timeout, i would use a non-zero final argument to BRPOP

If I build this against v14, it blocks until the server has an item pushed into the list.

hiredis:
    git clone https://github.com/redis/hiredis.git hiredis
    cd hiredis && git -c advice.detachedHead=false checkout v0.14.0

From a quick review of master I think this is a regression with the new SSL support, although I will need to bisect it to be sure.

[shuckc]

Reproducing code:

$ cat Makefile

LIBS=-lm
CFLAGS=-ggdb -O3 -Wall -I./hiredis
OBJS=blocker.o

blocker: blocker.o hiredis/libhiredis.a
	$(CC) $^ -o $@ $(CFLAGS) $(LIBS)

clean:
	rm -f $(OBJS)
	rm -Rf hiredis

blocker.o: hiredis blocker.c

hiredis:
	git clone https://github.com/redis/hiredis.git hiredis
	cd hiredis && git -c advice.detachedHead=false checkout master
	make -C hiredis

$ cat blocker.c

#include <stdio.h>
#include <stdlib.h>
#include <hiredis.h>

int main(int argc, char **argv) {
  redisContext *rc = NULL;
  struct timeval redis_timeout = { 1, 500000 }; // 1.5 seconds
  redisReply *reply;

  rc = redisConnectWithTimeout("localhost", 6379, redis_timeout);
  if (rc == NULL || rc->err) {
    if (rc) {
      printf("Redis connection error: %s\n", rc->errstr);
      redisFree(rc);
    } else {
      printf("Redis connection error: can't allocate redis context\n");
    }
    exit(1);
  }

  reply = redisCommand(rc, "BRPOP tokens 0");
  if (rc->err) {
    printf("Connection error on wakeup: %s\n", rc->errstr);
    exit(1);
  }
  freeReplyObject(reply);
  printf("finished\n");

(in another shell...) $ docker run -p 6379:6379 redis

$ make blocker
...
$ ./blocker 
Connection error on wakeup: Resource temporarily unavailable

You can see the BLPOP has not blocked. If you now edit Makefile to pull v0.14.0, run make clean, make blocker,

$ ./blocker

...

Which hangs indefinitely - the expected behaviour

[michael-grunder]

Deleted previous message because it wasn't that commit.

I think the relevant commit is 4830786.

You can get what you want by using redisSetTimeout after you connect though.

#include <stdio.h>
#include <stdlib.h>
#include "hiredis/hiredis.h"

int main(int argc, char **argv) {
    redisContext *rc = NULL;
    struct timeval redis_timeout = { 1, 500000 }; // 1.5 seconds
    redisReply *reply;

    rc = redisConnectWithTimeout("localhost", 6379, redis_timeout);
    if (rc == NULL || rc->err) {
        if (rc) {
            printf("Redis connection error: %s\n", rc->errstr);
            redisFree(rc);
        } else {
            printf("Redis connection error: can't allocate redis context\n");
        }
        exit(1);
    }

    /* Use an unlimited read timeout if we've been passed an argument */
    if (argc > 1) {
        struct timeval tm = {.tv_sec = 0, .tv_usec = 0};
        redisSetTimeout(rc, tm);
    }

    reply = redisCommand(rc, "BRPOP tokens 0");
    if (rc->err) {
        printf("Connection error on wakeup: %s\n", rc->errstr);
        exit(1);
    }
    freeReplyObject(reply);
    printf("finished\n");
}

[shuckc]

I agree on the bisect. Thanks for a workaround.

$ git bisect start
$ git bisect good v0.14.0
$ git bisect bad
...
$ ./blocker
Connection error on wakeup: Resource temporarily unavailable
....
$ git bisect bad
4830786c84b96d2a2e95b49fba55a1548d9971e1 is the first bad commit
commit 4830786c84b96d2a2e95b49fba55a1548d9971e1
Author: Mark Nunberg <[email protected]>
Date:   Wed Apr 10 08:28:36 2019 -0400

    ensure that blocking timeout is set

 hiredis.c | 4 ++++
 1 file changed, 4 insertions(+)

[michael-grunder]

No worries!

@mnunberg It looks like 4830786 was an intentional change in behavior, but I just want to double check.

[shuckc]

I think it is fair to say this is a substantial breaking change to your public API, and the various higher language API wrappers would need to address this.

[michael-grunder]

Fortunately many of the hiredis wrappers vendor the library so those would be unaffected.

Agree it's a breaking change, but we're able to do that if this and other changes are released as 1.0.

[michael-grunder]

I'm going to close this issue as we're about to release v1.0.0 and can break things 😄