You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "BewareMyPower (via GitHub)" <gi...@apache.org> on 2025/07/24 05:38:52 UTC
[PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
BewareMyPower opened a new pull request, #24554:
URL: https://github.com/apache/pulsar/pull/24554
### Motivation
https://github.com/apache/pulsar/pull/22206 misses handling the auto partitions update feature. By default, a producer or consumer schedules a periodical task to query the partition metadata via the old `PulsarClient#getPartitionsForTopic` API, which will trigger the partitioned topic auto creation.
This issue does not exist when `allowAutoTopicCreationType` is `non-partitioned` (default) because the number of partitions is 0 for non-partitioned topics, while
- Producer does not support partitions shrinking
- Consumer will ignore querying partition metadata of non-partitioned topics after https://github.com/apache/pulsar/pull/10708
### Modifications
- Use the new `getPartitionsForTopic` overload introduced in PIP-344 to disable topic auto creation in the auto partitions update task
- Add `testPartitionsNotCreatedAfterDeletion` to verify the change
NOTE: Since it relies on PIP-344, it can only be cherry-picked to branch-4.0.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: <!-- ENTER URL HERE -->
<!--
After opening this PR, the build in apache/pulsar will fail and instructions will
be provided for opening a PR in the PR author's forked repository.
apache/pulsar pull requests should be first tested in your own fork since the
apache/pulsar CI based on GitHub Actions has constrained resources and quota.
GitHub Actions provides separate quota for pull requests that are executed in
a forked repository.
The tests will be run in the forked repository until all PR review comments have
been handled, the tests pass and the PR is approved by a reviewer.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3132431618
backport for maintenance branches in #24571
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3117422907
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/24554?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `75.00000%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 74.25%. Comparing base [(`bbc6224`)](https://app.codecov.io/gh/apache/pulsar/commit/bbc62245c5ddba1de4b1e7cee4ab49334bc36277?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e36b1c6`)](https://app.codecov.io/gh/apache/pulsar/commit/e36b1c67d490097b3ccc5ca21557e1ac6ef2becc?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1225 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/24554?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/24554?src=pr&el=tree&filepath=managed-ledger%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fbookkeeper%2Fmledger%2Fimpl%2FManagedLedgerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01hbmFnZWRMZWRnZXJJbXBsLmphdmE=) | 66.66% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pulsar/pull/24554?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/pulsar/pull/24554?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #24554 +/- ##
============================================
+ Coverage 73.57% 74.25% +0.67%
+ Complexity 32624 32575 -49
============================================
Files 1877 1874 -3
Lines 139502 146230 +6728
Branches 15299 16772 +1473
============================================
+ Hits 102638 108580 +5942
- Misses 28908 29013 +105
- Partials 7956 8637 +681
```
| [Flag](https://app.codecov.io/gh/apache/pulsar/pull/24554/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [inttests](https://app.codecov.io/gh/apache/pulsar/pull/24554/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.67% <12.50%> (+2.09%)` | :arrow_up: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/24554/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `23.30% <12.50%> (-1.03%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/24554/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.75% <75.00%> (+0.91%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/24554?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...he/pulsar/client/impl/MultiTopicsConsumerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/24554?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FMultiTopicsConsumerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL011bHRpVG9waWNzQ29uc3VtZXJJbXBsLmphdmE=) | `76.48% <100.00%> (-1.25%)` | :arrow_down: |
| [...he/pulsar/client/impl/PartitionedProducerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/24554?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FPartitionedProducerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1BhcnRpdGlvbmVkUHJvZHVjZXJJbXBsLmphdmE=) | `79.41% <100.00%> (-1.36%)` | :arrow_down: |
| [...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/24554?src=pr&el=tree&filepath=managed-ledger%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fbookkeeper%2Fmledger%2Fimpl%2FManagedLedgerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01hbmFnZWRMZWRnZXJJbXBsLmphdmE=) | `81.34% <66.66%> (+0.68%)` | :arrow_up: |
... and [1112 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/24554/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3124982352
Yes, let me fix the tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #24554:
URL: https://github.com/apache/pulsar/pull/24554
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3342013120
When the fix is made, it's worth adding a backwards compatibility integration test to `tests/integration` to ensure that such regressions don't happen in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3338779929
@Shawyeok Could you open discussion in mail list so that more people will see it? I think it's an issue with the compatibility with the semantic of `getPartitionedTopicMetadata` after PIP-344, not only this PR. Client 4.x should have the same issue.
IMO, the code here https://github.com/apache/pulsar/blob/1050f48035c97a14094b812467f046f6d37ff6f4/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BinaryProtoLookupService.java#L301-L308 should be changed to something like:
```java
ByteBuf request;
if (clientCnx.isSupportsGetPartitionedMetadataWithoutAutoCreation()) {
request = Commands.newPartitionMetadataRequest(topicName.toString(), requestId,
metadataAutoCreationEnabled);
} else {
request = Commands.newPartitionMetadataRequest(topicName.toString(), requestId);
}
```
```java
public static ByteBuf newPartitionMetadataRequest(String topic, long requestId) {
BaseCommand cmd = localCmd(Type.PARTITIONED_METADATA);
cmd.setPartitionMetadata()
.setTopic(topic)
.setRequestId(requestId);
return serializeWithSize(cmd);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "Shawyeok (via GitHub)" <gi...@apache.org>.
Shawyeok commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3341024059
Mail thread: https://lists.apache.org/thread/tpd2kgl4v3x8bkw0skv0nj38rzrk56t7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3117023240
> NOTE: Since it relies on PIP-344, it can only be cherry-picked to branch-4.0.
@BewareMyPower PIP-344 has been backported to branch-3.0 and branch-3.3 too. https://lists.apache.org/thread/23ymkfdn5mhfsk1wjym8m1h0sgj6l937
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3119107557
I just noticed that ["PIP-414: Enforce topic consistency check"](https://github.com/apache/pulsar/blob/master/pip/pip-414.md) changes in #24118 are slightly related. /cc @nodece
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3119374364
@BewareMyPower The test fails in branch-4.0. I'll postpone this for 4.0.7 . Would you be able to backport this to branch-4.0 in a separate PR where we could first verify that tests pass before merging?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#discussion_r2231005543
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicAutoCreationTest.java:
##########
@@ -248,4 +254,39 @@ public void testClientWithAutoCreationGotNotFoundException() throws PulsarAdminE
admin.namespaces().deleteNamespace(namespace, true);
}
+ @Test
+ public void testPartitionsNotCreatedAfterDeletion() throws Exception {
+ @Cleanup final var client = PulsarClient.builder().serviceUrl(pulsar.getBrokerServiceUrl()).build();
+ final var topicName = TopicName.get("my-property/my-ns/testPartitionsNotCreatedAfterDeletion");
+ final var topic = topicName.toString();
+ final var interval = Duration.ofSeconds(1);
+ final ThrowableConsumer<ThrowableSupplier<Closeable>> verifier = creator -> {
+ admin.topics().createPartitionedTopic(topic, 1);
+ try (final var ignored = creator.get()) {
+ admin.topics().terminatePartitionedTopic(topic);
+ admin.topics().deletePartitionedTopic(topic, true);
+ Thread.sleep(interval.toMillis() + 500); // wait until the auto update partitions task has run
+
+ final var topics = admin.topics().getList(topicName.getNamespace()).stream()
+ .filter(__ -> __.contains(topicName.getLocalName())).toList();
+ assertTrue(topics.isEmpty(), "topics are " + topics);
+ }
+ };
+ verifier.accept(() -> client.newProducer().topic(topic)
+ .autoUpdatePartitionsInterval(interval.toSecondsPart(), TimeUnit.SECONDS).create());
+ verifier.accept(() -> client.newConsumer().topic(topic).subscriptionName("sub")
+ .autoUpdatePartitionsInterval(interval.toSecondsPart(), TimeUnit.SECONDS).subscribe());
+ verifier.accept(() -> client.newReader().topic(topic).startMessageId(MessageId.earliest)
+ .autoUpdatePartitionsInterval(interval.toSecondsPart(), TimeUnit.SECONDS).create());
+ }
+
+ private interface ThrowableConsumer<T> {
Review Comment:
btw. for future test cases, it's possible to use AssertJ's `org.assertj.core.api.ThrowingConsumer` together with AssertJ's "satisfies". I'd recommend switching to use AssertJ since the failure messages are better and there's a lot of flexibility with it's fluent API. We should migrate towards AssertJ assertions in our code base.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "Shawyeok (via GitHub)" <gi...@apache.org>.
Shawyeok commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3338482624
@BewareMyPower @poorbarcode
This patch has been backported to the 3.0.x branch and released in `3.0.13`. However, pulsar-client `3.0.13` is incompatible with older brokers when allowAutoTopicCreation is disabled, even if the topic already exists on the broker. Is this the expected behavior?
```
20:28:25.218 [pulsar-client-lookup-27-1] ERROR org.apache.pulsar.client.impl.PartitionedProducerImpl - [persistent://public/default/test-250926] Auto getting partitions failed
java.util.concurrent.CompletionException: org.apache.pulsar.client.api.PulsarClientException$FeatureNotSupportedException: The feature of getting partitions without auto-creation is not supported by the broker. Please upgrade the broker to version that supports PIP-344 to resolve this issue.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Partitioned topics are unexpectedly created by client after deletion [pulsar]
Posted by "ZachChuba (via GitHub)" <gi...@apache.org>.
ZachChuba commented on PR #24554:
URL: https://github.com/apache/pulsar/pull/24554#issuecomment-3365562684
Just want to add a +1 that this reported PIP-344 backwards compatibility is an issue that my team and other users are facing too, currently blocking our pulsar-client upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org