Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][broker] Inconsistent behaviour for topic auto_creation #20843

Merged
merged 3 commits into from Jul 21, 2023
Merged

[fix][broker] Inconsistent behaviour for topic auto_creation #20843

merged 3 commits into from Jul 21, 2023

Conversation

mattisonchao
Copy link
Member

@mattisonchao mattisonchao commented Jul 20, 2023

Motivation

Auto-creation topics mechanism relies on namespace policies which are very important for the broker to determine if we should auto create partitioned topic or non-partitioend topic.

We can't rely on the cache to get the random result. We should always load data from zk if the cache is missed to avoid fallback to broker default configuration.

e.g. If the metadata cache is missed, the logic will fallback to use broker configuration.

Broker configuration:

allowAutoTopicCreation=true
allowAutoTopicCreationType=non-partitioned

Namespace policies:

allowAutoTopicCreation=true
allowAutoTopicCreationType=partitioned

Modifications

  • Load cache if the policy does not exist in the metadata cache.
  • Change some code style to pass checkstyle.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

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

@mattisonchao mattisonchao self-assigned this Jul 20, 2023
@github-actions github-actions bot added doc-label-missing doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Jul 20, 2023
@apache apache deleted a comment from github-actions bot Jul 20, 2023
@mattisonchao mattisonchao added this to the 3.1.0 milestone Jul 20, 2023
@mattisonchao mattisonchao marked this pull request as ready for review July 20, 2023 09:43
@mattisonchao mattisonchao requested review from eolivelli, dlg99, Technoboy-, codelipenghui and coderzc and removed request for eolivelli July 20, 2023 10:03
@codecov-commenter
Copy link

Codecov Report

Merging #20843 (6d43e56) into master (57fbee4) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20843      +/-   ##
============================================
+ Coverage     72.97%   73.09%   +0.12%     
- Complexity    32157    32176      +19     
============================================
  Files          1868     1868              
  Lines        139164   139185      +21     
  Branches      15314    15315       +1     
============================================
+ Hits         101555   101742     +187     
+ Misses        29562    29373     -189     
- Partials       8047     8070      +23     
Flag Coverage Δ
inttests 24.16% <33.33%> (?)
systests 25.10% <88.88%> (+0.05%) ⬆️
unittests 72.40% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...rg/apache/pulsar/broker/service/BrokerService.java 81.74% <100.00%> (-0.08%) ⬇️
...ar/client/impl/PatternMultiTopicsConsumerImpl.java 87.96% <100.00%> (+2.11%) ⬆️

... and 86 files with indirect coverage changes

@mattisonchao mattisonchao merged commit 9b6a123 into apache:master Jul 21, 2023
63 of 68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants