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][broker] Update topic partition failed when config maxNumPartitionsPerPartitionedTopic<0 #22397

Merged

Conversation

hanmz
Copy link
Contributor

@hanmz hanmz commented Apr 2, 2024

Motivation

When the configuration maxNumPartitionsPerPartitionedTopic <= 0, we should consider the check to be disabled.

However, the behavior has been modified in this PR: #19166

This will cause an error when update topic partition if maxNumPartitionsPerPartitionedTopic<0.

When maxNumPartitionsPerPartitionedTopic=-1 try to update topic persistent://test-tenant/test-ns/test-topic partition number from 1 to 2. We will see the following log:
2024-04-02T19:42:01,593+0800 [pulsar-web-47-12] ERROR org.apache.pulsar.broker.admin.v2.PersistentTopics (PersistentTopics.java:844) - [null][persistent://test-tenant/test-ns/test-topic] Failed to update partition to 2 java.util.concurrent.CompletionException: org.apache.pulsar.broker.web.RestException: Desired partitions 2 can't be greater than the maximum partitions per topic -1.

Modifications

brokerMaximumPartitionsPerTopic != 0 => brokerMaximumPartitionsPerTopic > 0

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 2, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@hanmz
Copy link
Contributor Author

hanmz commented Apr 9, 2024

/pulsarbot rerun-failure-checks

@lhotari lhotari added this to the 3.3.0 milestone Apr 9, 2024
@lhotari
Copy link
Member

lhotari commented Apr 9, 2024

Closing and reopening to trigger a completely new build with changes from master since more than 3 days has passed when the build was run (shared build artifacts will no longer be available).

@lhotari lhotari closed this Apr 9, 2024
@lhotari lhotari reopened this Apr 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 9, 2024

Codecov Report

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

Project coverage is 73.15%. Comparing base (bbc6224) to head (4bf1e7a).
Report is 130 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22397      +/-   ##
============================================
- Coverage     73.57%   73.15%   -0.42%     
+ Complexity    32624    32182     -442     
============================================
  Files          1877     1885       +8     
  Lines        139502   143509    +4007     
  Branches      15299    16306    +1007     
============================================
+ Hits         102638   104989    +2351     
- Misses        28908    30309    +1401     
- Partials       7956     8211     +255     
Flag Coverage Δ
inttests 27.52% <0.00%> (+2.93%) ⬆️
systests 25.10% <0.00%> (+0.78%) ⬆️
unittests 72.19% <0.00%> (-0.65%) ⬇️

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

Files Coverage Δ
...org/apache/pulsar/broker/ServiceConfiguration.java 98.18% <ø> (-1.22%) ⬇️
...pulsar/broker/admin/impl/PersistentTopicsBase.java 56.33% <0.00%> (-9.12%) ⬇️

... and 192 files with indirect coverage changes

@lhotari lhotari merged commit fb5caeb into apache:master Apr 9, 2024
104 of 110 checks passed
lhotari pushed a commit that referenced this pull request Apr 22, 2024
…onsPerPartitionedTopic<0 (#22397)

(cherry picked from commit fb5caeb)
lhotari pushed a commit that referenced this pull request Apr 22, 2024
…onsPerPartitionedTopic<0 (#22397)

(cherry picked from commit fb5caeb)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 23, 2024
…onsPerPartitionedTopic<0 (apache#22397)

(cherry picked from commit fb5caeb)
(cherry picked from commit 386f6f0)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 23, 2024
…onsPerPartitionedTopic<0 (apache#22397)

(cherry picked from commit fb5caeb)
(cherry picked from commit 386f6f0)
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