[fix] [broker] Fix configurationMetadataSyncEventTopic is marked supporting dynamic setting, but not implemented by poorbarcode · Pull Request #22684 · apache/pulsar

poorbarcode · 2024-05-09T07:59:44Z

Motivation

The config configurationMetadataSyncEventTopic was introduced with PIP-136: Sync Pulsar policies across multiple clouds, which was released with 2.11.

This config was marked supporting dynamic setting, but has not been implemented.

Modifications

Add the implementation for dynamic setting configurationMetadataSyncEventTopic

Documentation

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

Matching PR in forked repository

PR in forked repository: x

pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java

...ar-broker/src/test/java/org/apache/pulsar/broker/service/DifferentMetadataStoreTestBase.java

pulsar-client-api/src/main/java/org/apache/pulsar/client/api/AsyncCloseable.java

…ionSyncTestBase

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java

lhotari

LGTM

poorbarcode · 2024-05-09T13:20:37Z

/pulsarbot rerun-failure-checks

codecov-commenter · 2024-05-09T15:29:27Z

Codecov Report

Attention: Patch coverage is 54.05405% with 85 lines in your changes missing coverage. Please review.

Project coverage is 73.14%. Comparing base (bbc6224) to head (fb240fc).
Report is 816 commits behind head on master.

Files with missing lines	Patch %	Lines
...roker/service/PulsarMetadataEventSynchronizer.java	45.52%	54 Missing and 13 partials ⚠️
...n/java/org/apache/pulsar/broker/PulsarService.java	60.46%	13 Missing and 4 partials ⚠️
...r/metadata/api/extended/MetadataStoreExtended.java	0.00%	1 Missing ⚠️

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

Flag	Coverage Δ
inttests	`27.37% <4.32%> (+2.79%)`	⬆️
systests	`24.64% <2.16%> (+0.32%)`	⬆️
unittests	`72.14% <54.05%> (-0.70%)`	⬇️

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

Files with missing lines	Coverage Δ
...rg/apache/pulsar/broker/service/BrokerService.java	`81.67% <100.00%> (+0.89%)`	⬆️
...pulsar/metadata/impl/LocalMemoryMetadataStore.java	`88.07% <100.00%> (+0.22%)`	⬆️
...che/pulsar/metadata/impl/RocksdbMetadataStore.java	`73.12% <100.00%> (+0.18%)`	⬆️
...ta/impl/batching/AbstractBatchedMetadataStore.java	`96.15% <100.00%> (+0.10%)`	⬆️
...e/pulsar/metadata/impl/oxia/OxiaMetadataStore.java	`90.51% <100.00%> (+0.51%)`	⬆️
...r/metadata/api/extended/MetadataStoreExtended.java	`25.00% <0.00%> (-8.34%)`	⬇️
...n/java/org/apache/pulsar/broker/PulsarService.java	`82.62% <60.46%> (+0.25%)`	⬆️
...roker/service/PulsarMetadataEventSynchronizer.java	`45.29% <45.52%> (+45.29%)`	⬆️

... and 332 files with indirect coverage changes

