From 4c4ef33baa4c53f9289a01f210a06b7916b735a8 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:16:39 +0530 Subject: [PATCH] fix(metrics): export-only clamp for buffer statistics (#5467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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] --- lib/fluent/plugin/buffer.rb | 11 ++++++++-- test/plugin/test_buffer.rb | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/lib/fluent/plugin/buffer.rb b/lib/fluent/plugin/buffer.rb index ea50eb3ecb..a183362553 100644 --- a/lib/fluent/plugin/buffer.rb +++ b/lib/fluent/plugin/buffer.rb @@ -917,8 +917,15 @@ def write_step_by_step(metadata, data, format, splits_count, &block) ] def statistics - stage_size, queue_size = @stage_size_metrics.get, @queue_size_metrics.get - buffer_space = 1.0 - ((stage_size + queue_size * 1.0) / @total_limit_size) + # Export-only clamp: internal gauges may go transiently negative during + # the deferred stage_size add vs enqueue_chunk sub race (#5303, #2712). + # Clamping the gauge store itself would turn that into a permanent + # over-count and break Buffer#storable? -- keep raw gauge semantics. + stage_size = [@stage_size_metrics.get, 0].max + queue_size = [@queue_size_metrics.get, 0].max + denom = @total_limit_size.to_f + # denom > 0 already excludes 0/0 NaN; stage/queue are floored above. + buffer_space = denom > 0.0 ? (1.0 - (stage_size + queue_size).to_f / denom).clamp(0.0, 1.0) : 0.0 @stage_length_metrics.set(@stage.size) @queue_length_metrics.set(@queue.size) @available_buffer_space_ratios_metrics.set(buffer_space * 100) diff --git a/test/plugin/test_buffer.rb b/test/plugin/test_buffer.rb index 9cb803f2f7..230b5f08cf 100644 --- a/test/plugin/test_buffer.rb +++ b/test/plugin/test_buffer.rb @@ -1536,5 +1536,47 @@ def create_chunk_es(metadata, es) test 'returns available_buffer_space_ratios' do assert_equal 10.0, @p.statistics['buffer']['available_buffer_space_ratios'] end + + # Export-only clamp: internal gauges may be negative (self-correcting race + # or set_gauge); statistics must still publish non-negative sizes (#5303). + test 'exports non-negative stage/queue byte sizes when gauges are negative' do + @p.stage_size_metrics.set(-50) + @p.queue_size_metrics.set(-100) + + stats = @p.statistics['buffer'] + assert_equal 0, stats['stage_byte_size'] + assert_equal 0, stats['queue_byte_size'] + assert_equal 0, stats['total_queued_size'] + assert stats['total_queued_size'] >= 0 + # Negative gauges floor to 0 usage => full free space (100.0% with total_limit_size=1024) + assert_equal 100.0, stats['available_buffer_space_ratios'] + end + + test 'clamps available_buffer_space_ratios when usage exceeds total_limit_size' do + # Simulate counter overshoot past configured limit + @p.stage_size_metrics.set(2000) + @p.queue_size_metrics.set(2000) + + stats = @p.statistics['buffer'] + assert_equal 2000, stats['stage_byte_size'] + assert_equal 2000, stats['queue_byte_size'] + assert_equal 4000, stats['total_queued_size'] + assert stats['available_buffer_space_ratios'] >= 0.0 + assert stats['available_buffer_space_ratios'] <= 100.0 + assert_equal 0.0, stats['available_buffer_space_ratios'] + end + + test 'available_buffer_space_ratios is safe when total_limit_size is zero' do + # 0/0 would be NaN; Array#max/min on NaN raises — must not crash export. + @p.instance_variable_set(:@total_limit_size, 0) + @p.stage_size_metrics.set(0) + @p.queue_size_metrics.set(0) + stats = @p.statistics['buffer'] + assert_equal 0, stats['stage_byte_size'] + assert_equal 0, stats['queue_byte_size'] + assert stats['available_buffer_space_ratios'] >= 0.0 + assert stats['available_buffer_space_ratios'] <= 100.0 + refute stats['available_buffer_space_ratios'].to_f.nan? + end end end