GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [connectors] Kafka tests: do not fail on transient transport errors. by blp · Pull Request #7849 · feldera/feldera

Conversation

@blp

@blp blp commented Nov 4, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Linked Issue

Closes #7641.

check_end_to_end, check_fault_tolerance, and the buffer test in crates/connectors/src/transport/kafka/ft/test.rs panicked on every controller error, also on non-fatal ones. librdkafka can report UnknownPartition once for a new topic or partition while its metadata spreads, and refine_kafka_error classifies that as non-fatal, so the endpoint retries. The tests now log non-fatal transport errors (new helper is_transient) and fail only on fatal ones. The output checks still catch lost data. See the analysis in the issue.

Describe Manual Test Plan

Ran cargo test -p static-connectors --test connector_suites -- transport::kafka::ft::test::random_end_to_end transport::kafka::ft::test::buffer against a local Redpanda: 5 passed. Ran ... -- transport::kafka::ft::test::ft_: 9 passed, including ft_with_topic_expiration and ft_with_tolerance_topic_expiration, whose rounds expect failure. The original flake depends on timing and does not reproduce on demand, so this checks only that the tests still pass.

Checklist

  • Unit tests added/updated
  • Integration tests added/updated
  • Documentation updated
  • Changelog updated

Breaking Changes?

None. Test-only change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@blp blp added connectors Issues related to the adapters/connectors crate QA Testing and quality assurance labels Nov 4, 2026
@blp blp self-assigned this Nov 4, 2026
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 4, 2026
View reviewed changes
Comment thread crates/connectors/src/transport/kafka/ft/test.rs

/// Returns true if `error` is a non-fatal transport error, such as `UnknownPartition` while
/// a new topic's metadata spreads across the brokers. The endpoint retries these itself.
fn is_transient(error: &ControllerError) -> bool {

@mythical-fred-oss mythical-fred-oss Bot Nov 4, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

pitfall (medium): check_fault_tolerance (~L2178) still uses Box::new(move |error, _| panic!("error: {error}")). It creates fresh topics with KafkaResources::create_topics, and its with_adding_partition_afterward step adds partitions while the pipeline runs. That is the same stale-metadata window, so it can hit the same non-fatal UnknownPartition flake. Should it also use is_transient? (The with_failure_expected rounds likely rely on fatal errors only, so the filter should still work for them.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@blp blp Nov 5, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Reply from Claude (AI assistant), on behalf of Ben.

Agreed. check_fault_tolerance now uses is_transient too (squashed into the single commit, 4c0baea). The rounds that expect failure still fail: without resume_earliest_if_data_expires, a missing checkpoint offset ends the poller with a fatal error, and the check is only that the records never arrive. All 9 ft_* tests pass against a local Redpanda.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Review summary. This change only touches tests. The diagnosis is right: refine_kafka_error marks UnknownPartition as non-fatal, and consumer.error(fatal, …) turns it into InputTransportError { fatal: false }, so is_transient matches it.

Check Result
cargo check --locked -p static-connectors --tests ✅ pass
rustfmt on test.rs ✅ clean
Kafka tests ⏭️ not run (no broker in this environment)
feldera-pr-checks gates (flaky tests, tests, docs, deps, unsafe, CI) ✅ none failed; the change removes a flake
  • pitfall (medium): check_fault_tolerance still panics on every error, which leaves the same flake possible there. Details are in the inline comment.

Apart from that, LGTM.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@blp
[connectors] Kafka tests: do not fail on transient transport errors.
4c0baea 
check_end_to_end, check_fault_tolerance, and the buffer test panicked
on every error that the controller reported, also on non-fatal ones.
librdkafka can report UnknownPartition once for a new topic or a new
partition, while its metadata spreads to the brokers, and the endpoint
retries it.  The tests now log non-fatal transport errors and fail only
on fatal ones.  The output checks still catch lost data.

Fixes #7641.

Signed-off-by: Ben Pfaff <[email protected]>
@blp
blp force-pushed the kafka-unknown-partition-flake branch from 9144d44 to 4c0baea Compare November 5, 2026 00:06
mihaibudiu
mihaibudiu approved these changes Nov 5, 2026
View reviewed changes
@blp
blp added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 2c7a1a7 Nov 5, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.
@blp
blp deleted the kafka-unknown-partition-flake branch November 5, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@blp blp

Labels

connectors Issues related to the adapters/connectors crate QA Testing and quality assurance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flaky Kafka FT test: random_end_to_end_json_array_small hits UnknownPartition under CI load

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@blp @mihaibudiu
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.