[fix][broker] Skip topic auto-creation for ExtensibleLoadManager internal topics#21729
Merged
heesung-sohn merged 2 commits intoDec 15, 2023
Merged
Conversation
| } | ||
|
|
||
| // ServiceUnitStateChannelImpl.TOPIC expects to be a non-partitioned-topic now. | ||
| // ExtensibleLoadManagerImpl.internal topics expects to be non-partitioned-topics now. |
Member
There was a problem hiding this comment.
Should we checkBundleOwnership in handlePartitionMetadataRequest?
Contributor
Author
There was a problem hiding this comment.
I think that can be a separate PR.
Here, I think we better explicitly say we don't allow topic auto-creation for these system topics, as we pre-create them.
Contributor
Author
There was a problem hiding this comment.
One thing we need to think about is that checking ownership there might cause some performance impact, as we will lookup/redirect to the owner brokers for every producer and consumer creation for non-persistent topics(and other topic related operations).
Demogorgon314
approved these changes
Dec 15, 2023
gaoran10
reviewed
Dec 15, 2023
gaoran10
approved these changes
Dec 15, 2023
Demogorgon314
pushed a commit
to streamnative/pulsar-archived
that referenced
this pull request
Dec 18, 2023
…rnal topics (apache#21729) (cherry picked from commit 88df040)
Member
|
Could be related to the increased flakiness of ExtensibleLoadManagerTest.testIsolationPolicy, #20608 |
15 tasks
heesung-sohn
added a commit
to heesung-sohn/pulsar
that referenced
this pull request
Jan 4, 2024
…rnal topics (apache#21729) (cherry picked from commit 88df040)
nikhil-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
Jan 4, 2024
…rnal topics (apache#21729) (cherry picked from commit 88df040) (cherry picked from commit b403f3c)
srinath-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
Jan 8, 2024
…rnal topics (apache#21729) (cherry picked from commit 88df040) (cherry picked from commit b403f3c)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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
For our load data system topics such as,
non-persistent://pulsar/system/loadbalancer-broker-load-data,allowAutoTopicCreationType=partitionedcauses other broker's producer creation to incorrectly create a new partitioned topic (although we pre-create the system topics as non-partitioned topics). Because of this, the leader broker cannot collect all brokers' load data as the other brokers publish the load data in different topics(partitioned topics).For non-persistent topics, Producer creation incorrectly creates a new partitioned topic, although the topic was created as a non-partitioned topic before.
Producer creation first fetches the partitionMedatadata from the broker, and when fetching this partitionMedatadata, brokers automatically create a new one with the conditions like the below code. The problem is that the broker serving
handlePartitionMetadataRequestmight not be the owner broker (I don't seecheckBundleOwnershipin thehandlePartitionMetadataRequestcode path.) Hence, thetopicExistscondition in the below code can becomefalse, if not owner, and create a new partitioned topic (by default).https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L3167-L3175
Modifications
Skip to auto-create ExtensibleLoadManager internal system topics, as we precreate them now.
Verifying this change
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: