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/08 15:30:19 UTC
[GitHub] [pulsar] yuruguo opened a new pull request #14608: Ensure the deletion consistency of topic and schema
yuruguo opened a new pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608
Master Issue: #12795
### Motivation
Deleting topic also deletes its schema.
### Documentation
- [x] `no-need-doc`
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1067886987
> I noticed there is a behavior change that might impact users' applications if the schema auto uploading is disabled.
>
> Before this change, users can delete the topic without deleting schema, so that the application can connect to the newly created topic with the same topic name and existing schema, but after this change, not able to connect to the topic again.
>
> Hmmm, before, users usually use the delete topic to truncate data of the topic. After #10326 introduced truncate topic, I think for now users should to use the truncate method to cleanup data of the topic. So there is no reason why the user still wants to use this topic, but also delete it.
>
> We'd better clarify in the document.
I have added explanation in the CLI / API doc.
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063675146
@codelipenghui @hangc0276 @nlu90 ptal
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r823300837
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
yes, this is a break change and we hope to release on next major branch( such as 1.11).
Detail in dev to [here](https://lists.apache.org/thread/wbny8x0b31jdkgoc9cjlc29gt9fdsm6v).
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833020129
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -653,14 +659,14 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
- "--deleteSchema" }, description = "Delete schema while deleting topic")
+ @Parameter(names = {"-d", "--deleteSchema"}, description = "Delete schema while deleting topic, "
+ + "but the parameter is invalid and the schema is always deleted", hidden = true)
private boolean deleteSchema = false;
Review comment:
[same](https://github.com/apache/pulsar/pull/14608#discussion_r833000802) with above
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
[t.delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-0210356c8a88e4efa89eb769a027fa6c166db479dbad8bbbbc704c6ed6e317f5R1009) will delete schema by default.
details: [delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1066-R1067) -> [delete(boolean,boolean,boolean)](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1098-R1100) -> [deleteSchema()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bL1150-R1142)
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1065851359
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r823300837
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
yes, this is a break change and we hope to release on next major branch( such as `2.11`？). See detail in @dev to [here](https://lists.apache.org/thread/wbny8x0b31jdkgoc9cjlc29gt9fdsm6v).
But after thinking about it I'd prefer to hide `-d` rather than remove for compatibility, then declare that this parameter is invalid and the new behavior.
what about your opinon? @Jason918
--
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] nlu90 commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
nlu90 commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r826376317
##########
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:
What happens to topics with multiple versions fo schema? Will all versions be cleaned?
--
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] codelipenghui commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r830836067
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -653,14 +659,14 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
- "--deleteSchema" }, description = "Delete schema while deleting topic")
+ @Parameter(names = {"-d", "--deleteSchema"}, description = "Delete schema while deleting topic, "
+ + "but the parameter is invalid and the schema is always deleted", hidden = true)
private boolean deleteSchema = false;
Review comment:
Should also update the default value to true?
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,19 +635,22 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
- "--deleteSchema" }, description = "Delete schema while deleting topic")
+ @Parameter(names = {"-d", "--deleteSchema"}, description = "Delete schema while deleting topic, "
+ + "but the parameter is invalid and the schema is always deleted", hidden = true)
private boolean deleteSchema = false;
Review comment:
Should also update the default value to true?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -581,14 +582,6 @@ protected void internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
if (numPartitions < 1){
return CompletableFuture.completedFuture(null);
}
- if (deleteSchema) {
- return pulsar().getBrokerService()
- .deleteSchemaStorage(topicName.getPartition(0).toString())
- .thenCompose(unused ->
- internalRemovePartitionsAuthenticationPoliciesAsync(numPartitions))
- .thenCompose(unused2 ->
- internalRemovePartitionsTopicAsync(numPartitions, force));
- }
Review comment:
If remove these lines, how does the method remove the schema?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
If remove these lines how do we delete the schema?
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1067886987
> I noticed there is a behavior change that might impact users' applications if the schema auto uploading is disabled.
>
> Before this change, users can delete the topic without deleting schema, so that the application can connect to the newly created topic with the same topic name and existing schema, but after this change, not able to connect to the topic again.
>
> Hmmm, before, users usually use the delete topic to truncate data of the topic. After #10326 introduced truncate topic, I think for now users should to use the truncate method to cleanup data of the topic. So there is no reason why the user still wants to use this topic, but also delete it.
>
> We'd better clarify in the document.
I have added explanation in the CLI / API doc.
--
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] yuruguo edited a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1076731767
> Please also help add a test to make sure the schema been removed after the topic has been deleted.
The test [testDeleteTopicAndSchemaForV1](https://github.com/apache/pulsar/pull/14608/files#diff-a66c994d7960c142df6d5f7b66762090d23808cee66406b6398f420f3727e928R778) in `SchemaTest` can cover this change.
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062608662
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062724211
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062939403
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063954147
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063675146
@codelipenghui @hangc0276 @nlu90 ptal
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1064989214
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
[L1009](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-0210356c8a88e4efa89eb769a027fa6c166db479dbad8bbbbc704c6ed6e317f5R1009) will delete schema but default.
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
[L1009](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-0210356c8a88e4efa89eb769a027fa6c166db479dbad8bbbbc704c6ed6e317f5R1009) will delete schema by default.
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
[L1009](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-0210356c8a88e4efa89eb769a027fa6c166db479dbad8bbbbc704c6ed6e317f5R1009) will delete schema by default.
[delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1066-R1067) -> [delete(boolean,boolean,boolean)](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1098-R1100) -> [deleteSchema()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bL1150-R1142)
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062719739
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063468989
/pulsarbot run-failure-checks
--
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] codelipenghui commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [pulsar] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1065857804
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1064942621
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r823300837
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
yes, this is a break change and we hope to release on next major branch( such as 2.11). Detail in @dev to [here](https://lists.apache.org/thread/wbny8x0b31jdkgoc9cjlc29gt9fdsm6v).
But after thinking about it I'd prefer to hide `-d` rather than remove for compatibility, then declare that the parameter is invalid and the new behavior.
what about your opinon? @Jason918
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062724211
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1065851359
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r826626923
##########
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:
All done
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
[t.delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-0210356c8a88e4efa89eb769a027fa6c166db479dbad8bbbbc704c6ed6e317f5R1009) will delete schema by default.
[delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1066-R1067) -> [delete(boolean,boolean,boolean)](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1098-R1100) -> [deleteSchema()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bL1150-R1142)
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062683392
/pulsarbot run-failure-checks
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063042940
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r823300837
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
yes, this is a break change and we hope to release on next major branch( such as `2.11`？). Detail in @dev to [here](https://lists.apache.org/thread/wbny8x0b31jdkgoc9cjlc29gt9fdsm6v).
But after thinking about it I'd prefer to hide `-d` rather than remove for compatibility, then declare that the parameter is invalid and the new behavior.
what about your opinon? @Jason918
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1064757069
@codelipenghui @hangc0276 @Technoboy- @nlu90 PTAL
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1067906991
> I noticed there is a behavior change that might impact users' applications if the schema auto uploading is disabled.
>
> Before this change, users can delete the topic without deleting schema, so that the application can connect to the newly created topic with the same topic name and existing schema, but after this change, not able to connect to the topic again.
>
> Hmmm, before, users usually use the delete topic to truncate data of the topic. After #10326 introduced truncate topic, I think for now users should to use the truncate method to cleanup data of the topic. So there is no reason why the user still wants to use this topic, but also delete it.
>
> We'd better clarify in the document.
I have added explanation in the CLI / API.
PTAL again, thx!
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833632609
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -581,14 +582,6 @@ protected void internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
if (numPartitions < 1){
return CompletableFuture.completedFuture(null);
}
- if (deleteSchema) {
- return pulsar().getBrokerService()
- .deleteSchemaStorage(topicName.getPartition(0).toString())
- .thenCompose(unused ->
- internalRemovePartitionsAuthenticationPoliciesAsync(numPartitions))
- .thenCompose(unused2 ->
- internalRemovePartitionsTopicAsync(numPartitions, force));
- }
Review comment:
L586 will delete schema but default.
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
L1009 will delete schema but default.
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1064942621
/pulsarbot run-failure-checks
--
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] Jason918 commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r823282315
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
Will this command fails if user keep "-d" in command line?
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062719739
/pulsarbot run-failure-checks
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062683392
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833633795
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1027,14 +1006,7 @@ public void unloadNamespaceBundlesGracefully(int maxConcurrentUnload, boolean cl
new IllegalStateException("Delete forbidden topic is replicated on clusters " + clusters));
}
- if (deleteSchema) {
- return t.deleteSchema().thenCompose(schemaVersion -> {
- log.info("Successfully delete topic {}'s schema of version {}", t.getName(), schemaVersion);
- return t.delete();
- });
Review comment:
[t.delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-0210356c8a88e4efa89eb769a027fa6c166db479dbad8bbbbc704c6ed6e317f5R1009) will delete schema by default.
details: [delete()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1066-R1067) -> [delete(boolean,boolean,boolean)](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bR1098-R1100) -> [deleteSchema()](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-5edf14cc6f25857d0cfdd26b2d3b3141230ecfb0dfa95aebf7583fd76ede4c4bL1150-R1142)
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833000802
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,19 +635,22 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
- "--deleteSchema" }, description = "Delete schema while deleting topic")
+ @Parameter(names = {"-d", "--deleteSchema"}, description = "Delete schema while deleting topic, "
+ + "but the parameter is invalid and the schema is always deleted", hidden = true)
private boolean deleteSchema = false;
Review comment:
Maybe we can't change the default value to `true` because that would change its usage.
Before modification: it means false if you don't specify `-d`, and it means true if you specify `-d`;
After modification: it means true if you don’t specify `-d`, but it means false only you specify `-d false`.
The two are not fully compatible.
It is similar to #12128
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1076731767
> Please also help add a test to make sure the schema been removed after the topic has been deleted.
The test [testDeleteTopicAndSchemaForV1](https://github.com/apache/pulsar/pull/14608/files#diff-a66c994d7960c142df6d5f7b66762090d23808cee66406b6398f420f3727e928R778) in `SchemaTest` can cover this change.
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833632609
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -581,14 +582,6 @@ protected void internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
if (numPartitions < 1){
return CompletableFuture.completedFuture(null);
}
- if (deleteSchema) {
- return pulsar().getBrokerService()
- .deleteSchemaStorage(topicName.getPartition(0).toString())
- .thenCompose(unused ->
- internalRemovePartitionsAuthenticationPoliciesAsync(numPartitions))
- .thenCompose(unused2 ->
- internalRemovePartitionsTopicAsync(numPartitions, force));
- }
Review comment:
L586 will delete schema by default.
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062939403
/pulsarbot run-failure-checks
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063954147
/pulsarbot run-failure-checks
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1065853658
/pulsarbot run-failure-checks
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063468989
/pulsarbot run-failure-checks
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r823300837
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
yes, this is a break change and we hope to release on next major branch( such as 1.11).
Detail in dev to [here](https://lists.apache.org/thread/wbny8x0b31jdkgoc9cjlc29gt9fdsm6v).
But after thinking about it I'd prefer to hide `-d` rather than remove for compatibility, then declare that the parameter is invalid and the new behavior.
--
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] Jason918 commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r824383724
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
> hide `-d` rather than remove for compatibility, then declare that this parameter is invalid and the new behavior.
Yes, I think it's better this way. Hide the parameter and print a warning log if user set this "-d false" explicitly.
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1064989214
/pulsarbot run-failure-checks
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1063042940
/pulsarbot run-failure-checks
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1083022767
@codelipenghui @315157973 @hangc0276 @Jason918 PTAL when you hava a moment, thx!
--
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] yuruguo removed a comment on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1065853658
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r824466387
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -632,14 +632,10 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
Review comment:
There is no such way of using `-d false` or `-d true`, the correct way is to specify `-d` or not to specify.
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833020129
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
##########
@@ -653,14 +659,14 @@ void run() throws Exception {
"--force" }, description = "Close all producer/consumer/replicator and delete topic forcefully")
private boolean force = false;
- @Parameter(names = { "-d",
- "--deleteSchema" }, description = "Delete schema while deleting topic")
+ @Parameter(names = {"-d", "--deleteSchema"}, description = "Delete schema while deleting topic, "
+ + "but the parameter is invalid and the schema is always deleted", hidden = true)
private boolean deleteSchema = false;
Review comment:
[same](https://github.com/apache/pulsar/pull/14608#discussion_r833000802)
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833632609
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -581,14 +582,6 @@ protected void internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
if (numPartitions < 1){
return CompletableFuture.completedFuture(null);
}
- if (deleteSchema) {
- return pulsar().getBrokerService()
- .deleteSchemaStorage(topicName.getPartition(0).toString())
- .thenCompose(unused ->
- internalRemovePartitionsAuthenticationPoliciesAsync(numPartitions))
- .thenCompose(unused2 ->
- internalRemovePartitionsTopicAsync(numPartitions, force));
- }
Review comment:
[L593](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-66aeb65a64cbe7c541f013ae807c5bcbeab567bef77706c7ff2e0cbfe0d77eb1L593) will delete schema by default.
--
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] yuruguo commented on a change in pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#discussion_r833632609
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -581,14 +582,6 @@ protected void internalDeletePartitionedTopic(AsyncResponse asyncResponse, boole
if (numPartitions < 1){
return CompletableFuture.completedFuture(null);
}
- if (deleteSchema) {
- return pulsar().getBrokerService()
- .deleteSchemaStorage(topicName.getPartition(0).toString())
- .thenCompose(unused ->
- internalRemovePartitionsAuthenticationPoliciesAsync(numPartitions))
- .thenCompose(unused2 ->
- internalRemovePartitionsTopicAsync(numPartitions, force));
- }
Review comment:
[internalRemovePartitionsTopicAsync(numPartitions, force)](https://github.com/apache/pulsar/pull/14608/files?diff=unified&w=0#diff-66aeb65a64cbe7c541f013ae807c5bcbeab567bef77706c7ff2e0cbfe0d77eb1L593) will delete schema by default.
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1064757069
@codelipenghui @hangc0276 @Technoboy- @nlu90 PTAL
--
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] yuruguo commented on pull request #14608: Ensure the deletion consistency of topic and schema
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #14608:
URL: https://github.com/apache/pulsar/pull/14608#issuecomment-1062608662
/pulsarbot run-failure-checks
--
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