You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "zjxxzjwang (via GitHub)" <gi...@apache.org> on 2024/12/26 07:55:33 UTC

[PR] [improve][broker]Optimize the partition size of the system theme to reduce resource consumption [pulsar]

zjxxzjwang opened a new pull request, #23785:
URL: https://github.com/apache/pulsar/pull/23785

   ### Motivation
   The number of partitions for creating system topics depends on the value of the "defaultNumPartitions" configuration field, but this field is also the number of partitions for automatically creating topics. If the value of defaultNumPartitions is set to a large value, the system theme partition is also large, causing resource waste.
   ### Modifications
   New "systemTopicDefaultNumPartitions" configuration field, isolation system topic creation and automatically create topic for "defaultNumPartitions" fields of mutual dependence. When metadata is mounted, the system topic and common topic depend on the default partition configuration parameters
   
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: <!-- ENTER URL HERE -->
   
   <!--
   After opening this PR, the build in apache/pulsar will fail and instructions will
   be provided for opening a PR in the PR author's forked repository.
   
   apache/pulsar pull requests should be first tested in your own fork since the 
   apache/pulsar CI based on GitHub Actions has constrained resources and quota.
   GitHub Actions provides separate quota for pull requests that are executed in 
   a forked repository.
   
   The tests will be run in the forked repository until all PR review comments have
   been handled, the tests pass and the PR is approved by a reviewer.
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "HQebupt (via GitHub)" <gi...@apache.org>.
HQebupt commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2572567026

   /pulsarbot rerun-failure-checks
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#discussion_r1901575677


##########
conf/broker.conf:
##########
@@ -204,6 +204,9 @@ allowAutoSubscriptionCreation=true
 # The number of partitioned topics that is allowed to be automatically created if allowAutoTopicCreationType is partitioned.
 defaultNumPartitions=1
 
+# Default number of partitions for the system theme

Review Comment:
   `theme` doesn't make sense here, these are system topics



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker]Optimize the partition size of the system theme to reduce resource consumption [pulsar]

Posted by "HQebupt (via GitHub)" <gi...@apache.org>.
HQebupt commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2568753045

   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "zjxxzjwang (via GitHub)" <gi...@apache.org>.
zjxxzjwang commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2579715103

   @lhotari Hello, please help me review


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker]Optimize the partition size of the system theme to reduce resource consumption [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2568850060

   Please rename the title of this PR since it's currently misleading. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker]Optimize the partition size of the system theme to reduce resource consumption [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2568866009

   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "zjxxzjwang (via GitHub)" <gi...@apache.org>.
zjxxzjwang commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2589286315

   @lhotari hello,please help me review it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#discussion_r1905309475


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -3291,12 +3291,14 @@ private CompletableFuture<PartitionedTopicMetadata> createDefaultPartitionedTopi
                                                                                         Optional<Policies> policies) {
         final int defaultNumPartitions = pulsar.getBrokerService().getDefaultNumPartitions(topicName, policies);
         final int maxPartitions = pulsar().getConfig().getMaxNumPartitionsPerPartitionedTopic();
+        final int systemTopicDefaultNumPartitions = pulsar.getConfiguration().getSystemTopicDefaultNumPartitions();
         checkArgument(defaultNumPartitions > 0,
                 "Default number of partitions should be more than 0");
         checkArgument(maxPartitions <= 0 || defaultNumPartitions <= maxPartitions,
                 "Number of partitions should be less than or equal to " + maxPartitions);
 
-        PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(defaultNumPartitions);
+        PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(
+                isSystemTopic(topicName) ? systemTopicDefaultNumPartitions : defaultNumPartitions);

Review Comment:
   For consistency, it might be necessary to also add `systemTopicDefaultNumPartitions` to `AutoTopicCreationOverride`. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker]Optimize the partition size of the system theme to reduce resource consumption [pulsar]

Posted by "HQebupt (via GitHub)" <gi...@apache.org>.
HQebupt commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2568755428

   /pulsarbot rerun-failure-checks
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#discussion_r2773497017


