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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
