From 38b5ae543f5c99eb4ccabbe277770fc6bc81226f Mon Sep 17 00:00:00 2001
From: valentinogeron <valentino@redislabs.com>
Date: Sun, 26 Jul 2020 22:32:27 +0300
Subject: add a command_timeout to redisContextOptions (#839)
Add an additional timeout so the user has a convenient way of controlling distinct connect and command timeouts
---
async.c | 12 ++++++------
async_private.h | 17 ++++++++---------
examples/example-libevent.c | 2 +-
examples/example-ssl.c | 2 +-
hiredis.c | 38 +++++++++++++++++++++++++++++---------
hiredis.h | 9 ++++++---
net.c | 42 +++++++++++++++++++++++++++++-------------
7 files changed, 80 insertions(+), 42 deletions(-)
diff --git a/async.c b/async.c
index 1073d8d..020cb09 100644
--- a/async.c
+++ b/async.c
@@ -868,19 +868,19 @@ redisAsyncPushFn *redisAsyncSetPushCallback(redisAsyncContext *ac, redisAsyncPus
}
int redisAsyncSetTimeout(redisAsyncContext *ac, struct timeval tv) {
- if (!ac->c.timeout) {
- ac->c.timeout = hi_calloc(1, sizeof(tv));
- if (ac->c.timeout == NULL) {
+ if (!ac->c.command_timeout) {
+ ac->c.command_timeout = hi_calloc(1, sizeof(tv));
+ if (ac->c.command_timeout == NULL) {
__redisSetError(&ac->c, REDIS_ERR_OOM, "Out of memory");
__redisAsyncCopyError(ac);
return REDIS_ERR;
}
}
- if (tv.tv_sec != ac->c.timeout->tv_sec ||
- tv.tv_usec != ac->c.timeout->tv_usec)
+ if (tv.tv_sec != ac->c.command_timeout->tv_sec ||
+ tv.tv_usec != ac->c.command_timeout->tv_usec)
{
- *ac->c.timeout = tv;
+ *ac->c.command_timeout = tv;
}
return REDIS_OK;
diff --git a/async_private.h b/async_private.h
index d0133ae..e4a22ab 100644
--- a/async_private.h
+++ b/async_private.h
@@ -54,15 +54,14 @@
} while(0);
static inline void refreshTimeout(redisAsyncContext *ctx) {
- if (ctx->c.timeout && ctx->ev.scheduleTimer &&
- (ctx->c.timeout->tv_sec || ctx->c.timeout->tv_usec)) {
- ctx->ev.scheduleTimer(ctx->ev.data, *ctx->c.timeout);
- // } else {
- // printf("Not scheduling timer.. (tmo=%p)\n", ctx->c.timeout);
- // if (ctx->c.timeout){
- // printf("tv_sec: %u. tv_usec: %u\n", ctx->c.timeout->tv_sec,
- // ctx->c.timeout->tv_usec);
- // }
+ if (!(ctx->c.flags & REDIS_CONNECTED)) {
+ if (ctx->c.connect_timeout && ctx->ev.scheduleTimer &&
+ (ctx->c.connect_timeout->tv_sec || ctx->c.connect_timeout->tv_usec)) {
+ ctx->ev.scheduleTimer(ctx->ev.data, *ctx->c.connect_timeout);
+ }
+ } else if (ctx->c.command_timeout && ctx->ev.scheduleTimer &&
+ (ctx->c.command_timeout->tv_sec || ctx->c.command_timeout->tv_usec)) {
+ ctx->ev.scheduleTimer(ctx->ev.data, *ctx->c.command_timeout);
}
}
diff --git a/examples/example-libevent.c b/examples/example-libevent.c
index f3fa2a6..49bddd0 100644
--- a/examples/example-libevent.c
+++ b/examples/example-libevent.c
@@ -47,7 +47,7 @@ int main (int argc, char **argv) {
REDIS_OPTIONS_SET_TCP(&options, "127.0.0.1", 6379);
struct timeval tv = {0};
tv.tv_sec = 1;
- options.timeout = &tv;
+ options.connect_timeout = &tv;
redisAsyncContext *c = redisAsyncConnectWithOptions(&options);
diff --git a/examples/example-ssl.c b/examples/example-ssl.c
index 5eb2bbb..c754177 100644
--- a/examples/example-ssl.c
+++ b/examples/example-ssl.c
@@ -33,7 +33,7 @@ int main(int argc, char **argv) {
struct timeval tv = { 1, 500000 }; // 1.5 seconds
redisOptions options = {0};
REDIS_OPTIONS_SET_TCP(&options, hostname, port);
- options.timeout = &tv;
+ options.connect_timeout = &tv;
c = redisConnectWithOptions(&options);
if (c == NULL || c->err) {
diff --git a/hiredis.c b/hiredis.c
index e9761ec..8d72d35 100644
--- a/hiredis.c
+++ b/hiredis.c
@@ -44,6 +44,9 @@
#include "async.h"
#include "win32.h"
+extern int redisContextUpdateConnectTimeout(redisContext *c, const struct timeval *timeout);
+extern int redisContextUpdateCommandTimeout(redisContext *c, const struct timeval *timeout);
+
static redisContextFuncs redisContextDefaultFuncs = {
.free_privdata = NULL,
.async_read = redisAsyncRead,
@@ -723,7 +726,8 @@ void redisFree(redisContext *c) {
hi_free(c->tcp.host);
hi_free(c->tcp.source_addr);
hi_free(c->unix_sock.path);
- hi_free(c->timeout);
+ hi_free(c->connect_timeout);
+ hi_free(c->command_timeout);
hi_free(c->saddr);
if (c->funcs->free_privdata) {
c->funcs->free_privdata(c->privdata);
@@ -761,18 +765,24 @@ int redisReconnect(redisContext *c) {
return REDIS_ERR;
}
+ int ret = REDIS_ERR;
if (c->connection_type == REDIS_CONN_TCP) {
- return redisContextConnectBindTcp(c, c->tcp.host, c->tcp.port,
- c->timeout, c->tcp.source_addr);
+ ret = redisContextConnectBindTcp(c, c->tcp.host, c->tcp.port,
+ c->connect_timeout, c->tcp.source_addr);
} else if (c->connection_type == REDIS_CONN_UNIX) {
- return redisContextConnectUnix(c, c->unix_sock.path, c->timeout);
+ ret = redisContextConnectUnix(c, c->unix_sock.path, c->connect_timeout);
} else {
/* Something bad happened here and shouldn't have. There isn't
enough information in the context to reconnect. */
__redisSetError(c,REDIS_ERR_OTHER,"Not enough information to reconnect");
+ ret = REDIS_ERR;
}
- return REDIS_ERR;
+ if (c->command_timeout != NULL && (c->flags & REDIS_BLOCK) && c->fd != REDIS_INVALID_FD) {
+ redisContextSetTimeout(c, *c->command_timeout);
+ }
+
+ return ret;
}
redisContext *redisConnectWithOptions(const redisOptions *options) {
@@ -790,13 +800,19 @@ redisContext *redisConnectWithOptions(const redisOptions *options) {
c->flags |= REDIS_NO_AUTO_FREE;
}
+ if (redisContextUpdateConnectTimeout(c, options->connect_timeout) != REDIS_OK ||
+ redisContextUpdateCommandTimeout(c, options->command_timeout) != REDIS_OK) {
+ __redisSetError(c, REDIS_ERR_OOM, "Out of memory");
+ return c;
+ }
+
if (options->type == REDIS_CONN_TCP) {
redisContextConnectBindTcp(c, options->endpoint.tcp.ip,
- options->endpoint.tcp.port, options->timeout,
+ options->endpoint.tcp.port, options->connect_timeout,
options->endpoint.tcp.source_addr);
} else if (options->type == REDIS_CONN_UNIX) {
redisContextConnectUnix(c, options->endpoint.unix_socket,
- options->timeout);
+ options->connect_timeout);
} else if (options->type == REDIS_CONN_USERFD) {
c->fd = options->endpoint.fd;
c->flags |= REDIS_CONNECTED;
@@ -805,6 +821,10 @@ redisContext *redisConnectWithOptions(const redisOptions *options) {
return NULL;
}
+ if (options->command_timeout != NULL && (c->flags & REDIS_BLOCK) && c->fd != REDIS_INVALID_FD) {
+ redisContextSetTimeout(c, *options->command_timeout);
+ }
+
return c;
}
@@ -820,7 +840,7 @@ redisContext *redisConnect(const char *ip, int port) {
redisContext *redisConnectWithTimeout(const char *ip, int port, const struct timeval tv) {
redisOptions options = {0};
REDIS_OPTIONS_SET_TCP(&options, ip, port);
- options.timeout = &tv;
+ options.connect_timeout = &tv;
return redisConnectWithOptions(&options);
}
@@ -858,7 +878,7 @@ redisContext *redisConnectUnix(const char *path) {
redisContext *redisConnectUnixWithTimeout(const char *path, const struct timeval tv) {
redisOptions options = {0};
REDIS_OPTIONS_SET_UNIX(&options, path);
- options.timeout = &tv;
+ options.connect_timeout = &tv;
return redisConnectWithOptions(&options);
}
diff --git a/hiredis.h b/hiredis.h
index 20e9bfa..fe2dae4 100644
--- a/hiredis.h
+++ b/hiredis.h
@@ -176,8 +176,10 @@ typedef struct {
int type;
/* bit field of REDIS_OPT_xxx */
int options;
- /* timeout value. if NULL, no timeout is used */
- const struct timeval *timeout;
+ /* timeout value for connect operation. if NULL, no timeout is used */
+ const struct timeval *connect_timeout;
+ /* timeout value for commands. if NULL, no timeout is used. (can be set later on with redisSetTimeout/redisAsyncSetTimeout) */
+ const struct timeval *command_timeout;
union {
/** use this field for tcp/ip connections */
struct {
@@ -230,7 +232,8 @@ typedef struct redisContext {
redisReader *reader; /* Protocol reader */
enum redisConnectionType connection_type;
- struct timeval *timeout;
+ struct timeval *connect_timeout;
+ struct timeval *command_timeout;
struct {
char *host;
diff --git a/net.c b/net.c
index 882e2c4..c6b0e5d 100644
--- a/net.c
+++ b/net.c
@@ -217,7 +217,7 @@ int redisSetTcpNoDelay(redisContext *c) {
static int redisContextTimeoutMsec(redisContext *c, long *result)
{
- const struct timeval *timeout = c->timeout;
+ const struct timeval *timeout = c->connect_timeout;
long msec = -1;
/* Only use timeout when not NULL. */
@@ -328,19 +328,35 @@ int redisContextSetTimeout(redisContext *c, const struct timeval tv) {
return REDIS_OK;
}
-static int _redisContextUpdateTimeout(redisContext *c, const struct timeval *timeout) {
+int redisContextUpdateConnectTimeout(redisContext *c, const struct timeval *timeout) {
/* Same timeval struct, short circuit */
- if (c->timeout == timeout)
+ if (c->connect_timeout == timeout)
return REDIS_OK;
/* Allocate context timeval if we need to */
- if (c->timeout == NULL) {
- c->timeout = hi_malloc(sizeof(*c->timeout));
- if (c->timeout == NULL)
+ if (c->connect_timeout == NULL) {
+ c->connect_timeout = hi_malloc(sizeof(*c->connect_timeout));
+ if (c->connect_timeout == NULL)
return REDIS_ERR;
}
- memcpy(c->timeout, timeout, sizeof(*c->timeout));
+ memcpy(c->connect_timeout, timeout, sizeof(*c->connect_timeout));
+ return REDIS_OK;
+}
+
+int redisContextUpdateCommandTimeout(redisContext *c, const struct timeval *timeout) {
+ /* Same timeval struct, short circuit */
+ if (c->command_timeout == timeout)
+ return REDIS_OK;
+
+ /* Allocate context timeval if we need to */
+ if (c->command_timeout == NULL) {
+ c->command_timeout = hi_malloc(sizeof(*c->command_timeout));
+ if (c->command_timeout == NULL)
+ return REDIS_ERR;
+ }
+
+ memcpy(c->command_timeout, timeout, sizeof(*c->command_timeout));
return REDIS_OK;
}
@@ -376,11 +392,11 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port,
}
if (timeout) {
- if (_redisContextUpdateTimeout(c, timeout) == REDIS_ERR)
+ if (redisContextUpdateConnectTimeout(c, timeout) == REDIS_ERR)
goto oom;
} else {
- hi_free(c->timeout);
- c->timeout = NULL;
+ hi_free(c->connect_timeout);
+ c->connect_timeout = NULL;
}
if (redisContextTimeoutMsec(c, &timeout_msec) != REDIS_OK) {
@@ -549,11 +565,11 @@ int redisContextConnectUnix(redisContext *c, const char *path, const struct time
}
if (timeout) {
- if (_redisContextUpdateTimeout(c, timeout) == REDIS_ERR)
+ if (redisContextUpdateConnectTimeout(c, timeout) == REDIS_ERR)
goto oom;
} else {
- hi_free(c->timeout);
- c->timeout = NULL;
+ hi_free(c->connect_timeout);
+ c->connect_timeout = NULL;
}
if (redisContextTimeoutMsec(c,&timeout_msec) != REDIS_OK)
--
cgit v1.2.3