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/03/15 02:40:57 UTC
[GitHub] [pulsar] codelipenghui commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
codelipenghui commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r826525592
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -888,10 +888,10 @@ public void deletePartitionedTopic(
@ApiParam(value = "Is authentication required to perform this operation")
@QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
@ApiParam(value = "Delete the topic's schema storage")
- @QueryParam("deleteSchema") @DefaultValue("false") boolean deleteSchema) {
+ @QueryParam("deleteSchema") @DefaultValue("true") boolean deleteSchema) {
Review comment:
We can remove it from the API definition?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -421,9 +420,8 @@ public void removeProducer(Producer producer) {
} else {
subscriptions.forEach((s, sub) -> futures.add(sub.delete()));
}
- if (deleteSchema) {
- futures.add(deleteSchema().thenApply(schemaVersion -> null));
- }
+
+ futures.add(deleteSchema().thenApply(schemaVersion -> null));
Review comment:
Yes, will delete all the versions
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentTopic.java
##########
@@ -421,9 +420,8 @@ public void removeProducer(Producer producer) {
} else {
subscriptions.forEach((s, sub) -> futures.add(sub.delete()));
}
- if (deleteSchema) {
- futures.add(deleteSchema().thenApply(schemaVersion -> null));
- }
+
+ futures.add(deleteSchema().thenApply(schemaVersion -> null));
Review comment:
Hmmm, it's a little confusing for using persistent schema storage for a non-persistent topic, after the broker restart and the producer/consumer will not connect to a non-persistent topic, the non-persistent topic is equivalent to being deleted, In some cases that users using a random name for non-persistent topic, after the application restarts, the topic name will be changes, looks like here will introduce a schema resource leak.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -956,9 +956,9 @@ public void deleteTopic(
@ApiParam(value = "Is authentication required to perform this operation")
@QueryParam("authoritative") @DefaultValue("false") boolean authoritative,
@ApiParam(value = "Delete the topic's schema storage")
- @QueryParam("deleteSchema") @DefaultValue("false") boolean deleteSchema) {
+ @QueryParam("deleteSchema") @DefaultValue("true") boolean deleteSchema) {
Review comment:
We can remove it from the API definition?
--
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