…orting dynamic setting, but not implemented (#22684) (cherry picked from commit ff4853e)

Demogorgon314 · 2024-05-11T06:51:05Z

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java

+                log.info("successfully created consumer {}", topicName);
+            } else {
+                State stateTransient = state;
+                log.info("[{}] Closing the new consumer because the synchronizer state is {}", stateTransient);


Fewer arguments provided (1) than placeholders specified (2)

Fixed it in the next PR : #22695

Demogorgon314 · 2024-05-11T06:51:25Z

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java

+                CompletableFuture closeConsumer = new CompletableFuture<>();
+                closeResource(() -> consumer.closeAsync(), closeConsumer);
+                closeConsumer.thenRun(() -> {
+                    log.info("[{}] Closed the new consumer because the synchronizer state is {}", stateTransient);


Same as previous.

Fixed it in the next PR : #22695

Demogorgon314 · 2024-05-11T06:52:34Z

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java

+                CompletableFuture closeConsumer = new CompletableFuture<>();
+                closeResource(() -> consumer.closeAsync(), closeConsumer);


CompletableFuture<Void> closeConsumer = new CompletableFuture<>(); closeResource(consumer::closeAsync, closeConsumer);

Improved the code in the next PR: #22695

…orting dynamic setting, but not implemented (apache#22684) (cherry picked from commit ff4853e) (cherry picked from commit 03da743)

…orting dynamic setting, but not implemented (apache#22684)

poorbarcode added 2 commits May 9, 2024 12:37

cache

24e1c6d

-

8a70022

poorbarcode added release/2.11.5 release/3.2.3 release/3.0.5 labels May 9, 2024

poorbarcode added this to the 3.4.0 milestone May 9, 2024

poorbarcode requested review from Demogorgon314, Technoboy-, codelipenghui and lhotari May 9, 2024 07:59

poorbarcode self-assigned this May 9, 2024

github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 9, 2024

lhotari reviewed May 9, 2024

View reviewed changes

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

poorbarcode added release/3.3.1 and removed release/3.2.3 labels May 9, 2024

address comment

6214623

poorbarcode requested a review from lhotari May 9, 2024 08:46

codelipenghui reviewed May 9, 2024

View reviewed changes

poorbarcode added 2 commits May 9, 2024 18:07

address comments

ae45ad6

rename DifferentMetadataStoreTestBase -> GeoReplicationWithConfigurat…

7191291

…ionSyncTestBase

poorbarcode requested a review from codelipenghui May 9, 2024 10:14

lhotari reviewed May 9, 2024

View reviewed changes

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java Show resolved Hide resolved

poorbarcode added 2 commits May 9, 2024 18:29

improve tests

bec99bb

address comment

2c229ab

poorbarcode requested a review from lhotari May 9, 2024 10:41

lhotari reviewed May 9, 2024

View reviewed changes

...r-broker/src/main/java/org/apache/pulsar/broker/service/PulsarMetadataEventSynchronizer.java Show resolved Hide resolved

poorbarcode requested a review from lhotari May 9, 2024 10:54

lhotari approved these changes May 9, 2024

View reviewed changes

improve logs

71600c7

poorbarcode added the ready-to-test label May 9, 2024

checkstyle

fb240fc

dao-jun approved these changes May 9, 2024

View reviewed changes

codelipenghui approved these changes May 10, 2024

View reviewed changes

codelipenghui merged commit ff4853e into apache:master May 10, 2024

poorbarcode deleted the fix/dynamic_Synchronizer branch May 10, 2024 02:15

poorbarcode added a commit that referenced this pull request May 10, 2024

[fix] [broker] Fix configurationMetadataSyncEventTopic is marked supp…

6e499fe

…orting dynamic setting, but not implemented (#22684) (cherry picked from commit ff4853e)

poorbarcode added a commit that referenced this pull request May 10, 2024

[fix] [broker] Fix configurationMetadataSyncEventTopic is marked supp…

03da743

…orting dynamic setting, but not implemented (#22684) (cherry picked from commit ff4853e)

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 May 10, 2024

Demogorgon314 reviewed May 11, 2024

View reviewed changes

lhotari mentioned this pull request May 13, 2024

lh jetty 12 lhotari/pulsar#189

Closed

coderzc added release/3.3.0 and removed release/3.3.1 labels May 14, 2024

hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025

[fix] [broker] Fix configurationMetadataSyncEventTopic is marked supp…

b892173

…orting dynamic setting, but not implemented (apache#22684)

		CompletableFuture closeConsumer = new CompletableFuture<>();
		closeResource(() -> consumer.closeAsync(), closeConsumer);

Conversation

poorbarcode commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Documentation

Matching PR in forked repository

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhotari left a comment

Choose a reason for hiding this comment

Uh oh!

poorbarcode commented May 9, 2024

Uh oh!

codecov-commenter commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Demogorgon314 May 11, 2024

Choose a reason for hiding this comment

Uh oh!

poorbarcode May 11, 2024

Choose a reason for hiding this comment

Uh oh!

Demogorgon314 May 11, 2024

Choose a reason for hiding this comment

Uh oh!

poorbarcode May 11, 2024

Choose a reason for hiding this comment

Uh oh!

Demogorgon314 May 11, 2024

Choose a reason for hiding this comment

Uh oh!

poorbarcode May 11, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

poorbarcode commented May 9, 2024 •

edited

Loading

codecov-commenter commented May 9, 2024 •

edited

Loading