engine: http_server: Add on-demand flush mechanism - #12192
Conversation
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
📝 WalkthroughWalkthroughThe change adds an engine API for on-demand flushing, tracks completed flush requests, and exposes ChangesOn-demand flush
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant APIv2Flush
participant EngineManager
participant flb_config
Client->>APIv2Flush: POST or PUT /api/v2/flush
APIv2Flush->>EngineManager: flb_engine_flush_request()
EngineManager->>flb_config: flush pending input tasks
EngineManager->>flb_config: increment flush_now_count
APIv2Flush-->>Client: JSON status and counter
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51c9b4e4ad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@include/fluent-bit/flb_config.h`:
- Around line 73-77: Make flush_now_count in struct flb_config an atomic counter
and use matching atomic operations for cross-thread access: replace the
increment in src/flb_engine.c lines 708-714, and use atomic loads for baseline,
the wait_for_flush_ack loop, and the final response value in
src/http_server/api/v2/flush.c lines 33-112; update the declaration in
include/fluent-bit/flb_config.h lines 73-77 accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 319c340c-05f0-47d0-b9c9-ef0fcdffbcaf
📒 Files selected for processing (10)
include/fluent-bit/flb_config.hinclude/fluent-bit/flb_engine.hinclude/fluent-bit/flb_engine_macros.hsrc/flb_engine.csrc/http_server/api/v2/CMakeLists.txtsrc/http_server/api/v2/flush.csrc/http_server/api/v2/flush.hsrc/http_server/api/v2/register.ctests/runtime/CMakeLists.txttests/runtime/core_engine_flush_now.c
The `flush_now_count` variable is accessed across threads (engine + HTTP server) and needs proper safety. Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
… document engine behavior Validated the page against the implementation in fluent/fluent-bit#12192 and corrected four inaccuracies: - The timeout example reported flush_now_count as 0 and described the counter as unchanged, contradicting the process-wide semantics described directly above it. The 503 path packs the current global counter, which reflects other flushes. Also clarify that a 503 is a missed acknowledgement, not a cancellation: the request stays queued on the manager channel and can still be processed afterwards. - Document the previously unmentioned 500 responses. A failed dispatch to the engine triggers no flush and leaves the counter alone, while a response encoding failure occurs after the flush was already requested, so the counter can still advance. Both return an empty body rather than JSON. - Replace "must specify an empty request body as -d '{}'" with an accurate description. The handler dispatches on the request method and never reads the body, and the HTTP/1 parser accepts a bodyless POST, so no payload is required. Use an explicit http:// URL. - Add a section describing what a flush does. Pending retries are invalidated and rescheduled to run immediately before buffered chunks are dispatched, so chunks in retry backoff are sent without waiting out their timer. Signed-off-by: Eric D. Schabell <eric@schabell.org>
|
Thanks for working on this. The endpoint is a useful building block for event-driven environments, including Lambda, but it does not yet provide a deterministic drain guarantee. A successful I suggest:
For the Lambda lifecycle specifically, Fluent Bit should remain alive between invocations because Lambda normally freezes and later reuses the environment. On an actual Finally, the PR is not currently merge-ready because Commit Prefix Lint rejects commit Overall, I support this as a generic on-demand flush/dispatch primitive, but it should not be considered sufficient to close the Lambda-support issue without the lifecycle adapter and a reliable completion mechanism. Plus, can we add integration tests for confirming this behavior? It would be nice to have to test this feature. |
Adds a
/api/v2/flushroute to the HTTP server to handle an on-demand flush request. This would be required for specific cases such as event-driven environments.The route supports
POSTandPUTto initiate a flush, as well asGETto retrieve the number of flushes that have been issued so far. Currently an on-demand flush forces chunks in a backoff to be retried regardless of their timer. This may not be appropriate in every cases and could be addressed with a URL parameter if needed.This PR adds a feature in itself, but is a building block to make fluent-bit working with AWS Lambda extensions, as outlined in #12191.
Following the configuration + valgrind run showcasing the feature:
In another terminal issuing flush commands:
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
fluent/fluent-bit-docs#2642
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
Tests