From 65f99530f454f1dce95ddc0e9dd04a43758274bc Mon Sep 17 00:00:00 2001 From: resec Date: Thu, 7 Nov 2019 11:57:53 +0800 Subject: [PATCH 1/5] [Fix] #3129 Multiple classifiers on redis working incorrectly --- src/libstat/stat_process.c | 61 ++++++++++++++------------------------ 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/src/libstat/stat_process.c b/src/libstat/stat_process.c index 486a9069f..2ad7080d0 100644 --- a/src/libstat/stat_process.c +++ b/src/libstat/stat_process.c @@ -261,38 +261,6 @@ rspamd_stat_backends_process (struct rspamd_stat_ctx *st_ctx, } } -static gboolean -rspamd_stat_backends_post_process (struct rspamd_stat_ctx *st_ctx, - struct rspamd_task *task) -{ - guint i; - struct rspamd_statfile *st; - struct rspamd_classifier *cl; - gpointer bk_run; - gboolean ret = TRUE; - - g_assert (task->stat_runtimes != NULL); - - for (i = 0; i < st_ctx->statfiles->len; i++) { - st = g_ptr_array_index (st_ctx->statfiles, i); - cl = st->classifier; - - if (cl->cfg->flags & RSPAMD_FLAG_CLASSIFIER_NO_BACKEND) { - continue; - } - - bk_run = g_ptr_array_index (task->stat_runtimes, i); - - if (bk_run != NULL) { - if (!st->backend->finalize_process (task, bk_run, st_ctx)) { - ret = FALSE; - } - } - } - - return ret; -} - static void rspamd_stat_classifiers_process (struct rspamd_stat_ctx *st_ctx, struct rspamd_task *task) @@ -327,6 +295,8 @@ rspamd_stat_classifiers_process (struct rspamd_stat_ctx *st_ctx, cl->ham_learns = 0; } + g_assert (task->stat_runtimes != NULL); + for (i = 0; i < st_ctx->statfiles->len; i++) { st = g_ptr_array_index (st_ctx->statfiles, i); cl = st->classifier; @@ -357,10 +327,28 @@ rspamd_stat_classifiers_process (struct rspamd_stat_ctx *st_ctx, g_assert (cl != NULL); - /* Ensure that all symbols enabled */ skip = FALSE; - if (!(cl->cfg->flags & RSPAMD_FLAG_CLASSIFIER_NO_BACKEND)) { + /* Do not process classifiers on backend failures */ + for (j = 0; j < cl->statfiles_ids->len; j++) { + if (cl->cfg->flags & RSPAMD_FLAG_CLASSIFIER_NO_BACKEND) { + continue; + } + + id = g_array_index (cl->statfiles_ids, gint, j); + bk_run = g_ptr_array_index (task->stat_runtimes, id); + st = g_ptr_array_index (st_ctx->statfiles, id); + + if (bk_run != NULL) { + if (!st->backend->finalize_process (task, bk_run, st_ctx)) { + skip = TRUE; + break + } + } + } + + /* Ensure that all symbols enabled */ + if (!skip && !(cl->cfg->flags & RSPAMD_FLAG_CLASSIFIER_NO_BACKEND)) { for (j = 0; j < cl->statfiles_ids->len; j++) { id = g_array_index (cl->statfiles_ids, gint, j); bk_run = g_ptr_array_index (task->stat_runtimes, id); @@ -425,10 +413,7 @@ rspamd_stat_classify (struct rspamd_task *task, lua_State *L, guint stage, } else if (stage == RSPAMD_TASK_STAGE_CLASSIFIERS_POST) { /* Process classifiers */ - if (rspamd_stat_backends_post_process (st_ctx, task)) { - rspamd_stat_classifiers_process (st_ctx, task); - } - /* Do not process classifiers on backend failures */ + rspamd_stat_classifiers_process (st_ctx, task); } task->processed_stages |= stage; From 606c653671216b6575e4966e4c84a2f8da2fe43f Mon Sep 17 00:00:00 2001 From: resec Date: Thu, 7 Nov 2019 18:56:09 +0800 Subject: [PATCH 2/5] [Fix] #3129 Multiple classifiers on redis working incorrectly --- src/libstat/backends/redis_backend.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libstat/backends/redis_backend.c b/src/libstat/backends/redis_backend.c index 90afe66d3..7d79272c0 100644 --- a/src/libstat/backends/redis_backend.c +++ b/src/libstat/backends/redis_backend.c @@ -1325,13 +1325,10 @@ rspamd_redis_connected (redisAsyncContext *c, gpointer r, gpointer priv) } } else { - if (!rt->err) { - g_set_error (&rt->err, rspamd_redis_stat_quark (), c->err, - "skip obtaining bayes tokens for %s: " - "not enough learns %d; %d required", - rt->stcf->symbol, (int)rt->learned, - rt->stcf->clcf->min_learns); - } + msg_warn_task ("skip obtaining bayes tokens for %s of classifier " + "%s: not enough learns %d; %d required", + rt->stcf->symbol, rt->stcf->clcf->name, + (int)rt->learned, rt->stcf->clcf->min_learns);); } } } From 95c85692488911c9c8aa1b44027ed2c6b8b7be9c Mon Sep 17 00:00:00 2001 From: resec Date: Thu, 7 Nov 2019 19:00:03 +0800 Subject: [PATCH 3/5] [Minor] #3129 Multiple classifiers on redis working incorrectly --- src/libstat/backends/redis_backend.c | 2 +- src/libstat/stat_process.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstat/backends/redis_backend.c b/src/libstat/backends/redis_backend.c index 7d79272c0..08edf1a4f 100644 --- a/src/libstat/backends/redis_backend.c +++ b/src/libstat/backends/redis_backend.c @@ -1328,7 +1328,7 @@ rspamd_redis_connected (redisAsyncContext *c, gpointer r, gpointer priv) msg_warn_task ("skip obtaining bayes tokens for %s of classifier " "%s: not enough learns %d; %d required", rt->stcf->symbol, rt->stcf->clcf->name, - (int)rt->learned, rt->stcf->clcf->min_learns);); + (int)rt->learned, rt->stcf->clcf->min_learns); } } } diff --git a/src/libstat/stat_process.c b/src/libstat/stat_process.c index 2ad7080d0..603e0ac96 100644 --- a/src/libstat/stat_process.c +++ b/src/libstat/stat_process.c @@ -342,7 +342,7 @@ rspamd_stat_classifiers_process (struct rspamd_stat_ctx *st_ctx, if (bk_run != NULL) { if (!st->backend->finalize_process (task, bk_run, st_ctx)) { skip = TRUE; - break + break; } } } From 14495547dea7e4a0c14cf1507ba7def74b58a4ce Mon Sep 17 00:00:00 2001 From: resec Date: Fri, 8 Nov 2019 16:05:36 +0800 Subject: [PATCH 4/5] [Fix] #3129 Multiple classifiers on redis working incorrectly --- src/libstat/backends/redis_backend.c | 20 ++++++++++++++++++-- src/libstat/learn_cache/redis_cache.c | 12 +++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/libstat/backends/redis_backend.c b/src/libstat/backends/redis_backend.c index 08edf1a4f..70011a628 100644 --- a/src/libstat/backends/redis_backend.c +++ b/src/libstat/backends/redis_backend.c @@ -1425,13 +1425,29 @@ rspamd_redis_parse_classifier_opts (struct redis_stat_ctx *backend, elt = ucl_object_lookup (obj, "prefix"); if (elt == NULL || ucl_object_type (elt) != UCL_STRING) { + gchar *redis_object; + /* Default non-users statistics */ if (backend->enable_users || backend->cbref_user != -1) { - backend->redis_object = REDIS_DEFAULT_USERS_OBJECT; + redis_object = REDIS_DEFAULT_USERS_OBJECT; } else { - backend->redis_object = REDIS_DEFAULT_OBJECT; + redis_object = REDIS_DEFAULT_OBJECT; + } + + /* Prepend classifier name if defined */ + elt = ucl_object_lookup (obj, "name"); + if (elt != NULL && ucl_object_type (elt) == UCL_STRING) { + const gchar *cl_name = ucl_object_tostring (elt); + gchar *temp; + temp = g_malloc (strlen (cl_name) + strlen (redis_object) + 2); + strcpy (temp, cl_name); + strcat (temp, "_"); + strcat (temp, redis_object); + redis_object = temp; } + + backend->redis_object = redis_object; } else { /* XXX: sanity check */ diff --git a/src/libstat/learn_cache/redis_cache.c b/src/libstat/learn_cache/redis_cache.c index 0df3783ab..320f422dd 100644 --- a/src/libstat/learn_cache/redis_cache.c +++ b/src/libstat/learn_cache/redis_cache.c @@ -292,7 +292,17 @@ rspamd_stat_cache_redis_init (struct rspamd_stat_ctx *ctx, cache_ctx->redis_object = ucl_object_tostring (obj); } else { - cache_ctx->redis_object = DEFAULT_REDIS_KEY; + gchar *cl_name = st->classifier->cfg->name; + if (cl_name) { + gchar *redis_object; + redis_object = g_malloc (strlen (cl_name) + strlen (DEFAULT_REDIS_KEY) + 2); + strcpy (redis_object, cl_name); + strcat (redis_object, "_"); + strcat (redis_object, DEFAULT_REDIS_KEY); + cache_ctx->redis_object = redis_object; + } else { + cache_ctx->redis_object = DEFAULT_REDIS_KEY; + } } cache_ctx->conf_ref = conf_ref; From 901e31e9772fcdf1043ace5536805c3c5e5b90c3 Mon Sep 17 00:00:00 2001 From: resec Date: Thu, 14 Nov 2019 16:04:24 +0800 Subject: [PATCH 5/5] Revert "[Fix] #3129 Multiple classifiers on redis working incorrectly" This reverts commit 14495547dea7e4a0c14cf1507ba7def74b58a4ce. --- src/libstat/backends/redis_backend.c | 20 ++------------------ src/libstat/learn_cache/redis_cache.c | 12 +----------- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/src/libstat/backends/redis_backend.c b/src/libstat/backends/redis_backend.c index 70011a628..08edf1a4f 100644 --- a/src/libstat/backends/redis_backend.c +++ b/src/libstat/backends/redis_backend.c @@ -1425,29 +1425,13 @@ rspamd_redis_parse_classifier_opts (struct redis_stat_ctx *backend, elt = ucl_object_lookup (obj, "prefix"); if (elt == NULL || ucl_object_type (elt) != UCL_STRING) { - gchar *redis_object; - /* Default non-users statistics */ if (backend->enable_users || backend->cbref_user != -1) { - redis_object = REDIS_DEFAULT_USERS_OBJECT; + backend->redis_object = REDIS_DEFAULT_USERS_OBJECT; } else { - redis_object = REDIS_DEFAULT_OBJECT; - } - - /* Prepend classifier name if defined */ - elt = ucl_object_lookup (obj, "name"); - if (elt != NULL && ucl_object_type (elt) == UCL_STRING) { - const gchar *cl_name = ucl_object_tostring (elt); - gchar *temp; - temp = g_malloc (strlen (cl_name) + strlen (redis_object) + 2); - strcpy (temp, cl_name); - strcat (temp, "_"); - strcat (temp, redis_object); - redis_object = temp; + backend->redis_object = REDIS_DEFAULT_OBJECT; } - - backend->redis_object = redis_object; } else { /* XXX: sanity check */ diff --git a/src/libstat/learn_cache/redis_cache.c b/src/libstat/learn_cache/redis_cache.c index 320f422dd..0df3783ab 100644 --- a/src/libstat/learn_cache/redis_cache.c +++ b/src/libstat/learn_cache/redis_cache.c @@ -292,17 +292,7 @@ rspamd_stat_cache_redis_init (struct rspamd_stat_ctx *ctx, cache_ctx->redis_object = ucl_object_tostring (obj); } else { - gchar *cl_name = st->classifier->cfg->name; - if (cl_name) { - gchar *redis_object; - redis_object = g_malloc (strlen (cl_name) + strlen (DEFAULT_REDIS_KEY) + 2); - strcpy (redis_object, cl_name); - strcat (redis_object, "_"); - strcat (redis_object, DEFAULT_REDIS_KEY); - cache_ctx->redis_object = redis_object; - } else { - cache_ctx->redis_object = DEFAULT_REDIS_KEY; - } + cache_ctx->redis_object = DEFAULT_REDIS_KEY; } cache_ctx->conf_ref = conf_ref;