##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java:
##########
@@ -51,6 +52,14 @@ public static ValidateResult validateOverride(AutoTopicCreationOverride override
                 if (override.getDefaultNumPartitions() <= 0) {
                     return ValidateResult.fail("[defaultNumPartitions] cannot be less than 1 for partition type.");
                 }
+                if (override.getSystemTopicDefaultNumPartitions() == null) {
+                    return ValidateResult.fail(
+                            "[systemTopicDefaultNumPartitions] cannot be null when the type is partitioned.");
+                }
+                if (override.getSystemTopicDefaultNumPartitions() <= 0) {
+                    return ValidateResult.fail(
+                            "[systemTopicDefaultNumPartitions] cannot be less than 1 for partition type.");
+                }

Review Comment:
   This wouldn't be backwards compatible for existing deployments of Pulsar. Please handle backwards compatibility in some way.



##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java:
##########
@@ -51,6 +52,14 @@ public static ValidateResult validateOverride(AutoTopicCreationOverride override
                 if (override.getDefaultNumPartitions() <= 0) {
                     return ValidateResult.fail("[defaultNumPartitions] cannot be less than 1 for partition type.");
                 }
+                if (override.getSystemTopicDefaultNumPartitions() == null) {
+                    return ValidateResult.fail(
+                            "[systemTopicDefaultNumPartitions] cannot be null when the type is partitioned.");
+                }
+                if (override.getSystemTopicDefaultNumPartitions() <= 0) {
+                    return ValidateResult.fail(
+                            "[systemTopicDefaultNumPartitions] cannot be less than 1 for partition type.");
+                }

Review Comment:
   One possible way would be to use defaultNumPartitions in the case that `systemTopicDefaultNumPartitions` is null or `<= 0`



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -3729,9 +3729,11 @@ public boolean isDefaultTopicTypePartitioned(final TopicName topicName, final Op
     public int getDefaultNumPartitions(final TopicName topicName, final Optional<Policies> policies) {
         AutoTopicCreationOverride autoTopicCreationOverride = getAutoTopicCreationOverride(topicName, policies);
         if (autoTopicCreationOverride != null) {
-            return autoTopicCreationOverride.getDefaultNumPartitions();
+            return isSystemTopic(topicName) ? autoTopicCreationOverride.getSystemTopicDefaultNumPartitions()
+                    : autoTopicCreationOverride.getDefaultNumPartitions();
         } else {
-            return pulsar.getConfiguration().getDefaultNumPartitions();
+            return isSystemTopic(topicName) ? pulsar.getConfiguration().getSystemTopicDefaultNumPartitions()
+                    : pulsar.getConfiguration().getDefaultNumPartitions();

Review Comment:
   this should check that `systemTopicDefaultNumPartitions > 0`.



##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -2343,6 +2343,12 @@ The max allowed delay for delayed delivery (in milliseconds). If the broker rece
                     + " if allowAutoTopicCreationType is partitioned."
     )
     private int defaultNumPartitions = 1;
+    @FieldContext(
+            category = CATEGORY_STORAGE_ML,
+            dynamic = true,
+            doc = "Default number of partitions for the system topics."
+    )
+    private int systemTopicDefaultNumPartitions = 1;

Review Comment:
   It would be useful to support existing way where the default number of partitions is determined by defaultNumPartitions, for example setting this value to `-1`.
   For backwards compatibility the default value should be `-1` when this change is introduced. We could make the default `1` in master branch for the feature release of Pulsar.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat closed pull request #23785: [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage
URL: https://github.com/apache/pulsar/pull/23785


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "zjxxzjwang (via GitHub)" <gi...@apache.org>.
zjxxzjwang commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2677617808

   @lhotari i，Hello, can you help me review it


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "zjxxzjwang (via GitHub)" <gi...@apache.org>.
zjxxzjwang commented on code in PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#discussion_r1905475142


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -3291,12 +3291,14 @@ private CompletableFuture<PartitionedTopicMetadata> createDefaultPartitionedTopi
                                                                                         Optional<Policies> policies) {
         final int defaultNumPartitions = pulsar.getBrokerService().getDefaultNumPartitions(topicName, policies);
         final int maxPartitions = pulsar().getConfig().getMaxNumPartitionsPerPartitionedTopic();
+        final int systemTopicDefaultNumPartitions = pulsar.getConfiguration().getSystemTopicDefaultNumPartitions();
         checkArgument(defaultNumPartitions > 0,
                 "Default number of partitions should be more than 0");
         checkArgument(maxPartitions <= 0 || defaultNumPartitions <= maxPartitions,
                 "Number of partitions should be less than or equal to " + maxPartitions);
 
-        PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(defaultNumPartitions);
+        PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(
+                isSystemTopic(topicName) ? systemTopicDefaultNumPartitions : defaultNumPartitions);

Review Comment:
   Good suggestion. I have made the corresponding modifications.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "HQebupt (via GitHub)" <gi...@apache.org>.
HQebupt commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2579853126

   /pulsarbot rerun-failure-checks
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "HQebupt (via GitHub)" <gi...@apache.org>.
HQebupt commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-3846104012

   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#discussion_r1905306191


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -3291,12 +3291,14 @@ private CompletableFuture<PartitionedTopicMetadata> createDefaultPartitionedTopi
                                                                                         Optional<Policies> policies) {
         final int defaultNumPartitions = pulsar.getBrokerService().getDefaultNumPartitions(topicName, policies);
         final int maxPartitions = pulsar().getConfig().getMaxNumPartitionsPerPartitionedTopic();
+        final int systemTopicDefaultNumPartitions = pulsar.getConfiguration().getSystemTopicDefaultNumPartitions();
         checkArgument(defaultNumPartitions > 0,
                 "Default number of partitions should be more than 0");
         checkArgument(maxPartitions <= 0 || defaultNumPartitions <= maxPartitions,
                 "Number of partitions should be less than or equal to " + maxPartitions);
 
-        PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(defaultNumPartitions);
+        PartitionedTopicMetadata configMetadata = new PartitionedTopicMetadata(
+                isSystemTopic(topicName) ? systemTopicDefaultNumPartitions : defaultNumPartitions);

Review Comment:
   Please move the logic to `getDefaultNumPartitions` method.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-2572632982

   ## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/23785?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Project coverage is 74.16%. Comparing base [(`bbc6224`)](https://app.codecov.io/gh/apache/pulsar/commit/bbc62245c5ddba1de4b1e7cee4ab49334bc36277?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`5b52438`)](https://app.codecov.io/gh/apache/pulsar/commit/5b5243898ed4598faaacd5b936edce9a74f9c5bd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 834 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/pulsar/pull/23785/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/pulsar/pull/23785?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #23785      +/-   ##
   ============================================
   + Coverage     73.57%   74.16%   +0.59%     
   + Complexity    32624     2376   -30248     
   ============================================
     Files          1877     1853      -24     
     Lines        139502   143471    +3969     
     Branches      15299    16292     +993     
   ============================================
   + Hits         102638   106406    +3768     
   + Misses        28908    28675     -233     
   - Partials       7956     8390     +434     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pulsar/pull/23785/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [inttests](https://app.codecov.io/gh/apache/pulsar/pull/23785/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.67% <25.00%> (+2.08%)` | :arrow_up: |
   | [systests](https://app.codecov.io/gh/apache/pulsar/pull/23785/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `23.14% <25.00%> (-1.19%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pulsar/pull/23785/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.69% <100.00%> (+0.85%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/23785?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...org/apache/pulsar/broker/ServiceConfiguration.java](https://app.codecov.io/gh/apache/pulsar/pull/23785?src=pr&el=tree&filepath=pulsar-broker-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2FServiceConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvU2VydmljZUNvbmZpZ3VyYXRpb24uamF2YQ==) | `98.15% <100.00%> (-1.25%)` | :arrow_down: |
   | [...rg/apache/pulsar/broker/service/BrokerService.java](https://app.codecov.io/gh/apache/pulsar/pull/23785?src=pr&el=tree&filepath=pulsar-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2Fservice%2FBrokerService.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0Jyb2tlclNlcnZpY2UuamF2YQ==) | `84.94% <100.00%> (+4.16%)` | :arrow_up: |
   
   ... and [1019 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/23785/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "zjxxzjwang (via GitHub)" <gi...@apache.org>.
zjxxzjwang commented on code in PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#discussion_r1903498132


##########
conf/broker.conf:
##########
@@ -204,6 +204,9 @@ allowAutoSubscriptionCreation=true
 # The number of partitioned topics that is allowed to be automatically created if allowAutoTopicCreationType is partitioned.
 defaultNumPartitions=1
 
+# Default number of partitions for the system theme

Review Comment:
   好的，我已经进行了修改，谢谢



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [improve][broker] Add dedicated partition configuration for system topics to reduce resource usage [pulsar]

Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on PR #23785:
URL: https://github.com/apache/pulsar/pull/23785#issuecomment-6348494915

   Hi @zjxxzjwang, thanks for the contribution and for working through the earlier review rounds.
   
   I'm going to close this one. For Pulsar 5.0, the direction is to move away from system topics: subsystems like topic policies are being reimplemented on top of the metadata store (Oxia) instead. Adding a new broker config and a new field to the public `AutoTopicCreationOverride` API to tune system-topic partitions would add surface we'd then have to deprecate. Done properly, it would also still need a PIP and the backward-compatibility changes lhotari pointed out: falling back to `defaultNumPartitions` when the new field is missing, and not requiring it in `validateOverride`.
   
   If you hit this partition overhead on 4.x in production, please open an issue with the details so we can look at a targeted fix for the maintenance branches.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org