Add evacuate_limit_size to bound chunk evacuation - #5473
Conversation
When an output stays unreachable past the retry limit, queued chunks get evacuated to the backup dir and can fill up the disk. evacuate_limit_size caps the total evacuated size: once the budget is used up the remaining chunks are purged without evacuating, and setting it to 0 turns evacuation off entirely. Default stays nil (no limit) so existing behavior doesn't change. Signed-off-by: Chris (ChrisJr404) <11917633+ChrisJr404@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new buffer configuration option to cap (or disable) chunk evacuation during Buffer#clear_queue!, preventing unbounded disk growth when an output stays down and the retry limit is reached (Fixes #5352).
Changes:
- Introduces
evacuate_limit_size(nil= unbounded/current behavior,0= disable evacuation, positive = cap total evacuated bytes perclear_queue!pass). - Updates
Buffer#clear_queue!to conditionally evacuate queued chunks based on the configured limit. - Adds unit tests covering default behavior, disabled evacuation (
0), and bounded evacuation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/fluent/plugin/buffer.rb | Adds evacuate_limit_size config and enforces it during queue clearing/evacuation. |
| test/plugin/test_buffer.rb | Adds tests validating evacuation behavior under different evacuate_limit_size settings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if evacuate_chunk?(q.bytesize, evacuated_size) | ||
| evacuate_chunk(q) | ||
| evacuated_size += q.bytesize | ||
| end |
| evacuate_chunk(q) | ||
| evacuated_size += q.bytesize |
There was a problem hiding this comment.
In clear_queue!, evacuated_size += q.bytesize runs unconditionally after evacuate_chunk(q), without checking whether the backup actually succeeded. Since evacuate_chunk in both buf_file.rb and buf_file_single.rb rescues its own exceptions instead of re-raising, a failed backup copy (e.g. ENOSPC, permission error) still counts against the budget.
Worth confirming success (or having evacuate_chunk report bytes actually written) before counting a chunk toward evacuated_size.
| log.on_trace { log.trace "clearing queue", instance: self.object_id } | ||
|
|
||
| synchronize do | ||
| evacuated_size = 0 |
There was a problem hiding this comment.
[Nit] since the budget actually resets on every clear_queue! call rather than being a true cumulative cap, the name evacuate_limit_size and the desc's "total size limit" phrasing read as more absolute than what's implemented. Worth clarifying in the desc that this is per-clear_queue! call, not a lifetime/cumulative limit — otherwise the naming suggestion (e.g. something like evacuate_limit_size_per_clear) is up to you.
Which issue(s) this PR fixes:
Fixes #5352
What this PR does / why we need it:
Adds a
evacuate_limit_sizebuffer param so chunk evacuation can't fill up the disk when an output stays down.Right now, when the retry limit is hit and
clear_queue!runs, every queued chunk gets evacuated to the backup dir. If an output is unreachable for a long time that keeps piling files onto disk with no bound, which is exactly what #5352 ran into (evacuated chunks filledroot_dir).The new param caps the total size that gets evacuated in one
clear_queue!pass. Chunks are evacuated in queue order until the next one wouldn't fit in the remaining budget; after that the rest are purged without evacuating, so disk usage stays under the limit. Setting it to0turns evacuation off completely, and the default isnilwhich keeps the current unbounded behavior, so nobody's setup changes unless they opt in.The size accounting and the skip/disable decision live in the base
Bufferclass (clear_queue!plus a smallevacuate_chunk?helper), so it applies to every buffer type that overridesevacuate_chunk(file, file_single). While I was in that loop I also fixed the trace log line there, which referenced an undefinedchunkinstead ofq.Tests: added three cases to
test/plugin/test_buffer.rbcovering the default (all chunks evacuated),evacuate_limit_size 0(nothing evacuated, everything still purged), and a positive limit (evacuation stops once the budget is spent). Ranbundle exec ruby -Itest test/plugin/test_buffer.rb-> 72 tests, 420 assertions, 0 failures, 0 errors.Docs Changes:
The new
evacuate_limit_sizebuffer parameter should be documented in the buffer section of the docs. Happy to open a PR on fluentd-docs-gitbook.Release Note:
Add
evacuate_limit_sizebuffer parameter to bound (or disable, with0) the total size of chunks evacuated when the retry limit is reached. Defaultnilkeeps the current unbounded behavior.