handle PreInit error case for runReady.Close#51911
Conversation
🤖 GitHub commentsJust comment with:
|
TL;DRThe Buildkite failure is a formatting check failure, not a Filebeat runtime or test failure. Remediation
Investigation detailsRoot CauseThe PR adds imports in Evidence
Verification
What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
|
Tick the box to add this pull request to the merge queue (same as
|
|
@Mergifyio backport 9.5 9.4 9.3 8.19 |
✅ Backports have been createdDetails
Cherry-pick of 397822f has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Cherry-pick of 397822f has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Cherry-pick of 397822f has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Cherry-pick of 397822f has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
…51929) * handle PreInit error case for runReady.Close (#51911) * handle PreInit error case for runReady.Close * fix fmt issue (cherry picked from commit 397822f) # Conflicts: # filebeat/beater/stop_test.go * fix the merge conflict --------- Co-authored-by: Lee E Hinman <57081003+leehinman@users.noreply.github.com> Co-authored-by: Lee E. Hinman <lee.e.hinman@elastic.co>
Proposed commit message
WHAT
filebeat/beater: close runReady on Manager.PreInit failure
PR #51864 (commit 00068f7) reordered the defers in Filebeat.Run so that defer fb.runReady.Close() runs before the managerEarlyStop defer (correct LIFO ordering to avoid a 5-second wait when the manager's stop callback calls StopWithContext). However, both defers were placed after the b.Manager.PreInit() call. If PreInit returns an error, Run exits before either defer is registered, leaving runReady.ch permanently unclosed.
An unclosed runReady.ch means any subsequent call to StopWithContext (e.g. via a SIGINT-triggered Manager.Stop that invokes the beat stop callback) will wait the full 5-second timeout before proceeding, rather than returning immediately.
WHY
Restores the invariant from before #51864: runReady is closed regardless of how Run exits. The fix adds an explicit fb.runReady.Close() on the PreInit error return path, which is the one path the deferred close cannot reach. A new unit test TestRunClosesRunReadyOnPreInitFailure directly asserts the channel is closed after Run returns due to a PreInit error; confirmed to fail against the buggy code and pass with the fix.
Checklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Disruptive User Impact
None. This is an internal shutdown-sequencing fix. The only observable change is that Elastic Agent-managed Filebeat instances where PreInit fails will no longer incur a spurious 5-second delay during shutdown.
How to test this PR locally
Related issues
Use cases
Screenshots
Logs