kafka: improve stability when creating many topics with Kafka v4 - #6081
kafka: improve stability when creating many topics with Kafka v4#60813AceShowHand wants to merge 8 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesKafka topic management
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change improves Kafka topic metadata error handling and retry behavior without identified merge-blocking risk; it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant TopicManager
participant KafkaAdmin
participant TopicCache
TopicManager->>KafkaAdmin: Create topic
loop Until metadata is visible
TopicManager->>KafkaAdmin: Get topic metadata
KafkaAdmin-->>TopicManager: Metadata or retryable error
end
TopicManager->>TopicCache: Cache partition count
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes address issue ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
/test all |
There was a problem hiding this comment.
Pull request overview
This PR improves Kafka topic-creation robustness in TiCDC’s Kafka sink by treating newly-created topics as potentially not yet visible in metadata, retrying transient Kafka metadata errors, and only caching a topic after Kafka confirms it is visible.
Changes:
- Update
GetTopicsMetato return topic-level errors (includingUnknownTopicOrPartition) when topic errors are not ignored, while preserving the previous “best-effort discovery” behavior when errors are ignored. - Introduce
IsRetryableTopicMetadataErrorto classify transient Kafka metadata errors as retryable and use it to drive bounded retries when waiting for topic visibility. - Ensure the topic manager’s cache is only updated after the topic becomes visible (and add/adjust unit tests to validate caching + retry behavior).
Validation:
- Not run here (code review only; no CI execution available in this environment).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/sink/kafka/admin.go | Makes topic metadata error handling stricter when not ignoring topic errors; adds retryable-metadata error classifier. |
| pkg/sink/kafka/sarama_admin_test.go | Adds unit coverage for the stricter unknown-topic error behavior and for retryable metadata error classification. |
| downstreamadapter/sink/topicmanager/kafka_topic_manager.go | Adds retryable-error classification to topic visibility wait; caches topic only after visibility. |
| downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go | Adds tests ensuring cache isn’t written before visibility and retry behavior honors cancellation / stops on non-retryable errors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/hold |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lidezhu, wk989898 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: close #6076
What is changed and how it works?
GetTopicsMetareturn topic-level errors, includingUnknownTopicOrPartition, whenignoreTopicErrorisfalse.ignoreTopicErroristrue.Check List
Tests
GOTOOLCHAIN=go1.25.12 GOMAXPROCS=2 go test -p 1 -vet=off ./pkg/sink/kafka ./downstreamadapter/sink/topicmanager -count=1Questions
Will it cause performance regression or break compatibility?
No public API, configuration, or Kafka protocol behavior changes. Metadata retries remain bounded by the existing six-attempt limit, and deterministic errors stop after the first attempt.
Do you need to update user documentation, design documentation or monitoring documentation?
No.
Release note
Summary by CodeRabbit