You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/04/12 22:08:17 UTC
[GitHub] [pulsar] rdhabalia opened a new pull request, #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
rdhabalia opened a new pull request, #15141:
URL: https://github.com/apache/pulsar/pull/15141
### Motivation
Right now, broker gives below error while deleting empty namespace without any topic under the namespace but namespace was having partitioned-topic metadata.
```
pulsar-admin namespaces delete prop/us-west/my-ns
Cannot delete non empty namespace
Reason: Cannot delete non empty namespace
```
### Modification
It's because broker was incorrectly assuming topic existence by checking partitioned-topic metadata. delete-admin API cleans up partitioned-metadata after successfully deleting namespace and it it should not incorrectly depend on partitioned-metadata check. this issue is already fixed with forcefully delete-ns admin-api and delete-admin also needs the same fix.
### Result
User can delete namespace after successfully deleting all topics under the namespace.
--
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
[GitHub] [pulsar] 315157973 commented on a diff in pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
315157973 commented on code in PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#discussion_r964994695
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java:
##########
@@ -2431,4 +2431,27 @@ public void testPartitionedStatsAggregationByProducerNamePerPartition(String top
assertEquals(topicStats.getPublishers().size(), 2);
topicStats.getPublishers().forEach(p -> assertTrue(p.isSupportsPartialProducer()));
}
+
+ @Test
+ public void testDeleteNamespaceWithDeletedPartitionedTopic() throws Exception {
+ final String namespace = "prop-xyz/ns-delete";
+ admin.namespaces().createNamespace(namespace);
+ final String topicName = "persistent://" + namespace + "/delete-ns-topic";
+ admin.topics().createPartitionedTopic(topicName, 10);
+ List<String> topics = pulsar.getPulsarResources().getTopicResources()
+ .listPersistentTopicsAsync(NamespaceName.get(namespace)).get();
+ topics.forEach(topic -> {
+ try {
+ admin.topics().delete(topic);
+ } catch (PulsarAdminException e) {
+ fail("should not fail to delete");
+ }
+ });
+ admin.namespaces().deleteNamespace(namespace);
+ try {
+ admin.namespaces().getPolicies(namespace);
Review Comment:
There must be an exception, so a `fail()` should be added here
--
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
[GitHub] [pulsar] dlg99 commented on pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
dlg99 commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-1231866307
@rdhabalia LGTM but the PR needs rebasing
--
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] [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat closed pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
URL: https://github.com/apache/pulsar/pull/15141
--
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
[GitHub] [pulsar] github-actions[bot] commented on pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-1159601644
The pr had no activity for 30 days, mark with Stale label.
--
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
[GitHub] [pulsar] rdhabalia commented on pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-1098643851
this PR doesn't need doc update.
--
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
[GitHub] [pulsar] github-actions[bot] commented on pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-1097277313
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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] [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-6369779494
Thanks @rdhabalia, and sorry this sat so long. Master's namespace delete was rewritten (#17230, #26869) and still treats partitioned-topic metadata as a topic on purpose: the topic still exists and lookups re-create its partitions. The supported paths are `pulsar-admin topics delete-partitioned-topic` before deleting the namespace, or `namespaces delete --force`. Closing as part of the backlog triage; please reopen if you hit a case where metadata survives one of those.
--
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
[GitHub] [pulsar] Anonymitaet commented on pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-1098627702
@rdhabalia is this a bug fix and we do not need to update docs?
--
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
[GitHub] [pulsar] github-actions[bot] commented on pull request #15141: [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-1131011897
The pr had no activity for 30 days, mark with Stale label.
--
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] [pulsar-broker] Fix delete empty namespace with partitioned-topic metadata [pulsar]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15141:
URL: https://github.com/apache/pulsar/pull/15141#issuecomment-2411100026
@rdhabalia Please add the following content to your PR description and select a checkbox:
```
- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
```
--
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