Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove inconsistent delays in kb routines. #230

Merged
merged 3 commits into from
May 13, 2019
+49 −57
Merged

Remove inconsistent delays in kb routines. #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa543fd
Remove active wait for DB slot in get_redis_ctx().
kroosec May 9, 2019
c90b198
Return distinct errors in get_redis_ctx() and kb_new().
kroosec May 9, 2019
5b03e53
Remove sleep on error in redis_find() and redis_flush_all().
kroosec May 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 48 additions & 56 deletions util/kb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include <stdio.h>
#include <stdlib.h> /* for atoi */
#include <string.h> /* for strlen, strerror, strncpy, memset */
#include <unistd.h> /* for sleep */

#undef G_LOG_DOMAIN
#define G_LOG_DOMAIN "lib kb"
Expand All @@ -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;

/**
Expand Down Expand Up @@ -257,44 +250,38 @@ 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;

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 -1;
}

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 -2;
}
while (rc != 0);

g_debug ("%s: connected to redis://%s/%d", __func__, kbr->path, kbr->db);
return kbr->rctx;
return 0;
}

/**
Expand Down Expand Up @@ -380,7 +367,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)
Expand All @@ -392,17 +379,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;
}

Expand Down Expand Up @@ -495,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
Expand All @@ -510,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);
Expand Down Expand Up @@ -649,22 +637,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);
Expand Down Expand Up @@ -935,7 +918,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);
Expand All @@ -947,16 +929,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);
Expand Down Expand Up @@ -1091,7 +1074,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.)
Expand Down Expand Up @@ -1171,7 +1156,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)
Expand Down Expand Up @@ -1207,7 +1194,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);
Expand Down Expand Up @@ -1260,11 +1249,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);
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion util/kb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down