Skip to content

[adapters] Tolerate transient Kafka errors in FT input tests#6586

Closed
ryzhyk wants to merge 1 commit into
mainfrom
fix-kafka-ft-test-transient-errors
Closed

[adapters] Tolerate transient Kafka errors in FT input tests#6586
ryzhyk wants to merge 1 commit into
mainfrom
fix-kafka-ft-test-transient-errors

Conversation

@ryzhyk

@ryzhyk ryzhyk commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Problem

transport::kafka::ft::test::multiple_input failed in the merge queue (run 28634798524) and kicked out an unrelated PR (#6584):

assertion `left == right` failed
  left: [Extended { num_records: 90, metadata: Object {"offsets": [{"start": 60, "end": 150}]} }]
 right: [Error(false)]

The job log shows the cause: librdkafka reported a momentary connectivity blip ("1/2 brokers are down", "AllBrokersDown"). refine_kafka_error classifies such errors as non-fatal, and the connector recovered on its own: the expected Extended { num_records: 90 } call arrived 100 ms after the panic.

The bug is in the test harness: DummyInputConsumer::error records every error, fatal or not, in the strict call sequence that expect() checks, so a transient blip anywhere in test_input or test_input_partition fails the test.

Fix

Record only fatal errors; log non-fatal ones. No test expects ConsumerCall::Error, so no expectation changes. This matches the MockInputConsumer-based Kafka tests, which install on_error(|_, _| {}). Same flake class as #1830.

Validation

Reproduced the blip locally by bouncing a dedicated Redpanda broker mid-test (docker restart -t 1), which produces the exact CI error signature (Message consumption error: AllBrokersDown ..., N/M brokers are down):

Run Code Broker bounce Result
baseline fixed no ok
blip tolerance fixed yes ok, 2 non-fatal errors received and ignored
repro fix reverted yes FAILED: right: [Error(false)], same as CI
regression fixed no full kafka::ft::test module: 39 passed, 0 failed

🤖 Generated with Claude Code

Transient librdkafka connectivity blips ("AllBrokersDown", "N/M brokers
are down") are reported to the consumer as non-fatal errors, and the
connector recovers on its own. DummyInputConsumer recorded every such
error in the strict call sequence that expect() checks, so a blip
anywhere in a test failed it, e.g.:

  thread 'transport::kafka::ft::test::multiple_input' panicked at
  crates/adapters/src/transport/kafka/ft/test.rs:463:22:
  assertion `left == right` failed
    left: [Extended { num_records: 90, ... }]
   right: [Error(false)]

Record only fatal errors. No test expects ConsumerCall::Error, so no
expectation changes.

Validated by bouncing the broker mid-test to inject the blip: with this
change multiple_input rides out the errors; without it, the test
reproduces the CI failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. Right fix: non-fatal errors are advisory and should not enter the strict call-sequence expectation. Matches the MockInputConsumer convention (on_error(|,| {})) and the validation table nicely isolates the flake with a broker bounce. info! on the non-fatal path keeps the signal in logs.

@ryzhyk ryzhyk closed this Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants