Skip to content

[fix][broker] Allow intermittent error from topic policies service when loading topics #24829

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.

Sign up for GitHub

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

Jump to bottom
Merged
codelipenghui merged 2 commits into from
Oct 8, 2025
Merged

[fix][broker] Allow intermittent error from topic policies service when loading topics #24829

codelipenghui merged 2 commits into from
Oct 8, 2025

Conversation

@BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Oct 8, 2025

Motivation

#24785 introduces a regression that when topic policies service failed to get topic policies, the topic future will still not be done, as well as not removed from the cache.

Modifications

Fix the regression and add testGetTopicWhenTopicPoliciesFail to prevent possible regressions.

Documentation

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

Matching PR in forked repository

PR in forked repository:

@BewareMyPower BewareMyPower self-assigned this Oct 8, 2025
@BewareMyPower BewareMyPower added type/bug The PR fixed a bug or issue reported a bug area/broker release/4.1.2 release/4.0.8 labels Oct 8, 2025
@BewareMyPower BewareMyPower added this to the 4.2.0 milestone Oct 8, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 8, 2025
@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.26%. Comparing base (5e59d0e) to head (fb6ca5b).
⚠️ Report is 25 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24829       +/-   ##
=============================================
+ Coverage     38.45%   74.26%   +35.80%     
- Complexity    13176    33411    +20235     
=============================================
  Files          1855     1912       +57     
  Lines        144926   149090     +4164     
  Branches      16814    17300      +486     
=============================================
+ Hits          55735   110719    +54984     
+ Misses        81668    29543    -52125     
- Partials       7523     8828     +1305
Flag Coverage Δ
inttests 26.26% <22.22%> (-0.14%) ⬇️
systests 22.67% <22.22%> (-0.09%) ⬇️
unittests 73.79% <100.00%> (+39.17%) ⬆️

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

Files with missing lines Coverage Δ
...rg/apache/pulsar/broker/service/BrokerService.java 83.38% <100.00%> (+24.86%) ⬆️

... and 1414 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codelipenghui codelipenghui merged commit 7b6f9fc into apache:master Oct 8, 2025
110 of 117 checks passed
lhotari pushed a commit that referenced this pull request Oct 8, 2025
@BewareMyPower @lhotari
[fix][broker] Allow intermittent error from topic policies service wh…
c3921ba 
…en loading topics (#24829)

(cherry picked from commit 7b6f9fc)
@lhotari
Copy link
Member

lhotari commented Oct 8, 2025

@BewareMyPower Please handle cherry-picking to branch-4.0 since there are some merge conflicts. It seems that merge conflicts are due to lack of PR #24118 in branch-4.0 (as per https://lists.apache.org/thread/llobypz1nc9w6wdnwd3z1h52z7bygdby).

@BewareMyPower BewareMyPower deleted the bewaremypower/handle-topic-policies-failure branch October 9, 2025 01:44
BewareMyPower added a commit that referenced this pull request Oct 9, 2025
@BewareMyPower
[fix][broker] Allow intermittent error from topic policies service wh…
dd149d4 
…en loading topics (#24829)

(cherry picked from commit 7b6f9fc)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 15, 2025
@BewareMyPower @manas-ctds
[fix][broker] Allow intermittent error from topic policies service wh…
1f2555e 
…en loading topics (apache#24829)

(cherry picked from commit 7b6f9fc)
(cherry picked from commit dd149d4)
@lhotari
Copy link
Member

lhotari commented Nov 5, 2025

@BewareMyPower In branch-4.0, The test org.apache.pulsar.broker.service.persistent.TopicDuplicationTest#testFinishTakeSnapshotWhenTopicLoading breaks with commit dd149d4. Do you have a chance to take a look?

dd149d4ba4b9bc59bbc793fb083abe09252be5bf is the first bad commit
commit dd149d4ba4b9bc59bbc793fb083abe09252be5bf
Author: Yunze Xu <xyzinfernity@163.com>
Date:   Wed Oct 8 23:42:11 2025 +0800

    [fix][broker] Allow intermittent error from topic policies service when loading topics (#24829)

    (cherry picked from commit 7b6f9fceeb56a1e85bdd917e97393ab8cb19544b)

 .../pulsar/broker/service/BrokerService.java       | 22 +++++++++++-------
 .../pulsar/broker/service/BrokerServiceTest.java   | 27 +++++++++++++++++++++-
 2 files changed, 40 insertions(+), 9 deletions(-)
bisect found first bad commit

srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 6, 2025
@BewareMyPower @srinath-ctds
[fix][broker] Allow intermittent error from topic policies service wh…
62350ef 
…en loading topics (apache#24829)

(cherry picked from commit 7b6f9fc)
(cherry picked from commit dd149d4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhotari lhotari lhotari approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

@Demogorgon314 Demogorgon314 Awaiting requested review from Demogorgon314

Assignees

@BewareMyPower BewareMyPower

Labels

area/broker cherry-picked/branch-4.0 cherry-picked/branch-4.1 doc-not-needed Your PR changes do not impact docs release/4.0.8 release/4.1.2 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Milestone

4.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@BewareMyPower @codecov-commenter @lhotari @codelipenghui