[improve][broker] Add dedicated partition configuration for system topics to reduce resource usage#23785
Conversation
…educe resource consumption
…educe resource consumption
…educe resource consumption
|
LGTM |
|
/pulsarbot rerun-failure-checks |
|
Please rename the title of this PR since it's currently misleading. |
|
/pulsarbot rerun-failure-checks |
lhotari
changed the title
…educe resource consumption
|
/pulsarbot rerun-failure-checks |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@lhotari i，Hello, can you help me review it |
lhotari
added
release/3.0.12
release/4.0.5
and removed
release/3.0.11
release/4.0.4
labels
lhotari
added
release/3.0.13
release/4.0.6
and removed
release/3.0.12
release/4.0.5
labels
lhotari
added
release/3.0.14
release/4.0.7
and removed
release/3.0.13
release/4.0.6
labels
lhotari
added
release/3.0.15
release/4.0.8
and removed
release/3.0.14
release/4.0.7
labels
lhotari
added
release/3.0.16
release/4.0.9
and removed
release/3.0.15
release/4.0.8
labels
…tionsForAutomaticSystemTopic
|
/pulsarbot rerun-failure-checks |
lhotari
left a comment
lhotari
left a comment
There was a problem hiding this comment.
Please check the review comments. Please ensure that the command line tool pulsar-admin also supports setting the value for the override.
| 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."); | ||
| } |
There was a problem hiding this comment.
This wouldn't be backwards compatible for existing deployments of Pulsar. Please handle backwards compatibility in some way.
There was a problem hiding this comment.
One possible way would be to use defaultNumPartitions in the case that systemTopicDefaultNumPartitions is null or <= 0
| dynamic = true, | ||
| doc = "Default number of partitions for the system topics." | ||
| ) | ||
| private int systemTopicDefaultNumPartitions = 1; |
There was a problem hiding this 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.
| return isSystemTopic(topicName) ? pulsar.getConfiguration().getSystemTopicDefaultNumPartitions() | ||
| : pulsar.getConfiguration().getDefaultNumPartitions(); |
There was a problem hiding this comment.
this should check that systemTopicDefaultNumPartitions > 0.
6 participants
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
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: