Skip to content

[fix][broker] PIP-428: Fix corrupted topic policies issues with sequential topic policy updates #24427

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
lhotari merged 16 commits into from
Aug 26, 2025
Merged

[fix][broker] PIP-428: Fix corrupted topic policies issues with sequential topic policy updates #24427

lhotari merged 16 commits into from
Aug 26, 2025

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Jun 18, 2025 •
edited
Loading

Fixes #24393
Fixes #21303

Motivation

There's a long time issue where topic policies get corrupted as a result of sequential or concurrent topic policy updates.
This PR contains the implementation of
"PIP-428: Change TopicPoliciesService interface to fix consistency issues". Please refer to the PIP document at https://github.com/apache/pulsar/blob/master/pip/pip-428.md for the broader context.

Modifications

Documentation

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

@lhotari lhotari added this to the 4.1.0 milestone Jun 18, 2025
@lhotari lhotari self-assigned this Jun 18, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 18, 2025
@lhotari lhotari mentioned this pull request Jun 18, 2025
Closed
3 tasks
@lhotari lhotari requested a review from gaoran10 June 18, 2025 20:20
@lhotari lhotari mentioned this pull request Jun 18, 2025
Merged
4 tasks
@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 85.30702% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.31%. Comparing base (95dcb58) to head (a4d3924).
⚠️ Report is 35 commits behind head on master.

Files with missing lines Patch % Lines
.../service/SystemTopicBasedTopicPoliciesService.java 83.33% 17 Missing and 10 partials ⚠️
...pulsar/broker/admin/impl/PersistentTopicsBase.java 90.55% 0 Missing and 22 partials ⚠️
...che/pulsar/common/policies/data/TopicPolicies.java 80.95% 3 Missing and 5 partials ⚠️
...sar/common/policies/data/SubscriptionPolicies.java 50.00% 1 Missing and 1 partial ⚠️
...rg/apache/pulsar/broker/service/BrokerService.java 0.00% 0 Missing and 1 partial ⚠️
...ar/common/policies/data/InactiveTopicPolicies.java 50.00% 1 Missing ⚠️
...lsar/common/policies/data/PersistencePolicies.java 50.00% 1 Missing ⚠️
...pache/pulsar/common/policies/data/PublishRate.java 50.00% 1 Missing ⚠️
...pulsar/common/policies/data/RetentionPolicies.java 50.00% 1 Missing ⚠️
...che/pulsar/common/policies/data/SubscribeRate.java 50.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24427      +/-   ##
============================================
+ Coverage     73.74%   74.31%   +0.57%     
- Complexity    33174    33224      +50     
============================================
  Files          1857     1885      +28     
  Lines        146500   146943     +443     
  Branches      16835    16927      +92     
============================================
+ Hits         108041   109208    +1167     
+ Misses        29828    29034     -794     
- Partials       8631     8701      +70
Flag Coverage Δ
inttests 26.66% <21.71%> (?)
systests 22.72% <7.67%> (?)
unittests 73.80% <84.86%> (+0.05%) ⬆️

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

Files with missing lines Coverage Δ
...he/pulsar/broker/service/TopicPoliciesService.java 96.15% <ø> (+12.82%) ⬆️
...rg/apache/pulsar/broker/service/BrokerService.java 83.94% <0.00%> (ø)
...ar/common/policies/data/InactiveTopicPolicies.java 50.00% <50.00%> (ø)
...lsar/common/policies/data/PersistencePolicies.java 83.33% <50.00%> (-3.04%) ⬇️
...pache/pulsar/common/policies/data/PublishRate.java 71.42% <50.00%> (-2.26%) ⬇️
...pulsar/common/policies/data/RetentionPolicies.java 81.81% <50.00%> (-3.19%) ⬇️
...che/pulsar/common/policies/data/SubscribeRate.java 73.68% <50.00%> (-2.79%) ⬇️
...ar/common/policies/data/impl/BacklogQuotaImpl.java 91.66% <50.00%> (-2.46%) ⬇️
...ar/common/policies/data/impl/DispatchRateImpl.java 95.45% <50.00%> (-4.55%) ⬇️
...sar/common/policies/data/SubscriptionPolicies.java 40.00% <50.00%> (+40.00%) ⬆️
... and 3 more

... and 212 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.

@lhotari lhotari changed the title [WIP][fix][broker] Fix corrupted topic policies issues with sequential topic policy updates [fix][broker] Fix corrupted topic policies issues with sequential topic policy updates Jun 24, 2025
@lhotari lhotari force-pushed the lh-fix-topic-policy-mutations branch from 0312bf6 to d4ad9ee Compare August 12, 2025 06:38
@lhotari lhotari mentioned this pull request Aug 13, 2025
Closed
15 tasks
@BewareMyPower BewareMyPower changed the title [fix][broker] Fix corrupted topic policies issues with sequential topic policy updates [fix][broker] PIP-428: Fix corrupted topic policies issues with sequential topic policy updates Aug 25, 2025
@lhotari lhotari requested a review from BewareMyPower August 25, 2025 08:57
@lhotari
Copy link
Member Author

lhotari commented Aug 25, 2025

Thanks for the very helpful review, @BewareMyPower. PTAL

BewareMyPower
BewareMyPower approved these changes Aug 25, 2025
View reviewed changes
Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just leave a minor comment

