[fix] [broker] Fix configurationMetadataSyncEventTopic is marked supporting dynamic setting, but not implemented #22684
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
poorbarcode
requested review from
lhotari,
Technoboy-,
codelipenghui and
Demogorgon314
lhotari
reviewed
May 9, 2024
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
Outdated
Show resolved
Hide resolved
...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java
Outdated
Show resolved
Hide resolved
...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java
Outdated
Show resolved
Hide resolved
...ar-broker/src/test/java/org/apache/pulsar/broker/service/DifferentMetadataStoreTestBase.java
Outdated
Show resolved
Hide resolved
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/AsyncCloseable.java
Outdated
Show resolved
Hide resolved
lhotari
reviewed
May 9, 2024
...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java
Show resolved
Hide resolved
lhotari
reviewed
May 9, 2024
...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java
Show resolved
Hide resolved
|
/pulsarbot rerun-failure-checks |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #22684 +/- ##
============================================
- Coverage 73.57% 73.14% -0.43%
+ Complexity 32624 32502 -122
============================================
Files 1877 1888 +11
Lines 139502 141237 +1735
Branches 15299 15504 +205
============================================
+ Hits 102638 103308 +670
- Misses 28908 29937 +1029
- Partials 7956 7992 +36
Flags with carried forward coverage won't be shown. Click here to find out more.
|
dao-jun
approved these changes
May 9, 2024
codelipenghui
approved these changes
May 10, 2024
poorbarcode
added
cherry-picked/branch-3.0
cherry-picked/branch-3.2
cherry-picked/branch-3.3
and removed
cherry-picked/branch-3.2
labels
| log.info("successfully created consumer {}", topicName); | ||
| } else { | ||
| State stateTransient = state; | ||
| log.info("[{}] Closing the new consumer because the synchronizer state is {}", stateTransient); |
There was a problem hiding this comment.
Fewer arguments provided (1) than placeholders specified (2)
There was a problem hiding this comment.
Fixed it in the next PR : #22695
| CompletableFuture closeConsumer = new CompletableFuture<>(); | ||
| closeResource(() -> consumer.closeAsync(), closeConsumer); | ||
| closeConsumer.thenRun(() -> { | ||
| log.info("[{}] Closed the new consumer because the synchronizer state is {}", stateTransient); |
There was a problem hiding this comment.
Fixed it in the next PR : #22695
Comment on lines
+224
to
+225
| CompletableFuture closeConsumer = new CompletableFuture<>(); | ||
| closeResource(() -> consumer.closeAsync(), closeConsumer); |
There was a problem hiding this comment.
CompletableFuture<Void> closeConsumer = new CompletableFuture<>();
closeResource(consumer::closeAsync, closeConsumer);
There was a problem hiding this comment.
Improved the code in the next PR: #22695
nikhil-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 13, 2024
…orting dynamic setting, but not implemented (apache#22684) (cherry picked from commit ff4853e) (cherry picked from commit 03da743)
srinath-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 16, 2024
…orting dynamic setting, but not implemented (apache#22684) (cherry picked from commit ff4853e) (cherry picked from commit 03da743)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The config
configurationMetadataSyncEventTopicwas introduced with PIP-136: Sync Pulsar policies across multiple clouds, which was released with2.11.This config was marked supporting dynamic setting, but has not been implemented.
Modifications
Add the implementation for dynamic setting
configurationMetadataSyncEventTopicDocumentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: x