[improve][broker] PIP-300: Add custom dynamic configuration for plugins #20884

nodece · 2023-07-26T10:37:31Z

Motivation

See PIP-300

Modifications

Update PIP-300 goals and fix the wrong code
Add registerCustomDynamicConfiguration() to support adding the dynamic configuration.

Verifying this change

Make sure that the change passes the CI checks.
Add testRegisterCustomDynamicConfiguration() test.

Documentation

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

poorbarcode · 2023-07-28T09:27:55Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

        final Field field;
+        // value holds the external dynamic configuration.
+        final String value;


Why add the new field value, I could not find the place which uses it.

This value can be removed, and then call the addCustomDynamicConfiguration() without the value.

poorbarcode · 2023-07-28T09:32:37Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

+     * <p>
+     * The custom dynamic configuration doesn't support setting up a listener.
+     */
+    public void addCustomDynamicConfiguration(String key, String defaultValue, Predicate<String> validator) {


This new API is dangerous because some config's dynamic update is not supported now, such as messageExpiryCheckIntervalInMinutes, serverPort, the field can be marked dynamic update only when we have genuinely implemented the dynamic update.

@codelipenghui @Technoboy- Could you take a look?

I see, this method is used for the third-party plugin, when a dynamic configuration has been added, the user can use the PulsarAdmin to update this dynamic configuration, and then the plugin can also call the pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfigurationAsync() to get the dynamic configuration.

nodece · 2023-08-17T05:54:05Z

@liudezhi2098 If you want to cherry-pick this PR to branch-2.10 and branch-2.11, please use release/2.10.6 and release/2.11.3 labels, when this PR into branch-2.10 and branch-2.11, please append cherry-picked/branch-2.10 and cherry-picked/branch-2.11 labels.

nodece · 2023-09-01T11:07:57Z

/pulsarbot rerun-failure-checks

codelipenghui · 2023-09-04T01:51:09Z

@nodece This is a new capability that will be introduced to Pulsar plugins. It should be started with a PIP, and we can't cherry-pick features to the patch releases.

github-actions · 2023-10-05T01:48:11Z

The pr had no activity for 30 days, mark with Stale label.

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

nodece · 2023-12-13T04:33:28Z

@nodece Are you still working on this proposal?

I'm sorry for missing out on this PR recently. I can continue working on it.

BewareMyPower · 2023-12-13T06:21:22Z

Could you rebase to master since the branch of PR is far behind the master branch?

codecov-commenter · 2023-12-13T07:46:35Z

Codecov Report

Merging #20884 (a2e2bad) into master (50007c3) will decrease coverage by 0.13%.
Report is 2 commits behind head on master.
The diff coverage is 87.50%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #20884      +/-   ##
============================================
- Coverage     73.50%   73.37%   -0.13%     
+ Complexity    32818    32750      -68     
============================================
  Files          1893     1893              
  Lines        140721   140726       +5     
  Branches      15502    15502              
============================================
- Hits         103432   103253     -179     
- Misses        29200    29356     +156     
- Partials       8089     8117      +28

Flag	Coverage Δ
inttests	`24.14% <4.16%> (-0.02%)`	⬇️
systests	`24.70% <4.16%> (-0.14%)`	⬇️
unittests	`72.65% <87.50%> (-0.13%)`	⬇️

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

Files	Coverage Δ
...rg/apache/pulsar/broker/service/BrokerService.java	`81.18% <87.50%> (-0.11%)`	⬇️

... and 77 files with indirect coverage changes

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

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

...r-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiDynamicConfigurationsTest.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

nodece · 2023-12-13T09:37:19Z

@BewareMyPower Thanks for your review and your suggestions have been applied.

BewareMyPower · 2023-12-13T11:48:36Z

@mattisonchao Could you take a look again?

BewareMyPower · 2023-12-13T13:08:36Z

@nodece Please fix the code style:

[INFO] There are 3 errors reported by Checkstyle 8.37 with /home/runner/work/pulsar/pulsar/buildtools/src/main/resources/pulsar/checkstyle.xml ruleset.
Error:  src/main/java/org/apache/pulsar/broker/service/BrokerService.java:[2528,79] (whitespace) WhitespaceAround: '==' is not followed by whitespace.
Error:  src/main/java/org/apache/pulsar/broker/service/BrokerService.java:[2528,93] (whitespace) WhitespaceAround: ':' is not followed by whitespace.
Error:  src/test/java/org/apache/pulsar/broker/admin/AdminApiDynamicConfigurationsTest.java:[28,8] (imports) UnusedImports: Unused import: java.util.function.Consumer.

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 26, 2023

nodece force-pushed the add-custom-dynamic-configuration branch from 3b2294a to 98eb21f Compare July 26, 2023 10:41

nodece requested review from lhotari and poorbarcode July 28, 2023 08:36

poorbarcode reviewed Jul 28, 2023

View reviewed changes

poorbarcode requested review from Technoboy- and codelipenghui July 28, 2023 09:32

Technoboy- added this to the 3.2.0 milestone Jul 31, 2023

liudezhi2098 added cherry-picked/branch-2.10 cherry-picked/branch-2.11 labels Aug 17, 2023

nodece marked this pull request as draft August 17, 2023 04:02

nodece force-pushed the add-custom-dynamic-configuration branch 3 times, most recently from ccd4c39 to 1d99f18 Compare August 17, 2023 05:49

nodece closed this Aug 17, 2023

nodece force-pushed the add-custom-dynamic-configuration branch from 1d99f18 to 0cb1c78 Compare August 17, 2023 05:50

nodece reopened this Aug 17, 2023

nodece marked this pull request as ready for review August 17, 2023 05:50

nodece added release/2.11.3 release/2.10.6 and removed cherry-picked/branch-2.10 cherry-picked/branch-2.11 labels Aug 17, 2023

nodece force-pushed the add-custom-dynamic-configuration branch from 62736f6 to bfd88d0 Compare August 17, 2023 05:55

liudezhi2098 approved these changes Sep 1, 2023

View reviewed changes

nodece added the ready-to-test label Sep 1, 2023

github-actions bot added the Stale label Oct 5, 2023

BewareMyPower mentioned this pull request Dec 12, 2023

[improve][broker] PIP-300: Add custom dynamic configuration for plugins #21714

Closed

4 tasks

nodece commented Dec 13, 2023

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java Outdated Show resolved Hide resolved

BewareMyPower closed this Dec 13, 2023

BewareMyPower reopened this Dec 13, 2023

[improve][broker] PIP-300: Add custom dynamic configuration for plugins

2da728d

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

nodece force-pushed the add-custom-dynamic-configuration branch from c04dbd2 to 2da728d Compare December 13, 2023 07:58

github-actions bot added the type/PIP label Dec 13, 2023

BewareMyPower reviewed Dec 13, 2023

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java Outdated Show resolved Hide resolved

Update

ce1ed61

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

BewareMyPower reviewed Dec 13, 2023

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java Outdated Show resolved Hide resolved

Update

ddd0552

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

nodece removed the ready-to-test label Dec 13, 2023

nodece and others added 4 commits December 13, 2023 17:24

Update

6da5743

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

Update

768143a

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

Update

34c8e84

Co-authored-by: Yunze Xu <xyzinfernity@163.com>

Update

459a9f1

nodece added the ready-to-test label Dec 13, 2023

nodece requested review from BewareMyPower and poorbarcode December 13, 2023 09:43

BewareMyPower approved these changes Dec 13, 2023

View reviewed changes

nodece added 2 commits December 13, 2023 22:12

Fix style

6c653a9

Fix style

a2e2bad

nodece merged commit 41b1ab1 into apache:master Dec 14, 2023
48 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[improve][broker] PIP-300: Add custom dynamic configuration for plugins #20884

[improve][broker] PIP-300: Add custom dynamic configuration for plugins #20884

nodece commented Jul 26, 2023 •

edited

Loading

poorbarcode Jul 28, 2023

nodece Jul 28, 2023

poorbarcode Jul 28, 2023 •

edited

Loading

poorbarcode Jul 28, 2023

nodece Jul 28, 2023

nodece commented Aug 17, 2023

nodece commented Sep 1, 2023

codelipenghui commented Sep 4, 2023 •

edited

Loading

github-actions bot commented Oct 5, 2023

nodece commented Dec 13, 2023

BewareMyPower commented Dec 13, 2023

codecov-commenter commented Dec 13, 2023 •

edited

Loading

nodece commented Dec 13, 2023

BewareMyPower commented Dec 13, 2023

BewareMyPower commented Dec 13, 2023

[improve][broker] PIP-300: Add custom dynamic configuration for plugins #20884

[improve][broker] PIP-300: Add custom dynamic configuration for plugins #20884

Conversation

nodece commented Jul 26, 2023 • edited Loading

Motivation

Modifications

Verifying this change

Documentation

poorbarcode Jul 28, 2023

Choose a reason for hiding this comment

nodece Jul 28, 2023

Choose a reason for hiding this comment

poorbarcode Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

poorbarcode Jul 28, 2023

Choose a reason for hiding this comment

nodece Jul 28, 2023

Choose a reason for hiding this comment

nodece commented Aug 17, 2023

nodece commented Sep 1, 2023

codelipenghui commented Sep 4, 2023 • edited Loading

github-actions bot commented Oct 5, 2023

nodece commented Dec 13, 2023

BewareMyPower commented Dec 13, 2023

codecov-commenter commented Dec 13, 2023 • edited Loading

Codecov Report

nodece commented Dec 13, 2023

BewareMyPower commented Dec 13, 2023

BewareMyPower commented Dec 13, 2023

nodece commented Jul 26, 2023 •

edited

Loading

poorbarcode Jul 28, 2023 •

edited

Loading

codelipenghui commented Sep 4, 2023 •

edited

Loading

codecov-commenter commented Dec 13, 2023 •

edited

Loading