Fix sender replication counter leak on obsolete charts - #22428
Merged
stelfrag merged 2 commits intoMay 6, 2026
Conversation
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.
stelfrag
marked this pull request as ready for review
May 6, 2026 12:01
stelfrag
marked this pull request as draft
May 6, 2026 12:01
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes sender-side replication accounting so that obsolete charts don’t permanently pin rrdhost_sender_replicating_charts above zero, which would otherwise prevent cleanup of obsoleted charts on streaming children.
Changes:
- Tightens the disconnect/reset path to only decrement the host “replicating charts” counter when a chart actually had
SENDER_REPLICATION_IN_PROGRESS, and removes the previous force-zero behavior. - Skips entering sender replication bookkeeping for charts already marked
RRDSET_FLAG_OBSOLETE, preventing counter leaks for charts the receiver won’t replicate. - Ensures obsolete-marking proactively releases any pending sender replication slot (clears
IN_PROGRESS, decrements host counter, updates pulse status on 0 boundary).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/streaming/stream-sender.c |
Makes per-chart reset decrement logic precise (keyed on IN_PROGRESS) and avoids force-zeroing the host counter; clears sender readiness earlier on disconnect. |
src/streaming/protocol/command-chart-definition.c |
Avoids replication bookkeeping for obsolete charts and adds claim/rollback logic to keep host counter balanced under races. |
src/database/rrdset.c |
When a chart becomes obsolete, releases any pending sender replication slot immediately to avoid pinning the host replication counter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
stelfrag
marked this pull request as ready for review
May 6, 2026 12:12
thiagoftsm
approved these changes
May 6, 2026
thiagoftsm
left a comment
Contributor
There was a problem hiding this comment.
Streaming is working as expected locally. LGTM!
|
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
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. (cherry picked from commit 177a3b6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rrdhost_sender_replicating_chartson streaming children that would prevent cleanup of an obsoleted chartSummary by cubic
Fixes a sender-side leak of the replication counter for obsolete charts, unblocking obsolete-chart cleanup and preventing memory growth on streaming children. Obsolete chart definitions no longer pin
rrdhost_sender_replicating_chartsabove zero.stream_sender_send_rrdset_definition: skip replication bookkeeping for obsolete charts; increment the host counter before settingIN_PROGRESS; roll back if another sender already set it; undo if the chart becomes obsolete or the host loses metadata readiness.IN_PROGRESSwas set; remove force-zero; log non-zero residuals without altering the counter.READY_4_METRICSbefore reset to block new pushes racing with the reset.Written for commit 470a929. Summary will update on new commits.