Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] [client] PIP-344 Do not create partitioned metadata when calling pulsarClient.getPartitionsForTopic(topicName) #22206

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Mar 6, 2024

Motivation

see also PIP-344 Correct the behavior of the public API pulsarClient.getPartitionsForTopic(topicName)

Background

  • When calling pulsarClient.getPartitionsForTopic(topicName), Pulsar will automatically create the partitioned topic metadata if it does not exist, either using HttpLookupService or BinaryProtoLookupService.
  • The partitioned topic auto-creation is dependent on pulsarClient.getPartitionsForTopic
  • It triggers partitioned metadata creation by pulsarClient.getPartitionsForTopic
  • And triggers the topic partition creation by producers' registration and consumers' registration.

Issue

Since the method pulsarClient.getPartitionsForTopic is a public API, and it is named getxxx, it should edit nothing.

Modifications

  • Do not create partitioned metadata when calling pulsarClient.getPartitionsForTopic(topicName)

Next PRs

  • Section Backward & Forward Compatibility
  • Section 2 of Goals: Instead of returning a 0 partitioned metadata, respond to a not found error when calling pulsarClient.getPartitionsForTopic(String) if the topic does not exist.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 6, 2024
@poorbarcode poorbarcode self-assigned this Mar 6, 2024
@poorbarcode poorbarcode added the type/bug The PR fixed a bug or issue reported a bug label Mar 6, 2024
@poorbarcode poorbarcode added this to the 3.3.0 milestone Mar 6, 2024
@poorbarcode poorbarcode force-pushed the fix/create_metadata_when_calling_get_metadata branch from 4c09019 to 5e7372b Compare May 2, 2024 16:53
@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
@poorbarcode poorbarcode force-pushed the fix/create_metadata_when_calling_get_metadata branch from 20ce22e to 5ba345d Compare May 20, 2024 17:18
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented May 21, 2024

Codecov Report

Attention: Patch coverage is 81.25000% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 73.17%. Comparing base (bbc6224) to head (2c6e826).
Report is 297 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22206      +/-   ##
============================================
- Coverage     73.57%   73.17%   -0.40%     
- Complexity    32624    32882     +258     
============================================
  Files          1877     1890      +13     
  Lines        139502   141491    +1989     
  Branches      15299    15525     +226     
============================================
+ Hits         102638   103537     +899     
- Misses        28908    29941    +1033     
- Partials       7956     8013      +57     
Flag Coverage Δ
inttests 27.40% <30.20%> (+2.82%) ⬆️
systests 24.71% <26.04%> (+0.39%) ⬆️
unittests 72.19% <81.25%> (-0.66%) ⬇️

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

Files Coverage Δ
...pulsar/broker/admin/impl/PersistentTopicsBase.java 69.29% <100.00%> (+3.83%) ⬆️
...e/pulsar/client/impl/BinaryProtoLookupService.java 81.99% <100.00%> (-0.55%) ⬇️
...apache/pulsar/client/impl/ConsumerBuilderImpl.java 87.11% <100.00%> (+0.17%) ⬆️
...g/apache/pulsar/client/impl/HttpLookupService.java 84.95% <100.00%> (+3.70%) ⬆️
...he/pulsar/client/impl/MultiTopicsConsumerImpl.java 78.79% <100.00%> (+1.06%) ⬆️
.../transaction/TransactionCoordinatorClientImpl.java 66.07% <100.00%> (+0.30%) ⬆️
...va/org/apache/pulsar/common/protocol/Commands.java 91.01% <100.00%> (+0.42%) ⬆️
...java/org/apache/pulsar/common/util/FutureUtil.java 77.04% <100.00%> (+2.50%) ⬆️
...apache/pulsar/proxy/server/LookupProxyHandler.java 45.56% <100.00%> (-15.03%) ⬇️
...a/org/apache/pulsar/client/impl/LookupService.java 0.00% <0.00%> (ø)
... and 2 more

... and 340 files with indirect coverage changes

@poorbarcode poorbarcode requested a review from gaoran10 May 21, 2024 14:06
@poorbarcode poorbarcode force-pushed the fix/create_metadata_when_calling_get_metadata branch from 16eea03 to f7a4805 Compare May 22, 2024 04:22
@poorbarcode poorbarcode force-pushed the fix/create_metadata_when_calling_get_metadata branch from f7a4805 to 2c6e826 Compare May 22, 2024 04:23
@poorbarcode
Copy link
Contributor Author

Rebase master

@poorbarcode poorbarcode merged commit 4e5c0bc into apache:master May 23, 2024
52 checks passed
poorbarcode added a commit that referenced this pull request Jun 19, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (#22206)

(cherry picked from commit 4e5c0bc)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 24, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (apache#22206)

(cherry picked from commit 4e5c0bc)
(cherry picked from commit 9e59dd0)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 24, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (apache#22206)

(cherry picked from commit 4e5c0bc)
(cherry picked from commit 9e59dd0)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 24, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (apache#22206)

(cherry picked from commit 4e5c0bc)
(cherry picked from commit 9e59dd0)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 25, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (apache#22206)

(cherry picked from commit 4e5c0bc)
(cherry picked from commit 9e59dd0)
poorbarcode added a commit that referenced this pull request Jun 28, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (#22206)

(cherry picked from commit 4e5c0bc)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 1, 2024
…g pulsarClient.getPartitionsForTopic(topicName) (apache#22206)

(cherry picked from commit 4e5c0bc)
(cherry picked from commit 9e59dd0)
@lhotari
Copy link
Member

lhotari commented Aug 20, 2024

There's a reported bug #23203 which is related to this change.

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

Successfully merging this pull request may close these issues.

7 participants