Fix sender replication counter leak on obsolete charts (#22428)
streaming: stop sender replication counter leak on obsolete charts The sender was bumping rrdhost_sender_replicating_charts when pushing obsolete chart definitions, but the receiver skips replication for obsolete charts so the counter never decremented. On stable streaming children it grew monotonically, permanently gating obsolete-chart cleanup and -- in RAM mode -- leaking dim mmaps until the cgroup OOM-killer fired Closes the leak end-to-end across concurrent obsoletion, sender reset, and disconnect: claim-before-publish ordering, post-CAS recheck with atomic rollback for OBSOLETE or lost readiness, precise per-chart accounting in sender reset (drops the racy force-zero), and an early READY_4_METRICS clear on disconnect.
Stelios Fragkakis committed
May 7, 2026 at 00:02 UTC
177a3b6ffa883706840bf38de29c7c23684184ed
3 files changed
+107
-13
src/database/rrdset.c
+16
@@ -127,6 +127,22 @@ void rrdset_is_obsolete___safe_from_collector_thread(RRDSET *st) {
127
128
st->last_accessed_time_s = now_realtime_sec();
129
130
+ // The parent skips replication for obsolete charts, so the natural
131
+ // "replication finished" decrement at stream-replication-sender.c will
132
+ // never fire for this chart. Release any pending replication slot now,
133
+ // otherwise rrdhost_sender_replicating_charts pins above zero and
134
+ // permanently blocks svc_rrd_cleanup_obsolete_charts_from_all_hosts.
135
+ // Mirror the natural-finalize path's pulse-status flip on the 0/1
136
+ // boundary so SND_REPLICATING -> SND_RUNNING is observed by pulse.
137
+ RRDSET_FLAGS old_repl = rrdset_flag_set_and_clear(
138
+ st,
139
+ RRDSET_FLAG_SENDER_REPLICATION_FINISHED,
140
+ RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS);
141
+ if(old_repl & RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS) {
142
+ if(rrdhost_sender_replicating_charts_minus_one(st->rrdhost) == 0)
143
+ pulse_host_status(st->rrdhost, PULSE_HOST_STATUS_SND_RUNNING, 0);
144
+ }
145
+
146
rrdset_metadata_updated(st);
147
148
// the chart will not get more updates (data collection)
src/streaming/protocol/command-chart-definition.c
+57
-6
@@ -117,13 +117,64 @@ bool stream_sender_send_rrdset_definition(BUFFER *wb, RRDSET *st) {
117
(unsigned long long)db_last_time_t,
118
(unsigned long long)now);
119
120
- RRDSET_FLAGS old = rrdset_flag_set_and_clear(st, RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS, RRDSET_FLAG_SENDER_REPLICATION_FINISHED);
121
- if(!(old & RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS)) {
122
- if(rrdhost_sender_replicating_charts_plus_one(st->rrdhost) == 1)
123
- pulse_host_status(st->rrdhost, PULSE_HOST_STATUS_SND_REPLICATING, 0);
124
- }
120
+ // The receiver skips replication for obsolete charts (stream-receiver.c),
121
+ // so do not enter the replication bookkeeping here either: it would pin
122
+ // rrdhost_sender_replicating_charts and permanently gate the cleanup loop
123
+ // in svc_rrd_cleanup_obsolete_charts_from_all_hosts.
124
+ if(!rrdset_flag_check(st, RRDSET_FLAG_OBSOLETE)) {
125
+ // Claim before publish: increment the host counter BEFORE setting
126
+ // RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS, so any concurrent
127
+ // observer (obsoleter at rrdset.c, finalizer at
128
+ // stream-replication-sender.c, reset at stream-sender.c) that
129
+ // sees the flag set in its CAS old-value also sees a counter
130
+ // already incremented to match. Without this ordering, an
131
+ // observer can clear the flag and call rrdhost_sender_replicating
132
+ // _charts_minus_one() before the sender's increment, causing a
133
+ // transient underflow that other concurrent inc/dec can latch.
134
+ bool first_claim = (rrdhost_sender_replicating_charts_plus_one(st->rrdhost) == 1);
135
+
136
+ RRDSET_FLAGS old = rrdset_flag_set_and_clear(st, RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS, RRDSET_FLAG_SENDER_REPLICATION_FINISHED);
137
+ bool we_caused_transition = !(old & RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS);
138
+
139
+ if(we_caused_transition) {
140
+ if(first_claim)
141
+ pulse_host_status(st->rrdhost, PULSE_HOST_STATUS_SND_REPLICATING, 0);
142
+ }
143
+ else {
144
+ // Lost race: another sender already had IN_PROGRESS set, so
145
+ // our +1 is one too many. Roll it back; mirror the natural-
146
+ // finalize pulse-status flip on the 0 boundary.
147
+ if(rrdhost_sender_replicating_charts_minus_one(st->rrdhost) == 0)
148
+ pulse_host_status(st->rrdhost, PULSE_HOST_STATUS_SND_RUNNING, 0);
149
+ }
150
126
- replication_progress = true;
151
+ // Recheck after our CAS: a concurrent obsoleter may have set
152
+ // RRDSET_FLAG_OBSOLETE, OR a concurrent disconnect may have
153
+ // cleared the host's metadata-readiness flag. In either case the
154
+ // parent will not drive replication for this chart to completion
155
+ // and the natural decrement never fires; undo our state to keep
156
+ // the host counter and pulse status balanced. The atomic CAS
157
+ // ensures only the thread that observes IN_PROGRESS=1 actually
158
+ // decrements (handles the case where stream_sender_charts_and_
159
+ // replication_reset() already cleared the flag during a disconnect
160
+ // racing with this push).
161
+ if(unlikely(rrdset_flag_check(st, RRDSET_FLAG_OBSOLETE) ||
162
+ !rrdhost_can_stream_metadata_to_parent(st->rrdhost))) {
163
+ if(we_caused_transition) {
164
+ RRDSET_FLAGS undo = rrdset_flag_set_and_clear(
165
+ st,
166
+ RRDSET_FLAG_SENDER_REPLICATION_FINISHED,
167
+ RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS);
168
+ if(undo & RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS) {
169
+ if(rrdhost_sender_replicating_charts_minus_one(st->rrdhost) == 0)
170
+ pulse_host_status(st->rrdhost, PULSE_HOST_STATUS_SND_RUNNING, 0);
171
+ }
172
+ }
173
+ }
174
+ else {
175
+ replication_progress = true;
176
+ }
177
+ }
178
179
#ifdef NETDATA_LOG_REPLICATION_REQUESTS
180
internal_error(true, "REPLAY: 'host:%s/chart:%s' replication starts",
src/streaming/stream-sender.c
+34
-7
@@ -81,8 +81,18 @@ void stream_sender_charts_and_replication_reset(struct sender_state *s) {
81
// reset the state of all charts
82
RRDSET *st;
83
rrdset_foreach_read(st, s->host) {
84
+ // Decrement only when this chart actually contributed +1 to the host
85
+ // counter, i.e. when IN_PROGRESS was set. The previous condition
86
+ // (!FINISHED) over-decremented initial-state charts (no flags set, no
87
+ // prior +1) and relied on a force-zero safety net below to compensate.
88
+ // Force-zero is unsafe against concurrent claim-before-publish in
89
+ // stream_sender_send_rrdset_definition: a sender that has just
90
+ // incremented but not yet published IN_PROGRESS would be desynced from
91
+ // the counter we forcibly cleared. Use the precise condition instead.
92
+ // Pulse status is intentionally not flipped here -- the surrounding
93
+ // sender connect/disconnect lifecycle drives it (e.g. SND_DISCONNECTED).
94
RRDSET_FLAGS old = rrdset_flag_set_and_clear(st, RRDSET_FLAG_SENDER_REPLICATION_FINISHED, RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS);
85
- if(!(old & RRDSET_FLAG_SENDER_REPLICATION_FINISHED))
95
+ if(old & RRDSET_FLAG_SENDER_REPLICATION_IN_PROGRESS)
96
rrdhost_sender_replicating_charts_minus_one(st->rrdhost);
97
98
#ifdef REPLICATION_TRACKING
@@ -100,13 +110,18 @@ void stream_sender_charts_and_replication_reset(struct sender_state *s) {
110
}
111
rrdset_foreach_done(st);
112
103
- if(rrdhost_sender_replicating_charts(s->host) != 0) {
113
+ // Observability only. The per-chart loop now precisely balances
114
+ // contributions; a non-zero residual either reflects a concurrent
115
+ // claim-before-publish in flight (will resolve) or a real accounting bug
116
+ // worth investigating. Do NOT force-zero: that would desynchronize the
117
+ // counter from any in-flight sender's not-yet-published IN_PROGRESS flag.
118
+ size_t residual = rrdhost_sender_replicating_charts(s->host);
119
+ if(residual != 0) {
120
nd_log(NDLS_DAEMON, NDLP_WARNING,
105
- "STREAM REPLAY ERROR: sender replicating instances counter should be zero, but it is %u"
106
- " - resetting it to zero",
107
- rrdhost_sender_replicating_charts(s->host));
108
-
109
- rrdhost_sender_replicating_charts_zero(s->host);
121
+ "STREAM REPLAY: sender replicating-charts counter is %zu after reset "
122
+ "(expected 0); leaving it untouched to preserve any concurrent "
123
+ "claim-before-publish in flight",
124
+ residual);
125
}
126
127
stream_sender_replicating_charts_zero(s);
@@ -165,6 +180,18 @@ void stream_sender_on_disconnect(struct sender_state *s) {
180
"STREAM SND '%s': running on-disconnect hooks...",
181
rrdhost_hostname(s->host));
182
183
+ // Stop new metadata pushes BEFORE the reset. New collectors that haven't
184
+ // yet entered stream_sender_send_rrdset_definition will fail the
185
+ // rrdhost_can_stream_metadata_to_parent() predicate and skip the
186
+ // bookkeeping entirely; in-flight collectors that already passed the
187
+ // predicate are caught by the post-CAS recheck in
188
+ // stream_sender_send_rrdset_definition (which then rolls back via atomic
189
+ // CAS, so the reset's per-chart accounting and the rollback do not
190
+ // double-decrement). The duplicate clear later in
191
+ // stream_sender_move_running_to_connector_or_remove_internal /
192
+ // stream_sender_remove is idempotent for atomic flag ops.
193
+ rrdhost_flag_clear(s->host, RRDHOST_FLAG_STREAM_SENDER_READY_4_METRICS);
194
+
195
stream_sender_on_connect_and_disconnect(s);
196
197
// update the child (the receiver side) for this parent