Skip to content

control-plane: avoid duplicate CreateTopics requests for existing topics - #4654

Open
jijo-OO7 wants to merge 2 commits into
knative-extensions:mainfrom
jijo-OO7:fix/avoid-duplicate-createtopics
Open

control-plane: avoid duplicate CreateTopics requests for existing topics#4654
jijo-OO7 wants to merge 2 commits into
knative-extensions:mainfrom
jijo-OO7:fix/avoid-duplicate-createtopics

Conversation

@jijo-OO7

Copy link
Copy Markdown
Contributor

Fixes #4406

Proposed Changes

  • Make CreateTopicIfDoesntExist idempotent by checking Kafka metadata before creating topics.
  • Avoid repeated CreateTopics requests and error responses during steady-state reconciliation.
  • Update unit tests and mocks to reflect the new DescribeTopics first behavior.
  • This change avoids repeated mutating CreateTopics requests by checking topic existence via Kafka metadata before creating.
  • While this still involves a read call to Kafka, it removes unnecessary write requests and error responses in steady state reconciliation.

@knative-prow

knative-prow Bot commented Feb 26, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jijo-OO7
Once this PR has been reviewed and has the lgtm label, please assign leo6leo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 26, 2026
@knative-prow
knative-prow Bot requested review from Cali0707 and aliok February 26, 2026 08:26
@jijo-OO7 jijo-OO7 changed the title control-plane: idempotent CreateTopicIfDosentExist control-plane: avoid duplicate CreateTopics requests for existing topics Feb 26, 2026
@codecov

codecov Bot commented Feb 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.19%. Comparing base (38083e7) to head (e5a1756).
⚠️ Report is 64 commits behind head on main.

Files with missing lines Patch % Lines
control-plane/pkg/kafka/topic.go 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4654      +/-   ##
============================================
+ Coverage     28.83%   36.19%   +7.35%     
- Complexity        0      901     +901     
============================================
  Files           294      403     +109     
  Lines         16172    20011    +3839     
  Branches          0      342     +342     
============================================
+ Hits           4664     7243    +2579     
- Misses        11053    12128    +1075     
- Partials        455      640     +185     
Flag Coverage Δ
java-unittests 67.15% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jijo-OO7

Copy link
Copy Markdown
Contributor Author

This change makes topic creation idempotent by checking topic metadata before issuing CreateTopic.
As a result, some existing unit/integration tests that assume CreateTopic is always called are currently failing and need their mocks updated to expect DescribeTopics.
I’m working through updating those test fixtures so they reflect the new behavior. Feedback on the approach is welcome.

@jijo-OO7

Copy link
Copy Markdown
Contributor Author

/retest-required

@jijo-OO7

jijo-OO7 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

@Cali0707 whenever you have a moment, could you take a look at this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/control-plane size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new Kafka Broker/Channel/Sink requests "CreateTopics" n times

1 participant