@lhotari lhotari requested a review from Technoboy- August 25, 2025 13:13
@lhotari lhotari merged commit e5e7981 into apache:master Aug 26, 2025
276 of 286 checks passed
@lhotari
Copy link
Member Author

lhotari commented Aug 28, 2025

dev mailing list thread to cherry-pick to branch-4.0: https://lists.apache.org/thread/ljnhcm4pgl933kc75ftgz9yzmy03rdqg

lhotari added a commit that referenced this pull request Aug 28, 2025
@lhotari
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
f9c3845 
…ntial topic policy updates (#24427)

(cherry picked from commit e5e7981)
@lhotari lhotari mentioned this pull request Aug 28, 2025
Open
3 tasks
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 28, 2025
@lhotari @manas-ctds
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
32ed9bf 
…ntial topic policy updates (apache#24427)

(cherry picked from commit e5e7981)
(cherry picked from commit f9c3845)
Technoboy- pushed a commit to Technoboy-/pulsar that referenced this pull request Sep 10, 2025
@lhotari @Technoboy-
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
7ae63e5 
…ntial topic policy updates (apache#24427)

(cherry picked from commit e5e7981)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 12, 2025
@lhotari @srinath-ctds
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
0dc8580 
…ntial topic policy updates (apache#24427)

(cherry picked from commit e5e7981)
(cherry picked from commit f9c3845)
nborisov pushed a commit to nborisov/pulsar that referenced this pull request Sep 12, 2025
@lhotari
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
21a788c 
…ntial topic policy updates (apache#24427)

(cherry picked from commit e5e7981)
@nodece
Copy link
Member

nodece commented Sep 17, 2025

@lhotari This PR fixes NullPointerException:

java.lang.NullPointerException: Cannot invoke "java.util.concurrent.atomic.AtomicInteger.incrementAndGet()" because the return value of "java.util.Map.get(Object)" is null

  at org.apache.pulsar.broker.service.SystemTopicBasedTopicPoliciesService.addOwnedNamespaceBundleAsync(SystemTopicBasedTopicPoliciesService.java:405) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at org.apache.pulsar.broker.service.SystemTopicBasedTopicPoliciesService$2.onLoad(SystemTopicBasedTopicPoliciesService.java:509) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at org.apache.pulsar.broker.namespace.NamespaceService.notifyNamespaceBundleOwnershipListener(NamespaceService.java:1322) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at org.apache.pulsar.broker.namespace.NamespaceService.onNamespaceBundleOwned(NamespaceService.java:1285) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at org.apache.pulsar.broker.namespace.OwnershipCache.lambda$tryAcquiringOwnership$3(OwnershipCache.java:218) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at java.util.concurrent.CompletableFuture.uniApplyNow(CompletableFuture.java:684) ~[?:?]

  at java.util.concurrent.CompletableFuture.uniApplyStage(CompletableFuture.java:662) ~[?:?]

  at java.util.concurrent.CompletableFuture.thenApply(CompletableFuture.java:2168) ~[?:?]

  at org.apache.pulsar.broker.namespace.OwnershipCache.tryAcquiringOwnership(OwnershipCache.java:216) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at org.apache.pulsar.broker.namespace.NamespaceService.searchForCandidateBroker(NamespaceService.java:631) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at org.apache.pulsar.broker.namespace.NamespaceService.lambda$findBrokerServiceUrl$10(NamespaceService.java:466) ~[pulsar-broker.jar:3.0.14-SNAPSHOT]

  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) ~[?:?]

  at java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]

  at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]

  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[?:?]

  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[?:?]

  at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[netty-common-4.1.124.Final.jar:4.1.124.Final]

  at java.lang.Thread.run(Thread.java:840) ~[?:?]

Do you consider fixing this on the branch-3.0?

@lhotari
Copy link
Member Author

lhotari commented Sep 17, 2025

Do you consider fixing this on the branch-3.0?

@nodece please propose on the mailing list

@nodece
Copy link
Member

nodece commented Sep 18, 2025

@lhotari, It seems that fixing the NullPointerException may be sufficient.

@nodece
Copy link
Member

nodece commented Sep 18, 2025

I submitted #24758 to the branch-3.0.

KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
@lhotari @KannarFr
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
ea11e13 
…ntial topic policy updates (apache#24427)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
@lhotari @walkinggo
[fix][broker] PIP-428: Fix corrupted topic policies issues with seque…
0a1abca 
…ntial topic policy updates (apache#24427)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BewareMyPower BewareMyPower BewareMyPower approved these changes

@Technoboy- Technoboy- Technoboy- approved these changes

@merlimat merlimat Awaiting requested review from merlimat

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@nodece nodece Awaiting requested review from nodece

@dao-jun dao-jun Awaiting requested review from dao-jun

@poorbarcode poorbarcode Awaiting requested review from poorbarcode

@liangyepianzhou liangyepianzhou Awaiting requested review from liangyepianzhou

@gaoran10 gaoran10 Awaiting requested review from gaoran10

Assignees

@lhotari lhotari

Labels

cherry-picked/branch-4.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/4.0.7 type/PIP

Projects

None yet

Milestone

4.1.0

6 participants

@lhotari @codecov-commenter @nodece @Technoboy- @BewareMyPower @coderzc