@cryptotaxi247 / netdata-1 / commits / 11c80885f

Fix rrdcontext metadata leak on non-dbengine hosts (#22438)

* Fix rrdcontext metadata leak on non-dbengine hosts contexts: trigger deep rrdcontext GC from chart-cleanup, not only dbengine The deep rrdcontext GC that drops archived RRDINSTANCE / RRDMETRIC entries was scheduled only by dbengine rotation. On RAM-mode (and any non-dbengine) hosts that schedule never fires, so archived metadata accumulates with every chart churn -- e.g. k8s cgroup spawn/teardown. Add rrdcontext_request_full_gc() and invoke it from svc_rrd_cleanup_obsolete_charts_from_all_hosts when at least one chart was actually freed. Reuses the existing 120 s debouncer slot, so dbengine hosts are unchanged. * contexts: keep GC deadline armed and trigger on any archival - rrdcontext_request_full_gc() rewrote rrdcontext_next_db_rotation_ut on every call. Under continuous churn (10 s maintenance loop) the deadline kept getting pushed out by another 120 s and never converged, so the deep GC never fired. Only arm when the slot is 0; the worker zeroes it after running, at which point the next archival arms a fresh window. - svc_rrdhost_cleanup_charts_marked_obsolete returned only the chart-free count. The partial-archive path (svc_rrdset_archive_obsolete_dimensions -> rrddim_free) archives dimensions without freeing the surrounding chart and still produces archived RRDMETRIC entries that need GC. Return partial_archives + full_archives and trigger on either. * contexts: count actual archives and atomicize the GC schedule slot - svc_rrdset_archive_obsolete_dimensions() now returns the number of dimensions actually archived this call, not a "fully done" bool. Callers detect "all candidates archived" by checking RRDSET_FLAG_OBSOLETE_DIMENSIONS after the call, which is cleared on entry and only re-set when some candidate could not be archived. Old contract missed partial-archive cases: archiving 3 of 5 dims returned false, callers stayed at 0 partial_archives, and the deep rrdcontext GC trigger never fired even though 3 RRDMETRICs were already archived. svc_rrdhost_cleanup_charts_marked_obsolete() now accumulates archived_items (every dim archived + 1 per chart freed for the RRDINSTANCE) and returns that as the trigger signal. - rrdcontext_next_db_rotation_ut is read/written from three threads (dbengine rotation, service maintenance, rrdcontext worker). All four access sites now go through __atomic_* with RELAXED ordering, so 32-bit platforms cannot tear the 64-bit value and the request_full_gc() check-then-set is a real CAS rather than a TOCTOU race. RELAXED is sufficient because the slot is a wake hint and does not publish any data the worker consumes; the worker re-reads dictionaries under their own locks. * contexts: don't drop GC requests racing the worker's clear pass Two paths in rrdcontext_main wrote rrdcontext_next_db_rotation_ut to 0 unconditionally: a pre-clear at the start of rrdcontext_recalculate_retention_all_hosts() and a final store after rrdcontext_garbage_collect_for_all_hosts(). With the new rrdcontext_request_full_gc() CAS'ing 0 -> deadline, a request landing between the pre-clear and the final store would arm a fresh deadline that the final unconditional store would silently overwrite, dropping the request and stranding the just-archived metadata until another chart-free triggered a new pass. - Remove the pre-clear in rrdcontext_recalculate_retention_all_hosts: leave the slot at the deadline value during the pass so concurrent request_full_gc() calls coalesce into the in-flight pass via their CAS-fail-on-non-zero semantics. - Replace the final unconditional store-zero with a CAS expecting the deadline we observed at the top of the branch. If a concurrent rrdcontext_db_rotation() (unconditional store) has armed a new deadline during the pass, leave it in place so the next iteration fires for whatever was archived after we walked. * Address review comments * Address review comments * contexts: keep extreme-cardinality guard on real dbengine rotations only The deep-GC pass is now reachable from two trigger paths -- dbengine rotation and chart-cleanup -- but extreme_cardinality.db_rotations was incremented unconditionally inside the worker's combined branch. That made the cardinality-protection guard (`extreme_cardinality.db_rotations && ...`) activate as soon as any chart-cleanup pass ran, even on hosts that never rotated dbengine, which broke its original "wait for first rotation" semantics. Move the increment out of the worker and into rrdcontext_db_rotation() via a small rrdcontext_count_db_rotation() helper. The chart-cleanup trigger (rrdcontext_request_full_gc) deliberately does not call it, so db_rotations now counts only real dbengine rotations, as the guard expects. * contexts: don't drop GC requests landing mid-pass; atomic-load db_rotations Two follow-ups: - rrdcontext_request_full_gc() previously dropped requests whose CAS failed because the worker was already mid-pass with an expired deadline. The host whose archive triggered the request may have been walked already, leaving the just-archived metadata stranded on quiet non-dbengine hosts. Add a rrdcontext_full_gc_rerun_requested flag, set when the CAS fails AND the observed deadline is already in the past (mid-pass case). The worker reads-and-clears the flag at end of pass and arms a fresh deadline iff the slot is currently 0; concurrent dbengine_rotation stores survive the follow-up CAS. Future-armed deadlines don't set the flag because their upcoming pass will see the archive. - extreme_cardinality.db_rotations is now atomically incremented from rrdcontext_count_db_rotation(). The remaining read in rrdcontext_post_process_updates() was a plain load, mixing atomic writes with non-atomic reads (a C data race; TSAN would flag it). Switch to __atomic_load_n with RELAXED ordering.

Stelios Fragkakis committed May 8, 2026 at 22:16 UTC 11c80885f4dd2d17335656b9fdc26185de9504a9
5 files changed +151 -18
src/daemon/service.c
+41 -11
@@ -28,9 +28,18 @@ static bool svc_rrddim_obsolete_to_archive(RRDDIM *rd) {
28 return true;
29 }
30
31 -static inline bool svc_rrdset_archive_obsolete_dimensions(RRDSET *st, bool all_dimensions) {
31 +// Returns the number of dimensions actually archived this call.
32 +//
33 +// Two callable shapes:
34 +// 1. all_dimensions == false and RRDSET_FLAG_OBSOLETE_DIMENSIONS unset:
35 +// early-return path. Nothing scanned, flag not touched, returns 0.
36 +// 2. Any other case: scans the dimensions. The flag is cleared up
37 +// front, then re-set at the end iff some candidate could not be
38 +// archived this pass. Callers on this path can detect "all
39 +// candidates archived" by reading the flag after the call.
40 +static inline size_t svc_rrdset_archive_obsolete_dimensions(RRDSET *st, bool all_dimensions) {
41 if(!all_dimensions && !rrdset_flag_check(st, RRDSET_FLAG_OBSOLETE_DIMENSIONS))
33 - return true;
42 + return 0;
43
44 worker_is_busy(UV_EVENT_ARCHIVE_CHART_DIMENSIONS);
45
@@ -59,12 +68,10 @@ static inline bool svc_rrdset_archive_obsolete_dimensions(RRDSET *st, bool all_d
68 }
69 dfe_done(rd);
70
62 - if(dim_archives != dim_candidates) {
71 + if(dim_archives != dim_candidates)
72 rrdset_flag_set(st, RRDSET_FLAG_OBSOLETE_DIMENSIONS);
64 - return false;
65 - }
73
67 - return true;
74 + return dim_archives;
75 }
76
77 static bool svc_rrdset_lock_for_deletion(RRDSET *st, time_t now) {
@@ -82,9 +89,9 @@ static bool svc_rrdset_lock_for_deletion(RRDSET *st, time_t now) {
89 return false;
90 }
91
85 -static inline void svc_rrdhost_cleanup_charts_marked_obsolete(RRDHOST *host) {
92 +static inline size_t svc_rrdhost_cleanup_charts_marked_obsolete(RRDHOST *host) {
93 if(!rrdhost_flag_check(host, RRDHOST_FLAG_PENDING_OBSOLETE_CHARTS|RRDHOST_FLAG_PENDING_OBSOLETE_DIMENSIONS))
87 - return;
94 + return 0;
95
96 worker_is_busy(UV_EVENT_CLEANUP_OBSOLETE_CHARTS);
97
@@ -94,6 +101,11 @@ static inline void svc_rrdhost_cleanup_charts_marked_obsolete(RRDHOST *host) {
101 size_t full_archives = 0;
102 size_t partial_candidates = 0;
103 size_t partial_archives = 0;
104 + // Total archived metadata items (RRDMETRIC + RRDINSTANCE). Used by the
105 + // caller to decide whether to schedule a deep rrdcontext GC pass.
106 + // Counts every dimension archived (each produces an archived RRDMETRIC)
107 + // plus every chart freed (each produces an archived RRDINSTANCE).
108 + size_t archived_items = 0;
109
110 time_t now = now_realtime_sec();
111 RRDSET *st;
@@ -106,7 +118,10 @@ static inline void svc_rrdhost_cleanup_charts_marked_obsolete(RRDHOST *host) {
118 if(flags & RRDSET_FLAG_OBSOLETE_DIMENSIONS) {
119 partial_candidates++;
120
109 - if(svc_rrdset_archive_obsolete_dimensions(st, false))
121 + archived_items += svc_rrdset_archive_obsolete_dimensions(st, false);
122 +
123 + // "all candidates archived" -> flag was not re-set inside.
124 + if(!rrdset_flag_check(st, RRDSET_FLAG_OBSOLETE_DIMENSIONS))
125 partial_archives++;
126 }
127
@@ -114,8 +129,11 @@ static inline void svc_rrdhost_cleanup_charts_marked_obsolete(RRDHOST *host) {
129 full_candidates++;
130
131 if(svc_rrdset_lock_for_deletion(st, now)) {
117 - if(svc_rrdset_archive_obsolete_dimensions(st, true)) {
132 + archived_items += svc_rrdset_archive_obsolete_dimensions(st, true);
133 +
134 + if(!rrdset_flag_check(st, RRDSET_FLAG_OBSOLETE_DIMENSIONS)) {
135 full_archives++;
136 + archived_items++; // rrdset_free archives the RRDINSTANCE
137
138 worker_is_busy(UV_EVENT_FREE_CHART);
139 rrdset_free(st);
@@ -134,6 +152,8 @@ static inline void svc_rrdhost_cleanup_charts_marked_obsolete(RRDHOST *host) {
152
153 if(full_archives != full_candidates)
154 rrdhost_flag_set(host, RRDHOST_FLAG_PENDING_OBSOLETE_CHARTS);
155 +
156 + return archived_items;
157 }
158
159 void svc_rrdhost_obsolete_all_charts(RRDHOST *host) {
@@ -151,12 +171,14 @@ static void svc_rrd_cleanup_obsolete_charts_from_all_hosts() {
171
172 rrd_rdlock();
173
174 + size_t archived = 0;
175 +
176 RRDHOST *host;
177 rrdhost_foreach_read(host) {
178 if(rrdhost_receiver_replicating_charts(host) || rrdhost_sender_replicating_charts(host))
179 continue;
180
159 - svc_rrdhost_cleanup_charts_marked_obsolete(host);
181 + archived += svc_rrdhost_cleanup_charts_marked_obsolete(host);
182
183 if (rrdhost_is_local(host) || IS_VIRTUAL_HOST_OS(host))
184 continue;
@@ -175,6 +197,14 @@ static void svc_rrd_cleanup_obsolete_charts_from_all_hosts() {
197 }
198
199 rrd_rdunlock();
200 +
201 + // If anything was archived (a chart freed, or just dimensions archived
202 + // on a still-live chart), schedule a deep rrdcontext GC pass. On
203 + // non-dbengine hosts, dbengine rotation never triggers it, so archived
204 + // RRDINSTANCE / RRDMETRIC entries would otherwise accumulate forever in
205 + // host->rrdctx.contexts -> rc->rrdinstances / ri->rrdmetrics.
206 + if(archived)
207 + rrdcontext_request_full_gc();
208 }
209
210 static void svc_rrdhost_cleanup_orphan_hosts(RRDHOST *protected_host) {
src/database/contexts/rrdcontext-internal.h
+6
@@ -529,6 +529,12 @@ void rrdcontext_initial_processing_after_loading(RRDCONTEXT *rc);
529
530 RRDLABELS *rrdinstance_labels(RRDINSTANCE *ri);
531
532 +// Bump the dbengine-rotations counter that gates extreme-cardinality
533 +// protection. Called from rrdcontext_db_rotation(); the chart-cleanup
534 +// trigger (rrdcontext_request_full_gc) deliberately does NOT bump it,
535 +// so the guard activates only on real dbengine rotations as before.
536 +void rrdcontext_count_db_rotation(void);
537 +
538 bool rrdcontext_post_process_updates(RRDCONTEXT *rc, bool force, RRD_FLAGS reason, bool worker_jobs);
539 void rrdcontext_post_process_queued_contexts(RRDHOST *host);
540 void rrdcontext_dispatch_queued_contexts_to_hub(RRDHOST *host, usec_t now_ut);
src/database/contexts/rrdcontext-worker.c
+50 -6
@@ -4,7 +4,7 @@
4
5 static struct {
6 bool enabled;
7 - size_t db_rotations;
7 + size_t db_rotations; // count of actual dbengine rotations only
8 size_t instances_count;
9 size_t active_vs_archived_percentage;
10 } extreme_cardinality = {
@@ -14,11 +14,16 @@ static struct {
14 .active_vs_archived_percentage = 50,
15 };
16
17 +void rrdcontext_count_db_rotation(void) {
18 + __atomic_add_fetch(&extreme_cardinality.db_rotations, 1, __ATOMIC_RELAXED);
19 +}
20 +
21 static uint64_t rrdcontext_get_next_version(RRDCONTEXT *rc);
22
23 static void rrdcontext_garbage_collect_for_all_hosts(void);
24
25 extern usec_t rrdcontext_next_db_rotation_ut;
26 +extern size_t rrdcontext_full_gc_rerun_requested;
27
28 // ----------------------------------------------------------------------------
29 // version hash calculation
@@ -133,7 +138,12 @@ void rrdcontext_recalculate_host_retention(RRDHOST *host, RRD_FLAGS reason, bool
138 }
139
140 static void rrdcontext_recalculate_retention_all_hosts(void) {
136 - rrdcontext_next_db_rotation_ut = 0;
141 + // Don't pre-clear rrdcontext_next_db_rotation_ut here -- the caller in
142 + // rrdcontext_main clears it via CAS at the end of the pass, expecting
143 + // the same deadline value it observed. Pre-clearing would let a
144 + // concurrent rrdcontext_request_full_gc() arm a new deadline that the
145 + // caller's unconditional store-to-zero would then silently overwrite,
146 + // dropping the request.
147 RRDHOST *host;
148 dfe_start_reentrant(rrdhost_root_index, host) {
149 worker_is_busy(WORKER_JOB_RETENTION);
@@ -690,7 +700,7 @@ bool rrdcontext_post_process_updates(RRDCONTEXT *rc, bool force, RRD_FLAGS reaso
700 dfe_done(ri);
701
702 if(extreme_cardinality.enabled &&
693 - extreme_cardinality.db_rotations &&
703 + __atomic_load_n(&extreme_cardinality.db_rotations, __ATOMIC_RELAXED) &&
704 instances_active &&
705 instances_no_tier0 >= extreme_cardinality.instances_count) {
706 size_t percent = (100 * instances_no_tier0 / instances_active);
@@ -1058,11 +1068,45 @@ void rrdcontext_main(void *ptr) {
1068
1069 usec_t now_ut = now_realtime_usec();
1070
1061 - if(rrdcontext_next_db_rotation_ut && now_ut > rrdcontext_next_db_rotation_ut) {
1062 - extreme_cardinality.db_rotations++;
1071 + usec_t deadline = __atomic_load_n(&rrdcontext_next_db_rotation_ut, __ATOMIC_RELAXED);
1072 + if(deadline && now_ut > deadline) {
1073 + // db_rotations is bumped by rrdcontext_count_db_rotation() from
1074 + // rrdcontext_db_rotation() only -- the chart-cleanup trigger
1075 + // (rrdcontext_request_full_gc) does NOT bump it, so the
1076 + // extreme-cardinality guard at line ~700 still activates only
1077 + // after a real dbengine rotation.
1078 rrdcontext_recalculate_retention_all_hosts();
1079 rrdcontext_garbage_collect_for_all_hosts();
1065 - rrdcontext_next_db_rotation_ut = 0;
1080 + // Clear the slot only if it still holds the deadline we
1081 + // processed. Two writers can race this pass:
1082 + // - rrdcontext_request_full_gc() arms only when the slot is
1083 + // zero, but the slot stays non-zero throughout this pass,
1084 + // so any such request during the pass is coalesced into
1085 + // the in-flight pass and does not arm a new deadline.
1086 + // - rrdcontext_db_rotation() stores unconditionally. If it
1087 + // fires during this pass, the slot now holds a fresh
1088 + // deadline; the CAS here fails and that deadline drives
1089 + // the next iteration.
1090 + __atomic_compare_exchange_n(&rrdcontext_next_db_rotation_ut,
1091 + &deadline, 0,
1092 + false,
1093 + __ATOMIC_RELAXED, __ATOMIC_RELAXED);
1094 +
1095 + // If a rrdcontext_request_full_gc() landed mid-pass (its CAS
1096 + // failed because the slot was non-zero with an expired
1097 + // deadline), the host whose archive motivated it may have
1098 + // already been walked in this pass. Schedule a follow-up:
1099 + // arm a fresh deadline iff the slot is currently zero. If
1100 + // another path (e.g. dbengine rotation) just armed a new
1101 + // deadline, the CAS leaves it alone.
1102 + if(__atomic_exchange_n(&rrdcontext_full_gc_rerun_requested, 0, __ATOMIC_RELAXED)) {
1103 + usec_t expected = 0;
1104 + usec_t fresh = now_realtime_usec() + FULL_RETENTION_SCAN_DELAY_AFTER_DB_ROTATION_SECS * USEC_PER_SEC;
1105 + __atomic_compare_exchange_n(&rrdcontext_next_db_rotation_ut,
1106 + &expected, fresh,
1107 + false,
1108 + __ATOMIC_RELAXED, __ATOMIC_RELAXED);
1109 + }
1110 }
1111
1112 size_t hub_queued_contexts_for_all_hosts = 0;
src/database/contexts/rrdcontext.c
+53 -1
@@ -106,10 +106,62 @@ ALWAYS_INLINE void rrdcontext_host_child_connected(RRDHOST *host) {
106 rrdset_foreach_done(st);
107 }
108
109 +// Cross-thread schedule slot for the deep rrdcontext GC pass. Written by
110 +// rrdcontext_db_rotation() (dbengine rotation), rrdcontext_request_full_gc()
111 +// (chart-cleanup), and the rrdcontext worker (reset to 0 after a pass).
112 +// All accesses go through __atomic_* so 32-bit platforms cannot tear the
113 +// 64-bit value, and the request_full_gc() check-then-set is a real CAS
114 +// rather than a TOCTOU race.
115 usec_t rrdcontext_next_db_rotation_ut = 0;
116 +
117 +// Companion flag for rrdcontext_request_full_gc(): set when its CAS
118 +// failed because the worker was already mid-pass with the deadline in
119 +// the past. The worker reads-and-clears this after each pass and arms
120 +// a follow-up if set, so an archive that landed mid-pass (after the
121 +// worker had already walked its host) doesn't get stranded.
122 +size_t rrdcontext_full_gc_rerun_requested = 0;
123 +
124 ALWAYS_INLINE void rrdcontext_db_rotation(void) {
125 // called when the db rotates its database
112 - rrdcontext_next_db_rotation_ut = now_realtime_usec() + FULL_RETENTION_SCAN_DELAY_AFTER_DB_ROTATION_SECS * USEC_PER_SEC;
126 + __atomic_store_n(&rrdcontext_next_db_rotation_ut,
127 + now_realtime_usec() + FULL_RETENTION_SCAN_DELAY_AFTER_DB_ROTATION_SECS * USEC_PER_SEC,
128 + __ATOMIC_RELAXED);
129 + // Count only real dbengine rotations (not chart-cleanup-driven scans),
130 + // so the extreme-cardinality guard in the rrdcontext worker preserves
131 + // its original "wait for first rotation" semantics.
132 + rrdcontext_count_db_rotation();
133 +}
134 +
135 +ALWAYS_INLINE void rrdcontext_request_full_gc(void) {
136 + // Schedule a deep rrdcontext GC pass. Called from chart-cleanup paths
137 + // (e.g. svc_rrd_cleanup_obsolete_charts_from_all_hosts) so non-dbengine
138 + // hosts also drop archived rrdinstance / rrdmetric entries -- otherwise
139 + // those grow unbounded with chart churn (k8s cgroups, etc.) because the
140 + // dbengine rotation trigger never fires on them.
141 + //
142 + // The maintenance loop runs every 10 s; under continuous churn it would
143 + // free charts on every pass. Unconditionally rewriting the deadline
144 + // would push it out by another 120 s on every call, so under sustained
145 + // churn the deep GC would never actually fire. Only arm the deadline if
146 + // no pass is already scheduled; the worker resets the slot to 0 after
147 + // it runs, at which point the next chart-free arms a fresh window.
148 + // Multiple requests within that window coalesce into a single GC pass.
149 + usec_t now = now_realtime_usec();
150 + usec_t expected = 0;
151 + usec_t deadline = now + FULL_RETENTION_SCAN_DELAY_AFTER_DB_ROTATION_SECS * USEC_PER_SEC;
152 + if(!__atomic_compare_exchange_n(&rrdcontext_next_db_rotation_ut,
153 + &expected, deadline,
154 + false,
155 + __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
156 + // CAS failed -- expected now holds the slot's actual value.
157 + // If that deadline is in the past, the worker is mid-pass and
158 + // may already have walked the host whose archive triggered this
159 + // request. Mark for a follow-up so the worker schedules another
160 + // pass after the current one. Future-armed deadlines need no
161 + // follow-up: their upcoming pass will see the archive.
162 + if(expected && expected <= now)
163 + __atomic_store_n(&rrdcontext_full_gc_rerun_requested, 1, __ATOMIC_RELAXED);
164 + }
165 }
166
167 int rrdcontext_find_dimension_uuid(RRDSET *st, const char *id, nd_uuid_t *store_uuid) {
src/database/contexts/rrdcontext.h
+1
@@ -128,6 +128,7 @@ void rrdcontext_hub_pending_checkpoint_replay(RRDHOST *host);
128 // public API for threads
129
130 void rrdcontext_db_rotation(void);
131 +void rrdcontext_request_full_gc(void);
132 void rrdcontext_main(void *);
133
134 // ----------------------------------------------------------------------------