Skip to content

[improve][pip] PIP-414: Enforce topic consistency check #24213

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nodece
Copy link
Member

@nodece nodece commented Apr 25, 2025

Motivation

The changes from #24118 have been merged and should be documented in a Pulsar Improvement Proposal (PIP) to record these important updates.

Documentation

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

@github-actions github-actions bot added PIP doc-not-needed Your PR changes do not impact docs labels Apr 25, 2025
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.

I agree not to add a new configuration. I proposed the option just to keep compatibility for old branches.

However, we still need to take care of the compatibility issues for upgrade.

For example, given a partitioned topic whose partition metadata is accidentally deleted.

> admin topics get-partitioned-topic-metadata my-topic
Topic persistent://public/default/my-topic not found

Reason: Topic persistent://public/default/my-topic not found
> admin topics list public/default
persistent://public/default/my-topic-partition-0

There is a reader reading messages directly from my-topic-partition-0 and a producer
sending messages to this partition.

After the upgrade, the producer and reader will not be able to serve this topic.

These errors help identify misconfigured clients.

It's not misconfigured. It's on purpose. Especially for the case that some clients don't support multi-topics readers. Users have to create readers on partitions directly.

At least we need to recover the partition metadata for such orphan partitions. Currently it's impossible:

> admin topics create-partitioned-topic my-topic -p 1
2025-04-25T19:45:14,373+0800 [AsyncHttpClient-7-2] WARN  org.apache.pulsar.client.admin.internal.BaseResource - [http://localhost:8080/admin/v2/persistent/public/default/my-topic/partitions?createLocalTopicOnly=false] Failed to perform http put request: javax.ws.rs.ClientErrorException: HTTP 409 {"reason":"This topic already exists"}
This topic already exists

@BewareMyPower
Copy link
Contributor

Another approach is to create the missed partitioned metadata automatically. For example, with this PIP implemented, when the broker detects my-topic-partition-0 is accessed by a client but there is no partition metadata for my-topic. The broker should try creating the partition metadata to unblock the client. The number of partitions are determined by the largest partition index N in my-topic-partition-N.

@nodece
Copy link
Member Author

nodece commented Apr 25, 2025

At least we need to recover the partition metadata for such orphan partitions. Currently it's impossible:

> admin topics create-partitioned-topic my-topic -p 1
2025-04-25T19:45:14,373+0800 [AsyncHttpClient-7-2] WARN  org.apache.pulsar.client.admin.internal.BaseResource - [http://localhost:8080/admin/v2/persistent/public/default/my-topic/partitions?createLocalTopicOnly=false] Failed to perform http put request: javax.ws.rs.ClientErrorException: HTTP 409 {"reason":"This topic already exists"}
This topic already exists

Good catch, I can fix this bug. I think this command helps the user to recover the partitioned metadata.

Another approach is to create the missed partitioned metadata automatically.

This is difficult because we can not obtain the maximum partition index.

@BewareMyPower
Copy link
Contributor

This is difficult because we can not obtain the maximum partition index.

We can list all non-partitioned topics of the <topic>-partition-<index> pattern and find the maximum index.

@nodece
Copy link
Member Author

nodece commented Apr 27, 2025

This is difficult because we can not obtain the maximum partition index.

We can list all non-partitioned topics of the <topic>-partition-<index> pattern and find the maximum index.

@BewareMyPower Do this during topic creation (e.g., via admin topics create-partitioned-topic) or at runtime?

@crossoverJie
Copy link
Member

Another approach is to create the missed partitioned metadata automatically. For example, with this PIP implemented, when the broker detects my-topic-partition-0 is accessed by a client but there is no partition metadata for my-topic. The broker should try creating the partition metadata to unblock the client. The number of partitions are determined by the largest partition index N in my-topic-partition-N.

I agree with this plan. Our cluster has been running for many years, so there are likely quite a few topics with metadata errors.

Perhaps we should provide an additional admin interface to query topics with metadata errors.

@nodece
Copy link
Member Author

nodece commented Apr 28, 2025

I agree with this plan. Our cluster has been running for many years, so there are likely quite a few topics with metadata errors.

@crossoverJie If the broker automatically creates the partitioned metadata, this may break the user behavior.

Perhaps we should provide an additional admin interface to query topics with metadata errors.

If the topic type is incorrect, the user can use the create partitioned topic command to fix the metadata error.

@BewareMyPower
Copy link
Contributor

Do this during topic creation (e.g., via admin topics create-partitioned-topic) or at runtime?

Topic creation, or BrokerService#getTopic on a non-partitioned topic whose name ends with -partition-<index>.

@nodece
Copy link
Member Author

nodece commented Apr 28, 2025

Do this during topic creation (e.g., via admin topics create-partitioned-topic) or at runtime?

Topic creation, or BrokerService#getTopic on a non-partitioned topic whose name ends with -partition-<index>.

@BewareMyPower Topic creation is the best place to handle this. Doing it at runtime (e.g., in BrokerService#getTopic) would be too late and would introduce black-box behavior that's harder to reason about and debug.

@dao-jun
Copy link
Member

dao-jun commented May 3, 2025

I don't think create missing topic partition metadata in runtime is OK, as @nodece says, it would introduce unpredictable consequences. Actually, I think add a new config is the relatively good way.

@BewareMyPower
Copy link
Contributor

Then support creating missed partitions via admin API for such cases could be the solution. Currently the create-missed-partitions API does not work.

Signed-off-by: Zixuan Liu <[email protected]>
@nodece
Copy link
Member Author

nodece commented May 20, 2025

I don't think create missing topic partition metadata in runtime is OK, as @nodece says, it would introduce unpredictable consequences. Actually, I think add a new config is the relatively good way.

@dao-jun Instead of introducing a flag to enable or disable the creation of missing topic partition metadata at runtime, this feature will always be enabled. Since disabling this feature can lead to unexpected behavior and complexity, enabling it unconditionally simplifies the design and avoids confusion.

@nodece
Copy link
Member Author

nodece commented May 20, 2025

Then support creating missed partitions via admin API for such cases could be the solution. Currently the create-missed-partitions API does not work.

@BewareMyPower admin topics create-partitioned-topic my-topic -p 1 has been fixed by #24225

Could you review this PIP again?

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 PIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants