Report ALL_BROKERS_DOWN only once per outage with metadata.recovery.strategy=rebootstrap - #5547
Open
Marko Lahma (lahma) wants to merge 1 commit into
Conversation
…trategy=rebootstrap With metadata.recovery.strategy=rebootstrap (default since v2.10.0), the re-bootstrap sequence triggered when all brokers are down resets each broker's down-reported state so that all brokers are re-tried before the all-brokers-down condition can be reached again (confluentinc#5126). During a sustained outage every broker keeps failing, so the 'Only 0 -> 1' guard in rd_kafka_broker_set_state() re-arms and ERR__ALL_BROKERS_DOWN is reported to the application on every re-bootstrap cycle, roughly every reconnect interval, instead of once per outage. Add an rk_all_brokers_down_reported latch: the error is reported only on its 0 -> 1 transition and subsequent cycles of the same outage log a debug message instead. The latch is cleared when any non-logical broker enters the UP state, so a new outage after a recovery is reported again. Re-bootstrap behavior and the confluentinc#5126 down-reported reset semantics are unchanged; only the application-visible event is de-duplicated. Add mock-based regression tests to 0152: single report during a sustained ApiVersion-handshake outage while re-bootstrap keeps cycling, re-arming across two outages, and reconnect backoff growing to its cap pacing the connection attempts. Fixes confluentinc#5546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
This PR de-duplicates application-visible RD_KAFKA_RESP_ERR__ALL_BROKERS_DOWN error events during sustained outages when metadata.recovery.strategy=rebootstrap is enabled, while keeping the existing re-bootstrap behavior unchanged.
Changes:
- Add a client-level latch (
rk_all_brokers_down_reported) to reportALL_BROKERS_DOWNonly once per outage and re-arm it when any non-logical broker becomes UP. - Update broker state handling to emit a debug log (instead of re-raising the error) on subsequent all-down cycles within the same outage.
- Add mock-cluster regression tests covering sustained outage de-duplication, re-arming after recovery, and reconnect backoff growth behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/0152-rebootstrap.c | Adds mock-cluster subtests and callbacks to verify ALL_BROKERS_DOWN is emitted once per outage and backoff continues to grow during sustained failure. |
| src/rdkafka.c | Initializes the new rk_all_brokers_down_reported latch in rd_kafka_new(). |
| src/rdkafka_int.h | Defines and documents the new rk_all_brokers_down_reported atomic latch in struct rd_kafka_s. |
| src/rdkafka_broker.c | Gates ALL_BROKERS_DOWN emission behind the new latch and clears it when any non-logical broker transitions to UP. |
| CHANGELOG.md | Documents the behavioral change and links it to #5546 / regression since #5126. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5546 (first part: repeated
ALL_BROKERS_DOWNevents).Problem
With
metadata.recovery.strategy=rebootstrap(the default since v2.10.0 / KIP-899), a clientwhose brokers are all unreachable for a sustained period reports
RD_KAFKA_RESP_ERR__ALL_BROKERS_DOWNto the application on every re-bootstrap cycle(roughly every reconnect interval) instead of once per outage.
Mechanism:
rd_kafka_broker_set_state()raises the error under the/* Only 0 -> 1 */rkb_down_reportedguard when the last non-logical broker goes down, and starts are-bootstrap in the same block (
src/rdkafka_broker.c).rd_kafka_reset_any_broker_down_reported(), whichresets every broker's
rkb_down_reportedso that all brokers are re-tried before theall-down condition can be reached again (intent of Avoid returning an all brokers down error on planned disconnections #5126).
re-fires on every cycle. Nothing rate-limits this
(
metadata.recovery.rebootstrap.trigger.msonly applies to the separate metadata-timeouttrigger). The Avoid returning an all brokers down error on planned disconnections #5126 commit message acknowledges this side effect ("after retrying on
re-bootstrap a new ALL_BROKERS_DOWN error is issued").
Reproduced with a mock cluster closing every connection during ApiVersion negotiation:
17 error events in 10 seconds on an idle consumer.
Fix
Add an
rk_all_brokers_down_reportedlatch:transitions 0 → 1; subsequent cycles of the same outage log a
BROKERdebug messageinstead.
recovery followed by a new outage reports the error again.
Avoid returning an all brokers down error on planned disconnections #5126 semantics (reset
rkb_down_reportedso all brokers are re-tried) are untouched —only the application-visible event is de-duplicated.
Tests
Three new mock-cluster subtests in
tests/0152-rebootstrap.c(local, no real clusterneeded), using ApiVersion +
RD_KAFKA_RESP_ERR__TRANSPORTrequest-error injection to makethe mock broker close every connection during the handshake (the exact failure mode from
#5546):
ALL_BROKERS_DOWNerror whilere-bootstrap keeps cycling (≥ 2 sequences; observed ~19). Fails with 17 errors
before the fix.
until capped by
reconnect.backoff.max.ms, pacing connection attempts (3-6 attempts in10 s with 1000/5000 ms settings) — guards the investigated-and-disproved hypothesis from
Repeated ALL_BROKERS_DOWN error on every re-bootstrap cycle during sustained outage (metadata.recovery.strategy=rebootstrap) #5546 that re-bootstrap resets the reconnect backoff.
0000(unit tests),0095,0151and the local quick suite pass.Note: the error de-duplication relies on
rd_atomic32_set()returning the previous value,which is broken in CMake builds before #5136 (shipped in v2.15.0) — this PR targets master
where that fix is present.