[Bug] update partition process is not reasonable since branch-3.0

[TakaHiR07]

Search before reporting

• [x] I searched in the issues and found nothing similar.

Read release policy

• [x] I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker.

User environment

pulsar version 3.0.x

Issue Description

step 1: admin update topic partition from 50 to 150, throw error step 2: see topic partition metadata has been updated to 150, and producer try to produce msg to new partition step 3: there are 4 partition keep throw exception "topic does not exist", and other new partition can succeed.

The problem is in this pr, https://github.com/apache/pulsar/pull/19166 before pr, update partition process is:

1. create missing partitions
2. create subscription for new partition
3. update topic metadata

after pr, it become:

1. update topic metadata
2. create missing partitions
3. create subscription for new partition

That is not reasonable, pulsar producer and consumer rely on topic partition metadata to know whether the topic partition has been updated, if metadata updated, client can add new producer or new consumer to the new partition.

However, with the pr's change, although update partition is not complete, client regard it as success and try to construct new connection to the new topic partition. That is wrong.

There is another high risk situation, if update topic metadata and create missing partitions succeed, but fail on create subscription. In this moment, producer can send new msg to new partition, but after a while consumer may auto create subscription on new partition, and the read position is latest. That would result in consumer lost some message. It is very dangerous.

I don't understand why previous pr need to change the process, while it only illustrate clean the duplicate code

Error messages

Reproducing the issue

update partition, fail on step 2 or 3

1. update topic metadata
2. create missing partitions
3. create subscription for new partition

Additional information

No response

Are you willing to submit a PR?

• [x] I'm willing to submit a PR!

[lhotari]

I don't understand why previous pr need to change the process, while it only illustrate clean the duplicate code

I think it links to another PR which refers to issue #19085 and mailing list discussion https://lists.apache.org/thread/3s7b7dh0h9qcnn8x3lclxmw75okkqf37 .

[TakaHiR07]

I think it links to another PR which refers to issue #19085 and mailing list discussion https://lists.apache.org/thread/3s7b7dh0h9qcnn8x3lclxmw75okkqf37 .

@lhotari I have read this mailing list before. It looks like https://github.com/apache/pulsar/issues/19085 is fix by https://github.com/apache/pulsar/pull/19086. But I don't find any relationship between previous issue and the refactor pr. Why fix https://github.com/apache/pulsar/issues/19085 need to change the step 1,2,3 in admin.updateTopicPartition?

I think just compare the partition number can reject create non-existent partition.

[lhotari]

I have read this mailing list before. It looks like #19085 is fix by #19086. But I don't find any relationship between previous issue and the refactor pr. Why fix #19085 need to change the step 1,2,3 in admin.updateTopicPartition?

@mattisonchao do you have a chance to answer this question?

[mattisonchao]

Hi @TakaHiR07

Thanks for reporting the issue. The context you provided is very clear to me.

However, with the pr's change, although update partition is not complete, client regard it as success and try to construct new connection to the new topic partition. That is wrong.

Theoretically, the partition topic metadata should be the key point and a required field to indicate success in updating partition numbers. Other steps are just trying our best to help the client side.

In the simplest condition, we could even update the metadata itself and return it to the client. All the other resources should be lazily created.

• Topic should be auto-created.
• Subscription should be auto-created with the correct init position.

However, above all just the ideal situation.

There is another high risk situation, if update topic metadata and create missing partitions succeed, but fail on create subscription. In this moment, producer can send new msg to new partition, but after a while consumer may auto create subscription on new partition, and the read position is latest. That would result in consumer lost some message. It is very dangerous.

Yes, that's my fault. I didn't carefully consider the client-side situation, which led to missing the client's auto-created consumer subscription position.

Back to the fixes, I would prefer to change the init position of the consumer partition auto-creation to "earliest". This approach is clearer, and no dirty data is left due to the interruption of the operation.

I would not object if you insist on the rollback. But it seems like you are trying to break the rule of "a partition topic can be created if no partition metadata".

Please let me know if anything I didn't explain clearly.

[TakaHiR07]

@mattisonchao Thinks for your clear explanation, that is very helpful for me. Now I understand why the previous code is designed for obey the rule "a partition topic can not be created if no partition metadata".

For some other logic, such as admin.deletePartitionedTopic is rely on topic metadata. So previous code with this case would have problem (although this case I think is hardly happen in actual environment):

1. user execute admin.updateTopicPartition
2. "create missing partitions" success but "update topic metadata" fail
3. user do not execute admin.updateTopicPartition again, and then execute admin.deletePartitionTopic
4. would remain some partitions not delete.

I think set the consumer's default start position to "earliest" maybe is not a good way, that would bring a huge compatibility problem, since pulsar and other message queue such as kafka, always set default start position to "latest". And so many pulsar-client's compatibility would be break. low version client may lost message when topic partition updated.

If just change the start position in client#autoUpdatePartitions module, that's not complete since maybe pulsar-server update partition, client may restart and use default start position "latest" to connect.

As for me, I prefer to revert the update step-1,2,3 if without other appropriate method. Because previous code's problem is not dangerous.

I agree that we need to obey the rule "a partition topic can not be created if no partition metadata" in some other place such as BrokerService#getTopic(), PersistentTopicsBase#internalCreateNonPartitionedTopicAsync(). But in admin.updateTopicPartition, the rule seems is not important. The middle status of update topic partition not completely success is not allow in actual environment. We normally only allow topic partition update, or not update.

[mattisonchao]

I think set the consumer's default start position to "earliest" maybe is not a good way, that would bring a huge compatibility problem, since pulsar and other message queue such as kafka, always set default start position to "latest". And so many pulsar-client's compatibility would be break. low version client may lost message when topic partition updated.

Yes, we won't do that.

If just change the start position in client#autoUpdatePartitions module, that's not complete since maybe pulsar-server update partition, client may restart and use default start position "latest" to connect.

You are right, client may restart and use the default start position "latest" to connect. Here's the main concern. :/

Anyway, I checked the codes, and it seems that we will create a topic from metadata directly, which is good because it will bypass the broker topic creation -partition- keyword rejection.

We could apply your fix to fix the risky issue first, then discuss how to improve this endpoint, which has many intermediate states left if the operation is interrupted.