Skip to content

Backport(v1.19): fix(buffer): decrement queue_size in ensure so failed purge does not leak (#5475) - #5487

Merged
Watson1978 merged 1 commit into
v1.19from
backport-to-v1.19/pr5475
Sep 7, 2026
Merged

Watson1978 merged 1 commit into
v1.19from
backport-to-v1.19/pr5475

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Which issue(s) this PR fixes:
Backport #5475
Fixes #5468

What this PR does / why we need it:
In Fluent::Plugin::Buffer#purge_chunk, @queue_size_metrics is only decremented inside begin/rescue. When chunk.purge raises (e.g. unlink/close failure on the buffer path), the error is logged and swallowed but the sub is skipped. The chunk is already removed from @dequeued, so it is never retried — the queued byte counter ratchets toward total_limit_size, making storable? permanently false and causing a spurious BufferOverflowError on a near-empty buffer.

This moves bytesize = chunk.bytesize above the begin and moves @queue_size_metrics.sub(bytesize) into an ensure block so the counter is always released exactly once per purge_chunk call, regardless of whether purge succeeds.

Why safe:

  • ensure runs exactly once per call → no double-decrement.
  • Every chunk reaching purge_chunk was previously enqueued (dequeue/emit path), so the sub is always balanced.
  • If chunk.purge raises, the chunk is lost on filesystem error (already a logged, separate failure); this change only preserves counter integrity so storable? no longer spuriously goes false.

Docs Changes:
None (behavior fix; inline comment added).

Release Note:

  • buffer: fix spurious BufferOverflowError caused by queue_size leaking when a chunk purge fails

…leak (#5475)

**Which issue(s) this PR fixes**:
Fixes #5468

**What this PR does / why we need it**:
In `Fluent::Plugin::Buffer#purge_chunk`, `@queue_size_metrics` is only
decremented inside `begin/rescue`. When `chunk.purge` raises (e.g.
unlink/close failure on the buffer path), the error is logged and
swallowed but the `sub` is skipped. The chunk is already removed from
`@dequeued`, so it is never retried — the queued byte counter ratchets
toward `total_limit_size`, making `storable?` permanently `false` and
causing a spurious `BufferOverflowError` on a near-empty buffer.

This moves `bytesize = chunk.bytesize` above the `begin` and moves
`@queue_size_metrics.sub(bytesize)` into an `ensure` block so the
counter is always released exactly once per `purge_chunk` call,
regardless of whether `purge` succeeds.

Why safe:
- `ensure` runs exactly once per call → no double-decrement.
- Every chunk reaching `purge_chunk` was previously enqueued
(dequeue/emit path), so the `sub` is always balanced.
- If `chunk.purge` raises, the chunk is lost on filesystem error
(already a logged, separate failure); this change only preserves counter
integrity so `storable?` no longer spuriously goes false.

**Docs Changes**:
None (behavior fix; inline comment added).

**Release Note**:
* buffer: fix spurious `BufferOverflowError` caused by `queue_size`
leaking when a chunk purge fails

---------

Signed-off-by: Mehrdad Biukian Naeini <mehrdadbiukian@gmail.com>
Co-authored-by: Mehrdad Biukian <mehrdad@example.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@Watson1978 Watson1978 added this to the v1.19.4 milestone Sep 6, 2026
@Watson1978
Watson1978 merged commit 16882ac into v1.19 Sep 7, 2026
36 of 37 checks passed
@Watson1978
Watson1978 deleted the backport-to-v1.19/pr5475 branch September 7, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants