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 2021/08/10 05:55:04 UTC

[GitHub] [pulsar] eolivelli commented on a change in pull request #11606: Fix race condition in concurrent schema deletion

eolivelli commented on a change in pull request #11606:
URL: https://github.com/apache/pulsar/pull/11606#discussion_r685710425



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -372,6 +374,21 @@ public String getReplicatorPrefix() {
         String id = TopicName.get(base).getSchemaName();
         SchemaRegistryService schemaRegistryService = brokerService.pulsar().getSchemaRegistryService();
         return schemaRegistryService.getSchema(id)
+                .exceptionally(t -> {
+                    if (t.getCause() != null

Review comment:
       This condition looks a little bit hacky.
   1) we are only checking `t.getCause()`, IMHO we should have some utility method that traverses the full exception chain, looking for a SchemaException
   2) a "non recoverable" SchemaException is not enough to say that this is a "Schema does not exist" exception, can we introduce some specific SchemaException subclass or method like `isRecoverable()` ?

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/BookkeeperSchemaStorage.java
##########
@@ -370,7 +377,21 @@ public void close() throws Exception {
 
     @NotNull
     private CompletableFuture<Long> deleteSchema(String schemaId, boolean forceFully) {
-        return (forceFully ? CompletableFuture.completedFuture(null) : getSchema(schemaId))
+        return (forceFully ? CompletableFuture.completedFuture(null) : getSchema(schemaId).exceptionally(t -> {
+            if (t.getCause() != null
+                    && (t.getCause() instanceof SchemaException)

Review comment:
       same as above, at least we should have one utility method to detect this case




-- 
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