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

[improve][broker] checkTopicExists supports checking partitioned topic without index #21701

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

nodece
Copy link
Member

@nodece nodece commented Dec 9, 2023

Motivation

The checkTopicExists method doesn't support checking the partitioned topic without the partition index.

Modifications

Refactor the checkTopicExists method:

  1. If the topic is persistent and includes -partition-, find the topic from the /managed-ledger path.
  2. If the topic is found in the managed-ledger path, return true, otherwise continue finding.
  3. Use the getPartitionedTopicName() name to query the metadata of the topic from /admin/partitioned-topics path, and return true if metadata.partitions is greater than 0, indicating that it is a partitioned topic and exists, otherwise go and find if the non-partitioned topic exist, the persistent topic exists in the /managed-ledgers path, the non-persistent topic only exist in the broker's topics map.

Verifying this change

Added test.

Documentation

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

@nodece nodece changed the title [improve][broker] checkTopicExists directly checks topic metadata [improve][broker] checkTopicExists supports checking partitioned topic without index Dec 9, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 9, 2023
…c without index

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

we should keep the original logic(check isPersistent first ) to avoid the topic is non-partitioned non-persistent

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
Copy link
Member Author

nodece commented Dec 11, 2023

we should keep the original logic(check isPersistent first ) to avoid the topic is non-partitioned non-persistent

@Technoboy- Done.

@nodece
Copy link
Member Author

nodece commented Dec 12, 2023

/pulsarbot rerun-failure-checks

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #21701 (a05463b) into master (2e31e71) will decrease coverage by 0.02%.
Report is 3 commits behind head on master.
The diff coverage is 94.44%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21701      +/-   ##
============================================
- Coverage     73.40%   73.38%   -0.02%     
- Complexity    32662    32757      +95     
============================================
  Files          1893     1893              
  Lines        140767   140709      -58     
  Branches      15506    15500       -6     
============================================
- Hits         103328   103262      -66     
- Misses        29321    29339      +18     
+ Partials       8118     8108      -10     
Flag Coverage Δ
inttests 24.15% <94.44%> (+0.01%) ⬆️
systests 24.80% <66.66%> (-0.16%) ⬇️
unittests 72.67% <94.44%> (-0.01%) ⬇️

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

Files Coverage Δ
...ache/pulsar/broker/namespace/NamespaceService.java 71.95% <94.44%> (-0.31%) ⬇️

... and 94 files with indirect coverage changes

@Technoboy- Technoboy- added this to the 3.2.0 milestone Dec 19, 2023
@nodece nodece merged commit f970534 into apache:master Dec 19, 2023
48 checks passed
Technoboy- pushed a commit that referenced this pull request May 18, 2024
…c without index (#21701)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 31, 2024
…c without index (apache#21701)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 8b8048c)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 4, 2024
…c without index (apache#21701)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 8b8048c)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 4, 2024
…c without index (apache#21701)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 8b8048c)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 7, 2024
…c without index (apache#21701)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 8b8048c)
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.

None yet

3 participants