Skip to content

[improve][broker] PIP-433: Ensure topic creation before starting GEO #24652

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
Open
nodece wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[improve][broker] PIP-433: Ensure topic creation before starting GEO #24652

nodece wants to merge 5 commits into from

Conversation

@nodece
Copy link
Member

@nodece nodece commented Aug 20, 2025 •
edited
Loading

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
poorbarcode
poorbarcode reviewed Sep 3, 2025
View reviewed changes
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

...roker/src/main/java/org/apache/pulsar/broker/service/persistent/GeoPersistentReplicator.java Outdated
}
// 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

@poorbarcode poorbarcode Sep 3, 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?

Copy link
Member Author

@nodece nodece Sep 9, 2025

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

@nodece nodece Nov 27, 2025

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 •
edited
Loading

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

@nodece nodece Dec 1, 2025

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

poorbarcode commented Nov 24, 2025

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 •
edited
Loading

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

poorbarcode commented Dec 25, 2025

@nodece I will review the PR again by this weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@poorbarcode poorbarcode poorbarcode approved these changes

@lhotari lhotari Awaiting requested review from lhotari

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

Assignees

@nodece nodece

Labels

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

Projects

None yet

Milestone

4.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@nodece @poorbarcode