input: private plugins must use global event loop#10922
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
WalkthroughAdjusts collector event loop selection in non-threaded inputs to prefer a thread-local engine event loop when appropriate. Adds an internal test validating that private inputs’ collectors bind to the main config’s event loop. No public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Cfg as Config
participant Eng as Engine
participant In as Input Instance
participant ELMain as EventLoop (config->evl)
participant ELTLS as EventLoop (tls_evl)
Note over In,Eng: Non-threaded input collector creation
In->>Eng: flb_engine_evl_get()
Eng-->>In: tls_evl (may be null)
alt tls_evl exists AND tls_evl != config->evl AND input not private
In->>ELTLS: register collector on tls_evl
Note right of ELTLS: Changed path: bind to thread-local loop
else
In->>ELMain: register collector on config->evl
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/flb_input.c (1)
2019-2031: Bug: destroy path uses config->evl instead of the collector’s evlcollector_start/pause/resume consistently use coll->evl, but flb_input_collector_destroy uses config->evl. With this PR, non-threaded, non‑private collectors may bind to a TLS event loop distinct from config->evl, so destroy can target the wrong loop (risk: failure to unregister, fd leak, or backend epoll/kqueue misuse).
Fix by using coll->evl consistently:
int flb_input_collector_destroy(struct flb_input_collector *coll) { - struct flb_config *config = coll->instance->config; - if (coll->type == FLB_COLLECT_TIME) { if (coll->fd_timer > 0) { - mk_event_timeout_destroy(config->evl, &coll->event); + mk_event_timeout_destroy(coll->evl, &coll->event); } } else { - mk_event_del(config->evl, &coll->event); + mk_event_del(coll->evl, &coll->event); } flb_free(coll); return 0; }
🧹 Nitpick comments (1)
src/flb_input.c (1)
1654-1661: Comment tweak (optional): clarify the exception explicitlyConsider stating “Private inputs (FLB_INPUT_PRIVATE) must always bind to the main engine event loop (config->evl)” to mirror the condition below verbatim.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/flb_input.c(1 hunks)tests/internal/processor.c(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
PR: fluent/fluent-bit#10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
tests/internal/processor.c
🧬 Code graph analysis (2)
src/flb_input.c (1)
src/flb_engine.c (1)
flb_engine_evl_get(89-95)
tests/internal/processor.c (7)
src/flb_lib.c (1)
flb_init_env(123-136)src/flb_config.c (2)
flb_config_init(225-438)flb_config_exit(440-617)lib/monkey/mk_core/mk_event.c (2)
mk_event_loop_create(61-88)mk_event_loop_destroy(91-96)src/flb_scheduler.c (3)
flb_sched_create(737-809)flb_sched_ctx_set(50-53)flb_sched_destroy(812-854)src/flb_storage.c (2)
flb_storage_create(632-738)flb_storage_destroy(750-771)src/flb_engine.c (1)
flb_engine_evl_set(97-100)src/flb_input.c (4)
flb_input_new(223-448)flb_input_instance_init(1091-1429)flb_input_instance_exit(1520-1536)flb_input_instance_destroy(799-964)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (4)
src/flb_input.c (1)
1652-1670: Private helpers now pinned to main loop — good fixCondition correctly forces FLB_INPUT_PRIVATE inputs (e.g., emitter) to use config->evl while still honoring TLS-local evl for non-private cases. This aligns with the CPU spin regression described.
tests/internal/processor.c (3)
25-31: Includes look right for constructing engine primitives in-testPulling in engine, scheduler, event loop, bucket queue, storage, and input headers is appropriate for this scenario.
127-186: Solid regression test: private emitter binds to main evlTest accurately simulates an input-thread TLS evl and verifies emitter collectors use config->evl. Nice.
Two small cleanups to consider:
- Confirm whether flb_config_exit() frees config->evl_bktq; if not, destroy it explicitly to avoid leaks:
@@ - flb_storage_destroy(config); - flb_config_exit(config); + flb_storage_destroy(config); + /* If not owned by flb_config_exit(), destroy explicitly */ + flb_bucket_queue_destroy(config->evl_bktq); + flb_config_exit(config);
- After tearing down, clear scheduler TLS to avoid dangling TLS pointers post-config_exit (mirrors flb_engine_evl_set(NULL)):
@@ - flb_storage_destroy(config); - flb_config_exit(config); + flb_storage_destroy(config); + flb_sched_ctx_set(NULL); + flb_config_exit(config);If you want, I can add a companion test asserting that a non‑private input created under a TLS evl binds its collectors to that TLS loop (guarded to avoid flakiness).
189-189: Test registration LGTMThe new test is properly registered in TEST_LIST.
|
Hi, It seems that this PR has broken flb-it-processor test on Windows - refer to https://github.com/fluent/fluent-bit/actions/runs/17930705179/job/50988224672 (I have the same on my local environment):
I failed to reproduce same issue (flb-it-processor test passes successfully) on my local Windows environment on e6bc684 commit (part of master branch) which is right before this PR. Update by 9/25/2025: this issue was fixed in #10930 |
3 participants
Fixes #10529
Forces the helper that instances private input plugins (emitter) to bind their collectors back to the main engine loop instead of the thread loop.
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
Bug Fixes
Tests