Backport(v1.19): fix(metrics): export-only clamp for buffer statistics (#5467) - #5488
Merged
Merged
Conversation
**Which issue(s) this PR fixes**: Fixes #5303 **What this PR does / why we need it**: Buffer size gauges (`stage_byte_size`, `queue_byte_size`, and derived `total_queued_size`) can go transiently negative when Fluentd core under/over-subtracts during concurrent stage/queue transitions (the deferred `@stage_size_metrics.add` after chunk unlock vs `enqueue_chunk`'s `sub` — see #2712 / #2734). Those values are mirrored by the Prometheus plugin as `fluentd_output_status_buffer_total_bytes` / `fluentd_output_status_buffer_stage_byte_size`, which is what #5303 reports. Clamping the **gauge store** on `sub`/`dec` is the wrong fix: it turns a self-correcting transient negative into a **permanent over-count**, so `Buffer#storable?` (which reads the raw gauge) eventually refuses every write. Thanks @Watson1978 for catching that. This PR takes an **export-only** approach: 1. Leave `LocalMetrics` gauge `sub`/`dec`/`set` semantics unchanged (negatives still allowed so the deferred-add race can self-heal). 2. Clamp stage/queue sizes to `>= 0` only when building `statistics` (the path Prometheus and the monitor agent consume). 3. Clamp `available_buffer_space_ratios` to `[0, 100]` when counters overshoot `total_limit_size`, and treat `total_limit_size == 0` as 0% free without dividing (no NaN / no dead NaN guard). **Docs Changes**: None **Release Note**: * buffer: clamp exported buffer size metrics to non-negative values **General Checklist**: - [x] I have read and followed [CONTRIBUTING.md](https://github.com/fluent/fluentd/blob/master/CONTRIBUTING.md) - [x] tests pass (CI) - [x] DCO signed-off **Tests**: - `test/plugin/test_buffer.rb` `#statistics`: negative underlying gauges export as 0; overshoot clamps ratio to 0; `total_limit_size == 0` does not raise and ratio stays finite. --------- Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Watson1978
approved these changes
Sep 7, 2026
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.
Which issue(s) this PR fixes:
Backport #5467
Fixes #5303
What this PR does / why we need it:
Buffer size gauges (
stage_byte_size,queue_byte_size, and derivedtotal_queued_size) can go transiently negative when Fluentd core under/over-subtracts during concurrent stage/queue transitions (the deferred@stage_size_metrics.addafter chunk unlock vsenqueue_chunk'ssub— see #2712 / #2734). Those values are mirrored by the Prometheus plugin asfluentd_output_status_buffer_total_bytes/fluentd_output_status_buffer_stage_byte_size, which is what #5303 reports.Clamping the gauge store on
sub/decis the wrong fix: it turns a self-correcting transient negative into a permanent over-count, soBuffer#storable?(which reads the raw gauge) eventually refuses every write. Thanks @Watson1978 for catching that.This PR takes an export-only approach:
LocalMetricsgaugesub/dec/setsemantics unchanged (negatives still allowed so the deferred-add race can self-heal).>= 0only when buildingstatistics(the path Prometheus and the monitor agent consume).available_buffer_space_ratiosto[0, 100]when counters overshoottotal_limit_size, and treattotal_limit_size == 0as 0% free without dividing (no NaN / no dead NaN guard).Docs Changes:
None
Release Note:
General Checklist:
Tests:
test/plugin/test_buffer.rb#statistics: negative underlying gauges export as 0; overshoot clamps ratio to 0;total_limit_size == 0does not raise and ratio stays finite.