Skip to content

Conversation

@nodece
Copy link
Member

@nodece nodece commented Aug 20, 2025

PIP: #24485

Motivation

In a GEO replication scenario, if the remote cluster does not have the replicated topic and the auto-creation type differs between the local and remote clusters, message replication may fail. To ensure seamless replication, the topic metadata must be properly synchronized across clusters.

This is part of PIP-433.

#24136 is the same as this PR.

Modifications

  • When both the local and remote partitioned topic metadata indicate partitions=0, this means the topic is non-partitioned. In this case, the local cluster sends a non-partitioned topic creation request to the remote cluster.

  • If the local partitioned topic metadata has partitions>0, this means the topic is partitioned:

    • If the remote partitioned topic metadata has partitions=0, the local cluster sends a partitioned topic creation request to the remote cluster.
    • If partitions differ between the local and remote cluster, please stop GEO.

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 20, 2025
@nodece nodece force-pushed the improve-topic-creation-and-partition-update-before-starting-geo branch from 9f91e51 to a613da2 Compare August 20, 2025 14:28
@poorbarcode poorbarcode added this to the 4.2.0 milestone Sep 3, 2025
Copy link
Contributor

@poorbarcode poorbarcode left a comment

Choose a reason for hiding this comment

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

@nodece Thanks for driving this feature

}
// Set useFallbackForNonPIP344Brokers to true when mix of PIP-344 and non-PIP-344 brokers are used, it
// can still work.
return client.getLookup().getPartitionedTopicMetadata(baseTopicName, false, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have pulsarAdmin object, can we use pulsar-admin.topics.getPartitionedTopicMetadataAsync here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I will refactor this logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

@poorbarcode This is an asynchronous method, but the keyword async is missing from its name.

Copy link
Contributor

@poorbarcode poorbarcode Nov 28, 2025

Choose a reason for hiding this comment

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

Since we have pulsarAdmin object, can we use pulsar-admin.topics.getPartitionedTopicMetadataAsync here?

I have modified the code, please review it. f7d38d7

Copy link
Member Author

Choose a reason for hiding this comment

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

@poorbarcode Good, I should use pulsarAdmin instead of pulsarClient, this is better way.

@poorbarcode
Copy link
Contributor

Rebase master

@nodece nodece force-pushed the improve-topic-creation-and-partition-update-before-starting-geo branch from a47eb1f to 4c833ed Compare November 25, 2025 03:34
@poorbarcode poorbarcode closed this Dec 1, 2025
@poorbarcode poorbarcode reopened this Dec 1, 2025
@nodece nodece force-pushed the improve-topic-creation-and-partition-update-before-starting-geo branch from 90de7bd to 36661e4 Compare December 25, 2025 08:59
@nodece
Copy link
Member Author

nodece commented Dec 25, 2025

The tests testReplicatorWithSpecialNonPartitionedTopic, testFailureReplicatorWithSpecialNonPartitionedTopic, and testFailureReplicatorWithSpecialNonPartitionedTopic2 were removed in commit 4238cbd. This behavior is no longer applicable because the corresponding scenario introduced by PIP-414 has been disabled, and PIP-433 was not cherry-picked into the older branch.

The new code has been refactored by the AI.

@nodece
Copy link
Member Author

nodece commented Dec 25, 2025

ping @poorbarcode

@poorbarcode
Copy link
Contributor

@nodece I will review the PR again by this weekend

@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2026

Codecov Report

❌ Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.50%. Comparing base (d72dc04) to head (b5953d9).

Files with missing lines Patch % Lines
...er/service/persistent/GeoPersistentReplicator.java 95.34% 1 Missing and 1 partial ⚠️
...sar/broker/service/persistent/PersistentTopic.java 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24652       +/-   ##
=============================================
+ Coverage     38.90%   74.50%   +35.60%     
- Complexity    13246    33699    +20453     
=============================================
  Files          1842     1899       +57     
  Lines        145553   149746     +4193     
  Branches      16920    17413      +493     
=============================================
+ Hits          56632   111575    +54943     
+ Misses        81267    29289    -51978     
- Partials       7654     8882     +1228     
Flag Coverage Δ
inttests 26.38% <39.65%> (-0.43%) ⬇️
systests 22.98% <0.00%> (-0.11%) ⬇️
unittests 74.03% <94.82%> (+39.40%) ⬆️

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

Files with missing lines Coverage Δ
...ache/pulsar/broker/service/AbstractReplicator.java 67.91% <100.00%> (+35.98%) ⬆️
...service/nonpersistent/NonPersistentReplicator.java 64.15% <ø> (+9.43%) ⬆️
...oker/service/nonpersistent/NonPersistentTopic.java 72.23% <100.00%> (+18.51%) ⬆️
...roker/service/persistent/PersistentReplicator.java 73.57% <ø> (+38.72%) ⬆️
...ar/broker/service/persistent/ShadowReplicator.java 45.90% <ø> (+45.90%) ⬆️
...sar/broker/service/persistent/PersistentTopic.java 80.67% <88.88%> (+30.26%) ⬆️
...er/service/persistent/GeoPersistentReplicator.java 79.22% <95.34%> (+48.51%) ⬆️

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

@nodece nodece force-pushed the improve-topic-creation-and-partition-update-before-starting-geo branch from 36661e4 to f2e4b5c Compare January 8, 2026 02:06
@nodece nodece force-pushed the improve-topic-creation-and-partition-update-before-starting-geo branch from 62df37d to b5953d9 Compare January 9, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants