commit 6b0297d66a1a4e12085bf5209a66a273e9a58d39
parent 251fee0ec8f187b8bf0dc6e5fc10363c3fbb10a4
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 5 Aug 2026 22:24:26 +0200
fix statistics GC logic: properly compute intervals
Diffstat:
3 files changed, 230 insertions(+), 3 deletions(-)
diff --git a/src/backenddb/meson.build b/src/backenddb/meson.build
@@ -250,6 +250,7 @@ test_merchantdb = executable(
talerjson_dep,
gnunetutil_dep,
gnunetjson_dep,
+ gnunetpq_dep,
json_dep,
pq_dep,
],
diff --git a/src/backenddb/pg_statistics_helpers.sql b/src/backenddb/pg_statistics_helpers.sql
@@ -614,7 +614,7 @@ LANGUAGE plpgsql
AS $$
DECLARE
my_rec RECORD;
- my_range TEXT;
+ my_range INTERVAL;
my_now INT8;
my_end INT8;
BEGIN
@@ -626,8 +626,17 @@ BEGIN
,ages[array_length(ages,1)] AS age
FROM merchant_statistic_bucket_meta
LOOP
- my_range = '1 ' || my_rec.range::TEXT;
- my_end = my_now - my_rec.age * EXTRACT(SECONDS FROM (SELECT my_range::INTERVAL)); -- age is given in multiples of the range (in seconds)
+ IF my_rec.range = 'quarter'
+ THEN
+ -- '1 quarter' is not a valid PostgreSQL interval
+ my_range = INTERVAL '3 months';
+ ELSE
+ my_range = ('1 ' || my_rec.range::TEXT)::INTERVAL;
+ END IF;
+ -- age is given in multiples of the range; EPOCH (and not SECONDS,
+ -- which is merely the seconds *field* of the interval and thus zero
+ -- for all of our ranges) converts that range into seconds
+ my_end = my_now - my_rec.age * EXTRACT(EPOCH FROM my_range);
IF my_rec.stype = 'amount'
THEN
DELETE
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -24,9 +24,13 @@
#include "microhttpd.h"
#include <taler/taler_util.h>
#include <taler/taler_json_lib.h>
+#include <taler/taler_pq_lib.h>
#include <taler/taler_signatures.h>
#include "taler/taler_merchant_util.h"
#include "merchantdb_lib.h"
+#include "helper.h"
+#include "merchant-database/gc.h"
+#include "merchant-database/iterate_statistic_bucket_counters.h"
#include "merchant-database/iterate_kyc_statuses.h"
#include "merchant-database/insert_kyc_status.h"
#include "merchant-database/delete_contract_terms.h"
@@ -7724,6 +7728,218 @@ test_pending_webhooks (void)
}
+/* ********** Statistics ********** */
+
+
+/**
+ * Closure for the statistics tests.
+ */
+struct TestStatistics_Closure
+{
+ /**
+ * Instance the statistics are collected for.
+ */
+ struct InstanceData instance;
+};
+
+
+/**
+ * Closure for #count_bucket_counter_cb().
+ */
+struct CountBuckets_Closure
+{
+ /**
+ * Number of bucket rows we were called with.
+ */
+ unsigned int count;
+};
+
+
+/**
+ * Runs @a sql directly on the database connection of the test. Used to
+ * set up statistics state that has no dedicated MERCHANTDB entry point
+ * (the statistics tables are only ever written by triggers).
+ *
+ * @param sql the SQL statement(s) to execute
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+statistics_exec_sql (const char *sql)
+{
+ struct GNUNET_PQ_ExecuteStatement es[] = {
+ GNUNET_PQ_make_execute (sql),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+
+ TEST_COND_RET_ON_FAIL (GNUNET_OK ==
+ GNUNET_PQ_exec_statements (pg->conn,
+ es),
+ "Direct SQL execution failed\n");
+ return 0;
+}
+
+
+/**
+ * Counts the bucket rows we are called for.
+ *
+ * @param cls a `struct CountBuckets_Closure *`
+ * @param description description of the statistic
+ * @param bucket_start start time of the bucket
+ * @param bucket_end end time of the bucket
+ * @param bucket_range range of the bucket
+ * @param cumulative_counter counter value
+ */
+static void
+count_bucket_counter_cb (void *cls,
+ const char *description,
+ struct GNUNET_TIME_Timestamp bucket_start,
+ struct GNUNET_TIME_Timestamp bucket_end,
+ const char *bucket_range,
+ uint64_t cumulative_counter)
+{
+ struct CountBuckets_Closure *cbc = cls;
+
+ (void) description;
+ (void) bucket_start;
+ (void) bucket_end;
+ (void) bucket_range;
+ (void) cumulative_counter;
+ cbc->count++;
+}
+
+
+/**
+ * Determines how many bucket rows exist for @a slug.
+ *
+ * @param instance the instance to look at.
+ * @param slug the statistic to count the buckets of.
+ * @param[out] count set to the number of bucket rows.
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+count_bucket_counters (const struct InstanceData *instance,
+ const char *slug,
+ unsigned int *count)
+{
+ struct CountBuckets_Closure cbc = {
+ .count = 0
+ };
+
+ TEST_SET_INSTANCE (instance->instance.id,
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT);
+ TEST_COND_RET_ON_FAIL (0 <=
+ TALER_MERCHANTDB_iterate_statistic_bucket_counters (
+ pg,
+ instance->instance.id,
+ slug,
+ &count_bucket_counter_cb,
+ &cbc),
+ "Lookup of bucket statistics failed\n");
+ *count = cbc.count;
+ return 0;
+}
+
+
+/**
+ * Tests that garbage collection does not throw away bucket statistics
+ * that are well within their retention age. Regression test: the
+ * retention cut-off in merchant_statistic_bucket_gc() used to be
+ * computed with EXTRACT(SECONDS FROM ...), which is the seconds *field*
+ * of the interval and thus zero for every range we use, so the cut-off
+ * degenerated to "now" and every bucket row was deleted on every run.
+ *
+ * @param instance the instance to collect statistics for.
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+test_statistics_bucket_gc (const struct InstanceData *instance)
+{
+ unsigned int before;
+ unsigned int after;
+
+ TEST_SET_INSTANCE (instance->instance.id,
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT);
+ TEST_RET_ON_FAIL (statistics_exec_sql (
+ "CALL merchant_do_bump_number_stat"
+ " ('tokens-issued'"
+ " ,CURRENT_TIMESTAMP(0)::TIMESTAMP"
+ " ,1)"));
+ TEST_RET_ON_FAIL (count_bucket_counters (instance,
+ "tokens-issued",
+ &before));
+ TEST_COND_RET_ON_FAIL (0 < before,
+ "Bumping a bucket statistic recorded nothing\n");
+ TEST_COND_RET_ON_FAIL (GNUNET_OK ==
+ TALER_MERCHANTDB_gc (pg),
+ "Garbage collection failed\n");
+ TEST_RET_ON_FAIL (count_bucket_counters (instance,
+ "tokens-issued",
+ &after));
+ TEST_COND_RET_ON_FAIL (before == after,
+ "Garbage collection deleted bucket statistics\n");
+ return 0;
+}
+
+
+/**
+ * Prepares for the statistics tests.
+ *
+ * @param[out] cls the closure to initialize.
+ */
+static void
+pre_test_statistics (struct TestStatistics_Closure *cls)
+{
+ make_instance ("test_inst_statistics",
+ &cls->instance);
+}
+
+
+/**
+ * Cleans up after the statistics tests.
+ *
+ * @param[in,out] cls the closure to clean up.
+ */
+static void
+post_test_statistics (struct TestStatistics_Closure *cls)
+{
+ free_instance_data (&cls->instance);
+}
+
+
+/**
+ * Runs the statistics tests.
+ *
+ * @param cls the closure to use.
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+run_test_statistics (struct TestStatistics_Closure *cls)
+{
+ TEST_RET_ON_FAIL (test_insert_instance (&cls->instance,
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
+ TEST_RET_ON_FAIL (test_statistics_bucket_gc (&cls->instance));
+ return 0;
+}
+
+
+/**
+ * Takes care of statistics testing.
+ *
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+test_statistics (void)
+{
+ struct TestStatistics_Closure test_cls;
+ int test_result;
+
+ pre_test_statistics (&test_cls);
+ test_result = run_test_statistics (&test_cls);
+ post_test_statistics (&test_cls);
+ return test_result;
+}
+
+
/**
* Function that runs all tests.
*
@@ -7743,6 +7959,7 @@ run_tests (void)
TEST_RET_ON_FAIL (test_templates ());
TEST_RET_ON_FAIL (test_webhooks ());
TEST_RET_ON_FAIL (test_pending_webhooks ());
+ TEST_RET_ON_FAIL (test_statistics ());
return 0;
}
