diff --git a/include/redisx-priv.h b/include/redisx-priv.h index 3bedfc9..e2bc4f3 100644 --- a/include/redisx-priv.h +++ b/include/redisx-priv.h @@ -57,6 +57,7 @@ typedef struct { RedisServer *servers; ///< List of sentinel servers. int nServers; ///< number of servers in list char *serviceName; ///< Name of service (for Sentinel). + int timeoutMillis; ///< [ms] Connection timeout for sentinel nodes. } RedisSentinel; diff --git a/include/redisx.h b/include/redisx.h index d9c8680..b02a1c5 100644 --- a/include/redisx.h +++ b/include/redisx.h @@ -51,6 +51,11 @@ # define REDISX_DEFAULT_TIMEOUT_MILLIS 3000 #endif +#ifndef REDISX_DEFAULT_SENTINEL_TIMEOUT_MILLIS +/// [ms] Default socket read/write timeout for Redis clients +# define REDISX_DEFAULT_SENTINEL_TIMEOUT_MILLIS 100 +#endif + // Various exposed constants -----------------------------------------------------> /// API major version @@ -343,6 +348,8 @@ boolean redisxIsVerbose(); void redisxDebugTraffic(boolean value); void redisxSetTcpBuf(int size); +int redisxSetSocketTimeout(Redis *redis, int millis); +int redisxSetSentinelTimeout(Redis *redis, int millis); int redisxSetTransmitErrorHandler(Redis *redis, RedisErrorHandler f); int redisxSetPort(Redis *redis, int port); diff --git a/src/redisx-net.c b/src/redisx-net.c index 0b479ef..ab3e1e9 100644 --- a/src/redisx-net.c +++ b/src/redisx-net.c @@ -241,10 +241,11 @@ static int rTryConnectSentinel(Redis *redis, int serverIndex) { static int rDiscoverSentinel(Redis *redis) { static const char *fn = "rConnectSentinel"; - const RedisPrivate *p = (RedisPrivate *) redis->priv; + RedisPrivate *p = (RedisPrivate *) redis->priv; const RedisSentinel *s = p->sentinel; - int i; + int i, savedTimeout = p->timeoutMillis; + p->timeoutMillis = s->timeoutMillis > 0 ? s->timeoutMillis : REDISX_DEFAULT_SENTINEL_TIMEOUT_MILLIS; for(i = 0; i < s->nServers; i++) if(rTryConnectSentinel(redis, i) == X_SUCCESS) { RESP *reply; @@ -261,13 +262,16 @@ static int rDiscoverSentinel(Redis *redis) { int port = (int) strtol((char *) component[1]->value, NULL, 10); status = rSetServerAsync(redis, "sentinel master", (char *) component[0]->value, port); + redisxDestroyRESP(reply); + p->timeoutMillis = savedTimeout; prop_error(fn, status); return X_SUCCESS; } } + p->timeoutMillis = savedTimeout; return x_error(X_NO_SERVICE, ENOTCONN, fn, "no Sentinel server available"); } @@ -872,15 +876,17 @@ Redis *redisxInit(const char *server) { } /** - * Initializes a Redis client with a Sentinel configuration of alternate servers + * Initializes a Redis client with a Sentinel configuration of alternate servers, and the default + * sentinel node connection timeout. * - * @param serviceName The service name as registered in the Sentinel server configuration. - * @param serverList An set of Sentinel servers to use to dynamically find the current master. A - * copy of the supplied name will be used, so the argument passed can be - * freely destroyed after the call. - * @param nServers The number of servers in the list - * @return X_SUCCESS (0) if successful, or else an error code <0. + * @param serviceName The service name as registered in the Sentinel server configuration. + * @param serverList An set of Sentinel servers to use to dynamically find the current master. A + * copy of the supplied name will be used, so the argument passed can be + * freely destroyed after the call. + * @param nServers The number of servers in the list + * @return X_SUCCESS (0) if successful, or else an error code <0. * + * @sa redisxSetSentinelTimeout() * @sa redisxInit() * @sa redisxConnect() */ @@ -926,12 +932,40 @@ Redis *redisxInitSentinel(const char *serviceName, const RedisServer *serverList s->nServers = nServers; s->serviceName = xStringCopyOf(serviceName); - + s->timeoutMillis = REDISX_DEFAULT_SENTINEL_TIMEOUT_MILLIS; p->sentinel = s; return redis; } +/** + * Changes the connection timeout for Sentinel server instances in the discovery phase. This is different + * from the timeout that is used for the master server, once it is discovered. + * + * @param redis The Redis instance, which was initialized for Sentinel via redisxInitSentinel(). + * @param millis [ms] The new connection timeout or <=0 to use the default value. + * @return X_SUCCESS (0) if successfully set sentinel connection timeout, or else X_NULL if the + * redis instance is NULL, or X_NO_INIT if the redis instance is not initialized for + * Sentinel. + * + * @sa redisxSetSocketTimeout() + * @sa redisxInitSentinel() + */ +int redisxSetSentinelTimeout(Redis *redis, int millis) { + static const char *fn = "redisxSetSentinelTimeout"; + + RedisPrivate *p; + int status = X_SUCCESS; + + prop_error(fn, rConfigLock(redis)); + p = (RedisPrivate *) redis->priv; + if(p->sentinel) p->sentinel->timeoutMillis = millis > 0 ? millis : REDISX_DEFAULT_SENTINEL_TIMEOUT_MILLIS; + else status = x_error(X_NO_INIT, EAGAIN, fn, "Redis was not initialized for Sentinel"); + rConfigUnlock(redis); + + return status; +} + static void rDestroySentinel(RedisSentinel *sentinel) { if(!sentinel) return; @@ -1029,17 +1063,17 @@ int redisxSetPort(Redis *redis, int port) { * or a negative value), then the timeout will not be configured for sockets, and the system default * timeout values will apply. * - * @param redis The Redis instance - * @param timeoutMillis [ms] The desired socket read/write timeout, or <0 for socket default. - * @return X_SUCCESS (0) if successful, or else X_NULL if the redis instance is NULL, - * or X_N_INIT if the redis instance is not initialized. + * @param redis The Redis instance + * @param millis [ms] The desired socket read/write timeout, or <0 for socket default. + * @return X_SUCCESS (0) if successful, or else X_NULL if the redis instance is NULL, + * or X_NO_INIT if the redis instance is not initialized. */ -int redisxSetSocketTimeout(Redis *redis, int timeoutMillis) { +int redisxSetSocketTimeout(Redis *redis, int millis) { RedisPrivate *p; prop_error("redisxSetPort", rConfigLock(redis)); p = (RedisPrivate *) redis->priv; - p->timeoutMillis = timeoutMillis; + p->timeoutMillis = millis; rConfigUnlock(redis); return X_SUCCESS; diff --git a/src/redisx-script.c b/src/redisx-script.c index a1b90a3..6dd25dd 100644 --- a/src/redisx-script.c +++ b/src/redisx-script.c @@ -41,11 +41,7 @@ int redisxLoadScript(Redis *redis, const char *script, char **sha1) { *sha1 = NULL; reply = redisxRequest(redis, "SCRIPT", "LOAD", script, NULL, &status); - - if(!status) { - redisxDestroyRESP(reply); - return x_trace(fn, NULL, status); - } + if(status) return x_trace(fn, NULL, status); prop_error(fn, redisxCheckDestroyRESP(reply, RESP_BULK_STRING, 0)); diff --git a/src/redisx-tab.c b/src/redisx-tab.c index 31eeaf4..759319f 100644 --- a/src/redisx-tab.c +++ b/src/redisx-tab.c @@ -62,11 +62,7 @@ RedisEntry *redisxGetTable(Redis *redis, const char *table, int *n) { } reply = redisxRequest(redis, "HGETALL", table, NULL, NULL, n); - - if(*n) { - redisxDestroyRESP(reply); - return x_trace_null(fn, NULL); - } + if(*n) return x_trace_null(fn, NULL); // Cast RESP2 array respone to RESP3 map also... if(reply && reply->type == RESP_ARRAY) { @@ -415,11 +411,7 @@ char **redisxGetKeys(Redis *redis, const char *table, int *n) { } reply = redisxRequest(redis, table ? "HKEYS" : "KEYS", table ? table : "*", NULL, NULL, n); - - if(*n) { - redisxDestroyRESP(reply); - return x_trace_null(fn, NULL); - } + if(*n) return x_trace_null(fn, NULL); *n = redisxCheckDestroyRESP(reply, RESP_ARRAY, 0); if(*n) return x_trace_null(fn, NULL); diff --git a/src/redisx.c b/src/redisx.c index 8705e33..9bd9b77 100644 --- a/src/redisx.c +++ b/src/redisx.c @@ -282,10 +282,7 @@ int redisxGetTime(Redis *redis, struct timespec *t) { memset(t, 0, sizeof(*t)); reply = redisxRequest(redis, "TIME", NULL, NULL, NULL, &status); - if(status) { - redisxDestroyRESP(reply); - return x_trace(fn, NULL, status); - } + if(status) return x_trace(fn, NULL, status); status = redisxCheckDestroyRESP(reply, RESP_ARRAY, 2); prop_error(fn, status); @@ -338,18 +335,15 @@ int redisxPing(Redis *redis, const char *message) { RESP *reply; reply = redisxRequest(redis, "PING", message, NULL, NULL, &status); + prop_error(fn, status); - if(!status) { - if(!reply) status = x_error(X_NULL, errno, fn, "reply was NULL"); - else { - status = redisxCheckRESP(reply, message ? RESP_BULK_STRING : RESP_SIMPLE_STRING, 0); - if(!status) if(strcmp(message ? message : "PONG", (char *)reply->value) != 0) - status = x_error(REDIS_UNEXPECTED_RESP, ENOMSG, fn, "expected 'PONG', got '%s'", (char *)reply->value); - } - } + if(!reply) return x_error(X_NULL, errno, fn, "reply was NULL"); + + status = redisxCheckRESP(reply, message ? RESP_BULK_STRING : RESP_SIMPLE_STRING, 0); + if(!status) if(strcmp(message ? message : "PONG", (char *)reply->value) != 0) + status = x_error(REDIS_UNEXPECTED_RESP, ENOMSG, fn, "expected 'PONG', got '%s'", (char *)reply->value); redisxDestroyRESP(reply); - prop_error(fn, status); return X_SUCCESS; } @@ -554,7 +548,7 @@ int redisxSetPipelineConsumer(Redis *redis, RedisPipelineProcessor f) { * the error code set by redisxArrayRequest(). * * \return A freshly allocated RESP array containing the Redis response, or NULL if no valid - * response could be obtained. + * response could be obtained or status is not X_SUCCESS. * * @sa redisxArrayRequest() * @sa redisxSendRequestAsync() @@ -572,7 +566,6 @@ RESP *redisxRequest(Redis *redis, const char *command, const char *arg1, const c else n = 4; reply = redisxArrayRequest(redis, (char **) args, NULL, n, &s); - if(status) *status = s; if(s) {