journal-rate-limit: replace in-house management of JournalRateLimitGroup with OrderedHashmap

No functional change, just refactoring.

5 files changed, 60 insertions, 123 deletions

diff --git a/src/journal/journald-rate-limit.c b/src/journal/journald-rate-limit.c
index afba668e0f..a1ae17269c 100644
--- a/src/journal/journald-rate-limit.c
+++ b/src/journal/journald-rate-limit.c
@@ -1,18 +1,13 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
-#include <errno.h>
-
 #include "alloc-util.h"
 #include "hashmap.h"
 #include "journald-rate-limit.h"
-#include "list.h"
 #include "logarithm.h"
-#include "random-util.h"
 #include "string-util.h"
 #include "time-util.h"
 
 #define POOLS_MAX 5
-#define BUCKETS_MAX 127
 #define GROUPS_MAX 2047
 
 static const int priority_map[] = {
@@ -26,17 +21,14 @@ static const int priority_map[] = {
         [LOG_DEBUG]   = 4,
 };
 
-typedef struct JournalRateLimitPool JournalRateLimitPool;
-typedef struct JournalRateLimitGroup JournalRateLimitGroup;
-
-struct JournalRateLimitPool {
+typedef struct JournalRateLimitPool {
         usec_t begin;
         unsigned num;
         unsigned suppressed;
-};
+} JournalRateLimitPool;
 
-struct JournalRateLimitGroup {
-        JournalRateLimit *parent;
+typedef struct JournalRateLimitGroup {
+        OrderedHashmap *groups_by_id;
 
         char *id;
 
@@ -44,49 +36,16 @@ struct JournalRateLimitGroup {
         usec_t interval;
 
         JournalRateLimitPool pools[POOLS_MAX];
-        uint64_t hash;
-
-        LIST_FIELDS(JournalRateLimitGroup, bucket);
-        LIST_FIELDS(JournalRateLimitGroup, lru);
-};
-
-struct JournalRateLimit {
-
-        JournalRateLimitGroup* buckets[BUCKETS_MAX];
-        JournalRateLimitGroup *lru, *lru_tail;
-
-        unsigned n_groups;
-
-        uint8_t hash_key[16];
-};
-
-JournalRateLimit *journal_ratelimit_new(void) {
-        JournalRateLimit *r;
-
-        r = new0(JournalRateLimit, 1);
-        if (!r)
-                return NULL;
-
-        random_bytes(r->hash_key, sizeof(r->hash_key));
-
-        return r;
-}
+} JournalRateLimitGroup;
 
 static JournalRateLimitGroup* journal_ratelimit_group_free(JournalRateLimitGroup *g) {
         if (!g)
                 return NULL;
 
-        if (g->parent) {
-                assert(g->parent->n_groups > 0);
-
-                if (g->parent->lru_tail == g)
-                        g->parent->lru_tail = g->lru_prev;
-
-                LIST_REMOVE(lru, g->parent->lru, g);
-                LIST_REMOVE(bucket, g->parent->buckets[g->hash % BUCKETS_MAX], g);
-
-                g->parent->n_groups--;
-        }
+        if (g->groups_by_id && g->id)
+                /* The group is already removed from the hashmap when this is called from the
+                 * destructor of the hashmap. Hence, do not check the return value here. */
+                ordered_hashmap_remove_value(g->groups_by_id, g->id, g);
 
         free(g->id);
         return mfree(g);
@@ -94,14 +53,13 @@ static JournalRateLimitGroup* journal_ratelimit_group_free(JournalRateLimitGroup
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(JournalRateLimitGroup*, journal_ratelimit_group_free);
 
-void journal_ratelimit_free(JournalRateLimit *r) {
-        assert(r);
-
-        while (r->lru)
-                journal_ratelimit_group_free(r->lru);
-
-        free(r);
-}
+DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
+        journal_ratelimit_group_hash_ops,
+        char,
+        string_hash_func,
+        string_compare_func,
+        JournalRateLimitGroup,
+        journal_ratelimit_group_free);
 
 static bool journal_ratelimit_group_expired(JournalRateLimitGroup *g, usec_t ts) {
         assert(g);
@@ -113,26 +71,29 @@ static bool journal_ratelimit_group_expired(JournalRateLimitGroup *g, usec_t ts)
         return true;
 }
 
-static void journal_ratelimit_vacuum(JournalRateLimit *r, usec_t ts) {
-        assert(r);
+static void journal_ratelimit_vacuum(OrderedHashmap *groups_by_id, usec_t ts) {
 
         /* Makes room for at least one new item, but drop all expired items too. */
 
-        while (r->n_groups >= GROUPS_MAX ||
-               (r->lru_tail && journal_ratelimit_group_expired(r->lru_tail, ts)))
-                journal_ratelimit_group_free(r->lru_tail);
+        while (ordered_hashmap_size(groups_by_id) >= GROUPS_MAX)
+                journal_ratelimit_group_free(ordered_hashmap_first(groups_by_id));
+
+        JournalRateLimitGroup *g;
+        while ((g = ordered_hashmap_first(groups_by_id)) && journal_ratelimit_group_expired(g, ts))
+                journal_ratelimit_group_free(g);
 }
 
 static int journal_ratelimit_group_new(
-                JournalRateLimit *rl,
+                OrderedHashmap **groups_by_id,
                 const char *id,
                 usec_t interval,
                 usec_t ts,
                 JournalRateLimitGroup **ret) {
 
         _cleanup_(journal_ratelimit_group_freep) JournalRateLimitGroup *g = NULL;
+        int r;
 
-        assert(rl);
+        assert(groups_by_id);
         assert(id);
         assert(ret);
 
@@ -142,51 +103,40 @@ static int journal_ratelimit_group_new(
 
         *g = (JournalRateLimitGroup) {
                 .id = strdup(id),
-                .hash = siphash24_string(id, rl->hash_key),
                 .interval = interval,
         };
         if (!g->id)
                 return -ENOMEM;
 
-        journal_ratelimit_vacuum(rl, ts);
+        journal_ratelimit_vacuum(*groups_by_id, ts);
 
-        LIST_PREPEND(bucket, rl->buckets[g->hash % BUCKETS_MAX], g);
-        LIST_PREPEND(lru, rl->lru, g);
-        if (!g->lru_next)
-                rl->lru_tail = g;
-        rl->n_groups++;
+        r = ordered_hashmap_ensure_put(groups_by_id, &journal_ratelimit_group_hash_ops, g->id, g);
+        if (r < 0)
+                return r;
+        assert(r > 0);
 
-        g->parent = rl;
+        g->groups_by_id = *groups_by_id;
 
         *ret = TAKE_PTR(g);
         return 0;
 }
 
 static int journal_ratelimit_group_acquire(
-                JournalRateLimit *rl,
+                OrderedHashmap **groups_by_id,
                 const char *id,
                 usec_t interval,
                 usec_t ts,
                 JournalRateLimitGroup **ret) {
 
-        JournalRateLimitGroup *head, *g = NULL;
-        uint64_t h;
+        JournalRateLimitGroup *g;
 
-        assert(rl);
+        assert(groups_by_id);
         assert(id);
         assert(ret);
 
-        h = siphash24_string(id, rl->hash_key);
-        head = rl->buckets[h % BUCKETS_MAX];
-
-        LIST_FOREACH(bucket, i, head)
-                if (streq(i->id, id)) {
-                        g = i;
-                        break;
-                }
-
+        g = ordered_hashmap_get(*groups_by_id, id);
         if (!g)
-                return journal_ratelimit_group_new(rl, id, interval, ts, ret);
+                return journal_ratelimit_group_new(groups_by_id, id, interval, ts, ret);
 
         g->interval = interval;
 
@@ -223,7 +173,7 @@ static unsigned burst_modulate(unsigned burst, uint64_t available) {
 }
 
 int journal_ratelimit_test(
-                JournalRateLimit *rl,
+                OrderedHashmap **groups_by_id,
                 const char *id,
                 usec_t rl_interval,
                 unsigned rl_burst,
@@ -236,6 +186,7 @@ int journal_ratelimit_test(
         usec_t ts;
         int r;
 
+        assert(groups_by_id);
         assert(id);
 
         /* Returns:
@@ -245,12 +196,9 @@ int journal_ratelimit_test(
          * < 0   → error
          */
 
-        if (!rl)
-                return 1;
-
         ts = now(CLOCK_MONOTONIC);
 
-        r = journal_ratelimit_group_acquire(rl, id, rl_interval, ts, &g);
+        r = journal_ratelimit_group_acquire(groups_by_id, id, rl_interval, ts, &g);
         if (r < 0)
                 return r;
 
diff --git a/src/journal/journald-rate-limit.h b/src/journal/journald-rate-limit.h
index 4539e174c3..566bba4be2 100644
--- a/src/journal/journald-rate-limit.h
+++ b/src/journal/journald-rate-limit.h
@@ -1,14 +1,13 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-#include "time-util.h"
+#include <inttypes.h>
 
-typedef struct JournalRateLimit JournalRateLimit;
+#include "hashmap.h"
+#include "time-util.h"
 
-JournalRateLimit *journal_ratelimit_new(void);
-void journal_ratelimit_free(JournalRateLimit *r);
 int journal_ratelimit_test(
-                JournalRateLimit *rl,
+                OrderedHashmap **groups_by_id,
                 const char *id,
                 usec_t rl_interval,
                 unsigned rl_burst,
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index b2203a87ff..1d03350d6a 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -1274,7 +1274,7 @@ void server_dispatch_message(
                 (void) server_determine_space(s, &available, /* limit= */ NULL);
 
                 rl = journal_ratelimit_test(
-                                s->ratelimit,
+                                &s->ratelimit_groups_by_id,
                                 c->unit,
                                 c->log_ratelimit_interval,
                                 c->log_ratelimit_burst,
@@ -2809,10 +2809,6 @@ int server_init(Server *s, const char *namespace) {
         if (r < 0)
                 return r;
 
-        s->ratelimit = journal_ratelimit_new();
-        if (!s->ratelimit)
-                return log_oom();
-
         r = cg_get_root_path(&s->cgroup_root);
         if (r < 0)
                 return log_error_errno(r, "Failed to acquire cgroup root path: %m");
@@ -2913,8 +2909,7 @@ Server* server_free(Server *s) {
         safe_close(s->notify_fd);
         safe_close(s->forward_socket_fd);
 
-        if (s->ratelimit)
-                journal_ratelimit_free(s->ratelimit);
+        ordered_hashmap_free(s->ratelimit_groups_by_id);
 
         server_unmap_seqnum_file(s->seqnum, sizeof(*s->seqnum));
         server_unmap_seqnum_file(s->kernel_seqnum, sizeof(*s->kernel_seqnum));
diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h
index 3878061d09..5138908996 100644
--- a/src/journal/journald-server.h
+++ b/src/journal/journald-server.h
@@ -14,7 +14,6 @@ typedef struct Server Server;
 #include "hashmap.h"
 #include "journal-file.h"
 #include "journald-context.h"
-#include "journald-rate-limit.h"
 #include "journald-stream.h"
 #include "list.h"
 #include "prioq.h"
@@ -108,7 +107,7 @@ struct Server {
 
         char *buffer;
 
-        JournalRateLimit *ratelimit;
+        OrderedHashmap *ratelimit_groups_by_id;
         usec_t sync_interval_usec;
         usec_t ratelimit_interval;
         unsigned ratelimit_burst;
diff --git a/src/journal/test-journald-rate-limit.c b/src/journal/test-journald-rate-limit.c
index 89d69bb950..a08fabab26 100644
--- a/src/journal/test-journald-rate-limit.c
+++ b/src/journal/test-journald-rate-limit.c
@@ -4,41 +4,37 @@
 #include "tests.h"
 
 TEST(journal_ratelimit_test) {
-        JournalRateLimit *rl;
+        _cleanup_ordered_hashmap_free_ OrderedHashmap *rl = NULL;
         int r;
 
-        assert_se(rl = journal_ratelimit_new());
-
         for (unsigned i = 0; i < 20; i++) {
-                r = journal_ratelimit_test(rl, "hoge", USEC_PER_SEC, 10, LOG_DEBUG, 0);
+                r = journal_ratelimit_test(&rl, "hoge", USEC_PER_SEC, 10, LOG_DEBUG, 0);
                 assert_se(r == (i < 10 ? 1 : 0));
-                r = journal_ratelimit_test(rl, "foo", 10 * USEC_PER_SEC, 10, LOG_DEBUG, 0);
+                r = journal_ratelimit_test(&rl, "foo", 10 * USEC_PER_SEC, 10, LOG_DEBUG, 0);
                 assert_se(r == (i < 10 ? 1 : 0));
         }
 
         /* Different priority group with the same ID is not ratelimited. */
-        assert_se(journal_ratelimit_test(rl, "hoge", USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
-        assert_se(journal_ratelimit_test(rl, "foo", 10 * USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
+        assert_se(journal_ratelimit_test(&rl, "hoge", USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
+        assert_se(journal_ratelimit_test(&rl, "foo", 10 * USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
         /* Still LOG_DEBUG is ratelimited. */
-        assert_se(journal_ratelimit_test(rl, "hoge", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 0);
-        assert_se(journal_ratelimit_test(rl, "foo", 10 * USEC_PER_SEC, 10, LOG_DEBUG, 0) == 0);
+        assert_se(journal_ratelimit_test(&rl, "hoge", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 0);
+        assert_se(journal_ratelimit_test(&rl, "foo", 10 * USEC_PER_SEC, 10, LOG_DEBUG, 0) == 0);
         /* Different ID is not ratelimited. */
-        assert_se(journal_ratelimit_test(rl, "quux", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 1);
+        assert_se(journal_ratelimit_test(&rl, "quux", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 1);
 
         usleep_safe(USEC_PER_SEC);
 
         /* The ratelimit is now expired (11 trials are suppressed, so the return value should be 12). */
-        assert_se(journal_ratelimit_test(rl, "hoge", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 1 + 11);
+        assert_se(journal_ratelimit_test(&rl, "hoge", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 1 + 11);
 
         /* foo is still ratelimited. */
-        assert_se(journal_ratelimit_test(rl, "foo", 10 * USEC_PER_SEC, 10, LOG_DEBUG, 0) == 0);
+        assert_se(journal_ratelimit_test(&rl, "foo", 10 * USEC_PER_SEC, 10, LOG_DEBUG, 0) == 0);
 
         /* Still other priority and/or other IDs are not ratelimited. */
-        assert_se(journal_ratelimit_test(rl, "hoge", USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
-        assert_se(journal_ratelimit_test(rl, "foo", 10 * USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
-        assert_se(journal_ratelimit_test(rl, "quux", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 1);
-
-        journal_ratelimit_free(rl);
+        assert_se(journal_ratelimit_test(&rl, "hoge", USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
+        assert_se(journal_ratelimit_test(&rl, "foo", 10 * USEC_PER_SEC, 10, LOG_INFO, 0) == 1);
+        assert_se(journal_ratelimit_test(&rl, "quux", USEC_PER_SEC, 10, LOG_DEBUG, 0) == 1);
 }
 
 DEFINE_TEST_MAIN(LOG_INFO);