-
Notifications
You must be signed in to change notification settings - Fork 1.9k
input: add thread.ring_buffer.retry_limit option #11394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
input: add thread.ring_buffer.retry_limit option #11394
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a per-input configurable retry limit for writing to the threaded input ring buffer: new field Changes
Sequence Diagram(s)(Skipped — changes are focused and do not introduce a multi-component sequential flow requiring visualization.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a configurable retry limit for threaded input plugins' ring buffer writes. Previously, the retry limit was hardcoded to 10 attempts (1 second total with 100ms sleep between retries). This caused data loss during temporary backpressure situations that lasted longer than 1 second.
Changes:
- Added
thread.ring_buffer.retry_limitconfiguration option with default value of 10 to maintain backward compatibility - Modified ring buffer write logic to use the configurable retry limit instead of hardcoded value
- Updated documentation to explain the new configuration option
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| include/fluent-bit/flb_input.h | Added ring_buffer_retry_limit field to flb_input_instance struct |
| src/flb_input.c | Added configuration map entry, initialization, and property parsing for the new retry limit option |
| src/flb_input_chunk.c | Removed hardcoded retry limit and updated to use instance-level configurable value |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else if (prop_key_check("thread.ring_buffer.retry_limit", k, len) == 0 && tmp) { | ||
| ret = atoi(tmp); | ||
| flb_sds_destroy(tmp); | ||
| if (ret <= 0) { | ||
| flb_error("[input] thread.ring_buffer.retry_limit must be greater than 0"); | ||
| return -1; | ||
| } | ||
| ins->ring_buffer_retry_limit = ret; | ||
| } |
Copilot
AI
Jan 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding an upper bound validation for the retry_limit to prevent potential issues with excessively large values.
With the 100ms sleep between retries, a very large retry_limit could cause the input thread to block for an extended period. For example, a value of 1,000,000 would result in approximately 27.7 hours of blocking. This could cause unexpected behavior or resource exhaustion.
A reasonable upper bound might be something like 36000 (1 hour of retries) or 600 (1 minute), with a warning message if the value exceeds a certain threshold. The appropriate limit should depend on your system's requirements and typical backpressure patterns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally chose not to enforce an upper bound because the primary use case for this option is environments where data loss is unacceptable (e.g., audit logs, financial transactions).
In such cases, users prefer the input thread to block and wait for backpressure to resolve rather than dropping data. The user explicitly configures a large value knowing the trade-off.
Add a new configuration option 'thread.ring_buffer.retry_limit' for threaded input plugins. This option controls the maximum number of retry attempts when the ring buffer is full before dropping data. The default value is 10 (maintaining backward compatibility). Fixes fluent#11393 Signed-off-by: jinyong.choi <inimax801@gmail.com>
Replace the hardcoded retry limit (10) with the configurable 'ring_buffer_retry_limit' value from the input instance. This allows users to increase retry attempts for handling temporary backpressure situations without dropping data. Signed-off-by: jinyong.choi <inimax801@gmail.com>
0e15cc1 to
dba41e4
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
When a threaded input plugin's ring buffer is full, the input thread retries writing to the buffer before dropping data. Previously, this retry limit was hardcoded to 10 (1 second total with 100ms sleep between retries).
This patch makes the retry limit configurable via the new 'thread.ring_buffer.retry_limit' option. The default value remains 10 for backward compatibility. Increasing this value allows the system to handle temporary backpressure situations without dropping data.
Fixes #11393
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Bug Fixes / Observability
✏️ Tip: You can customize this high-level summary in your review settings.