From aa543fd984b481823db76a0c38d28cd8bfd2cea8 Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Thu, 9 May 2019 17:11:51 +0100 Subject: [PATCH 1/3] Remove active wait for DB slot in get_redis_ctx(). --- util/kb.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/util/kb.c b/util/kb.c index 08b578572..f0c1928ff 100644 --- a/util/kb.c +++ b/util/kb.c @@ -267,31 +267,25 @@ get_redis_ctx (struct kb_redis *kbr) if (kbr->rctx != NULL) return kbr->rctx; - do + kbr->rctx = redisConnectUnix (kbr->path); + if (kbr->rctx == NULL || kbr->rctx->err) { - kbr->rctx = redisConnectUnix (kbr->path); - if (kbr->rctx == NULL || kbr->rctx->err) - { - g_log (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, - "%s: redis connection error: %s", __func__, - kbr->rctx ? kbr->rctx->errstr : strerror (ENOMEM)); - redisFree (kbr->rctx); - kbr->rctx = NULL; - return NULL; - } + g_log (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, + "%s: redis connection error: %s", __func__, + kbr->rctx ? kbr->rctx->errstr : strerror (ENOMEM)); + redisFree (kbr->rctx); + kbr->rctx = NULL; + return NULL; + } - rc = select_database (kbr); - if (rc) - { - g_log (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, - "%s: No redis DB available, retrying in %ds...", __func__, - KB_RETRY_DELAY); - sleep (KB_RETRY_DELAY); - redisFree (kbr->rctx); - kbr->rctx = NULL; - } + rc = select_database (kbr); + if (rc) + { + g_log (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "No redis DB available"); + redisFree (kbr->rctx); + kbr->rctx = NULL; + return NULL; } - while (rc != 0); g_debug ("%s: connected to redis://%s/%d", __func__, kbr->path, kbr->db); return kbr->rctx; From c90b1981316808dd13baa2affa396253af63abd8 Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Thu, 9 May 2019 17:13:51 +0100 Subject: [PATCH 2/3] Return distinct errors in get_redis_ctx() and kb_new(). -1 on connection error, -2 when no DB slot is available. --- util/kb.c | 58 ++++++++++++++++++++++++++++++------------------------- util/kb.h | 2 +- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/util/kb.c b/util/kb.c index f0c1928ff..89d396810 100644 --- a/util/kb.c +++ b/util/kb.c @@ -257,15 +257,15 @@ redis_release_db (struct kb_redis *kbr) * a connection. * @param[in] kbr Subclass of struct kb where to fetch the context. * or where it is saved in case of a new connection. - * @return Redis context on success, NULL otherwise. + * @return 0 on success, -1 on connection error, -2 on unavailable DB slot. */ -static redisContext * +static int get_redis_ctx (struct kb_redis *kbr) { int rc; if (kbr->rctx != NULL) - return kbr->rctx; + return 0; kbr->rctx = redisConnectUnix (kbr->path); if (kbr->rctx == NULL || kbr->rctx->err) @@ -275,7 +275,7 @@ get_redis_ctx (struct kb_redis *kbr) kbr->rctx ? kbr->rctx->errstr : strerror (ENOMEM)); redisFree (kbr->rctx); kbr->rctx = NULL; - return NULL; + return -1; } rc = select_database (kbr); @@ -284,11 +284,11 @@ get_redis_ctx (struct kb_redis *kbr) g_log (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "No redis DB available"); redisFree (kbr->rctx); kbr->rctx = NULL; - return NULL; + return -2; } g_debug ("%s: connected to redis://%s/%d", __func__, kbr->path, kbr->db); - return kbr->rctx; + return 0; } /** @@ -374,7 +374,7 @@ redis_get_kb_index (kb_t kb) * @brief Initialize a new Knowledge Base object. * @param[in] kb Reference to a kb_t to initialize. * @param[in] kb_path Path to KB. - * @return 0 on success, non-null on error. + * @return 0 on success, -1 on connection error, -2 when no DB is available. */ static int redis_new (kb_t *kb, const char *kb_path) @@ -386,17 +386,18 @@ redis_new (kb_t *kb, const char *kb_path) kbr->kb.kb_ops = &KBRedisOperations; strncpy (kbr->path, kb_path, strlen (kb_path)); - rc = redis_test_connection (kbr); - if (rc) + if ((rc = get_redis_ctx (kbr)) < 0) + return rc; + if (redis_test_connection (kbr)) { g_log (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "%s: cannot access redis at '%s'", __func__, kb_path); redis_delete ((kb_t) kbr); kbr = NULL; + rc = -1; } *kb = (kb_t) kbr; - return rc; } @@ -643,22 +644,17 @@ redis_cmd (struct kb_redis *kbr, const char *fmt, ...) va_start (ap, fmt); do { - redisContext *ctx; - - rep = NULL; - - ctx = get_redis_ctx (kbr); - if (ctx == NULL) + if (get_redis_ctx (kbr) < 0) { va_end (ap); return NULL; } va_copy (aq, ap); - rep = redisvCommand (ctx, fmt, aq); + rep = redisvCommand (kbr->rctx, fmt, aq); va_end (aq); - if (ctx->err) + if (kbr->rctx->err) { if (rep != NULL) freeReplyObject (rep); @@ -929,7 +925,6 @@ redis_get_pattern (kb_t kb, const char *pattern) struct kb_item *kbi = NULL; redisReply *rep; unsigned int i; - redisContext *ctx; kbr = redis_kb (kb); rep = redis_cmd (kbr, "KEYS %s", pattern); @@ -941,16 +936,17 @@ redis_get_pattern (kb_t kb, const char *pattern) return NULL; } - ctx = get_redis_ctx (kbr); + if (get_redis_ctx (kbr) < 0) + return NULL; for (i = 0; i < rep->elements; i++) - redisAppendCommand (ctx, "LRANGE %s 0 -1", rep->element[i]->str); + redisAppendCommand (kbr->rctx, "LRANGE %s 0 -1", rep->element[i]->str); for (i = 0; i < rep->elements; i++) { struct kb_item *tmp; redisReply *rep_range; - redisGetReply (ctx, (void **) &rep_range); + redisGetReply (kbr->rctx, (void **) &rep_range); if (!rep) continue; tmp = redis2kbitem (rep->element[i]->str, rep_range); @@ -1085,7 +1081,9 @@ redis_add_str_unique (kb_t kb, const char *name, const char *str, size_t len) redisContext *ctx; kbr = redis_kb (kb); - ctx = get_redis_ctx (kbr); + if (get_redis_ctx (kbr) < 0) + return -1; + ctx = kbr->rctx; /* Some VTs still rely on values being unique (ie. a value inserted multiple * times, will only be present once.) @@ -1165,7 +1163,9 @@ redis_set_str (kb_t kb, const char *name, const char *val, size_t len) int rc = 0, i = 4; kbr = redis_kb (kb); - ctx = get_redis_ctx (kbr); + if (get_redis_ctx (kbr) < 0) + return -1; + ctx = kbr->rctx; redisAppendCommand (ctx, "MULTI"); redisAppendCommand (ctx, "DEL %s", name); if (len == 0) @@ -1201,7 +1201,9 @@ redis_add_int_unique (kb_t kb, const char *name, int val) redisContext *ctx; kbr = redis_kb (kb); - ctx = get_redis_ctx (kbr); + if (get_redis_ctx (kbr) < 0) + return -1; + ctx = kbr->rctx; redisAppendCommand (ctx, "LREM %s 1 %d", name, val); redisAppendCommand (ctx, "RPUSH %s %d", name, val); redisGetReply (ctx, (void **) &rep); @@ -1254,11 +1256,15 @@ redis_add_int (kb_t kb, const char *name, int val) static int redis_set_int (kb_t kb, const char *name, int val) { + struct kb_redis *kbr; redisReply *rep = NULL; redisContext *ctx; int rc = 0, i = 4; - ctx = get_redis_ctx (redis_kb (kb)); + kbr = redis_kb (kb); + if (get_redis_ctx (redis_kb (kb)) < 0) + return -1; + ctx = kbr->rctx; redisAppendCommand (ctx, "MULTI"); redisAppendCommand (ctx, "DEL %s", name); redisAppendCommand (ctx, "RPUSH %s %d", name, val); diff --git a/util/kb.h b/util/kb.h index 0545db5b9..23a412d4a 100644 --- a/util/kb.h +++ b/util/kb.h @@ -235,7 +235,7 @@ kb_item_free (struct kb_item *); * @brief Initialize a new Knowledge Base object. * @param[in] kb Reference to a kb_t to initialize. * @param[in] kb_path Path to KB. - * @return 0 on success, non-null on error. + * @return 0 on success, -1 on connection error, -2 on unavailable DB spot. */ static inline int kb_new (kb_t *kb, const char *kb_path) From 5b03e53d945bd6863af1080d09a5fdcfe847f6e8 Mon Sep 17 00:00:00 2001 From: Hani Benhabiles Date: Thu, 9 May 2019 17:53:49 +0100 Subject: [PATCH 3/3] Remove sleep on error in redis_find() and redis_flush_all(). --- util/kb.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/util/kb.c b/util/kb.c index 89d396810..2a55afd0f 100644 --- a/util/kb.c +++ b/util/kb.c @@ -33,7 +33,6 @@ #include #include /* for atoi */ #include /* for strlen, strerror, strncpy, memset */ -#include /* for sleep */ #undef G_LOG_DOMAIN #define G_LOG_DOMAIN "lib kb" @@ -50,12 +49,6 @@ */ #define GLOBAL_DBINDEX_NAME "GVM.__GlobalDBIndex" -/** - * @brief Number of seconds to wait for between two attempts to acquire a KB - * namespace. - */ -#define KB_RETRY_DELAY 60 - static const struct kb_operations KBRedisOperations; /** @@ -490,7 +483,7 @@ redis_find (const char *kb_path, const char *key) rep = redisCommand (kbr->rctx, "SELECT %u", i); if (rep == NULL || rep->type != REDIS_REPLY_STATUS) { - sleep (KB_RETRY_DELAY); + redisFree (kbr->rctx); kbr->rctx = NULL; } else @@ -505,8 +498,8 @@ redis_find (const char *kb_path, const char *key) return (kb_t) kbr; } } + redisFree (kbr->rctx); } - redisFree (kbr->rctx); i++; } while (i < kbr->max_db); @@ -1406,7 +1399,6 @@ redis_flush_all (kb_t kb, const char *except) if (rep == NULL || rep->type != REDIS_REPLY_STATUS) { freeReplyObject (rep); - sleep (KB_RETRY_DELAY); redisFree (kbr->rctx); kbr->rctx = NULL; }