aloyszhang commented on code in PR #18293:
URL: https://github.com/apache/pulsar/pull/18293#discussion_r1016441750


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/SchemaRegistryServiceImpl.java:
##########
@@ -180,57 +182,85 @@ public 
CompletableFuture<List<CompletableFuture<SchemaAndMetadata>>> getAllSchem
     @Override
     @NotNull
     public CompletableFuture<SchemaVersion> putSchemaIfAbsent(String schemaId, 
SchemaData schema,
-                                                              
SchemaCompatibilityStrategy strategy) {
+            SchemaCompatibilityStrategy strategy) {
+
+        return putSchemaIfAbsent(schemaId, schema, strategy, 0, 0);
+    }
+
+    @NotNull
+    public CompletableFuture<SchemaVersion> putSchemaIfAbsent(String schemaId, 
SchemaData schema,
+            SchemaCompatibilityStrategy strategy, long startTime, int retry) {
         return 
trimDeletedSchemaAndGetList(schemaId).thenCompose(schemaAndMetadataList ->
                 getSchemaVersionBySchemaData(schemaAndMetadataList, 
schema).thenCompose(schemaVersion -> {
-            if (schemaVersion != null) {
-                if (log.isDebugEnabled()) {
-                    log.debug("[{}] Schema is already exists", schemaId);
-                }
-                return CompletableFuture.completedFuture(schemaVersion);
-            }
-            CompletableFuture<Void> checkCompatibilityFuture = new 
CompletableFuture<>();
-            if (schemaAndMetadataList.size() != 0) {
-                if (isTransitiveStrategy(strategy)) {
-                    checkCompatibilityFuture =
-                            checkCompatibilityWithAll(schemaId, schema, 
strategy, schemaAndMetadataList);
-                } else {
-                    checkCompatibilityFuture = 
checkCompatibilityWithLatest(schemaId, schema, strategy);
-                }
-            } else {
-                checkCompatibilityFuture.complete(null);
-            }
-            return checkCompatibilityFuture.thenCompose(v -> {
-                byte[] context = 
hashFunction.hashBytes(schema.getData()).asBytes();
-                SchemaRegistryFormat.SchemaInfo info = 
SchemaRegistryFormat.SchemaInfo.newBuilder()
-                        
.setType(Functions.convertFromDomainType(schema.getType()))
-                        .setSchema(ByteString.copyFrom(schema.getData()))
-                        .setSchemaId(schemaId)
-                        .setUser(schema.getUser())
-                        .setDeleted(false)
-                        .setTimestamp(clock.millis())
-                        .addAllProps(toPairs(schema.getProps()))
-                        .build();
-
-                long start = this.clock.millis();
-
-                return schemaStorage
-                        .put(schemaId, info.toByteArray(), context)
-                        .whenComplete((__, t) -> {
-                            if (t != null) {
-                                log.error("[{}] Put schema failed", schemaId);
-                                this.stats.recordPutFailed(schemaId);
-                            } else {
-                                if (log.isDebugEnabled()) {
-                                    log.debug("[{}] Put schema finished", 
schemaId);
-                                }
-                                this.stats.recordPutLatency(schemaId, 
this.clock.millis() - start);
-                            }
-                        });
-
-            });
-
-        }));
+                    if (schemaVersion != null) {
+                        if (log.isDebugEnabled()) {
+                            log.debug("[{}] Schema is already exists", 
schemaId);
+                        }
+                        return 
CompletableFuture.completedFuture(schemaVersion);
+                    }
+                    CompletableFuture<Void> checkCompatibilityFuture = new 
CompletableFuture<>();
+                    if (schemaAndMetadataList.size() != 0) {
+                        if (isTransitiveStrategy(strategy)) {
+                            checkCompatibilityFuture = 
checkCompatibilityWithAll(schemaId, schema,
+                                    strategy, schemaAndMetadataList);
+                        } else {
+                            checkCompatibilityFuture = 
checkCompatibilityWithLatest(schemaId,
+                                    schema, strategy);
+                        }
+                    } else {
+                        checkCompatibilityFuture.complete(null);
+                    }
+                    return checkCompatibilityFuture.thenCompose(v -> {
+                        byte[] context = 
hashFunction.hashBytes(schema.getData()).asBytes();
+                        SchemaRegistryFormat.SchemaInfo info = 
SchemaRegistryFormat.SchemaInfo
+                                .newBuilder()
+                                
.setType(Functions.convertFromDomainType(schema.getType()))
+                                
.setSchema(ByteString.copyFrom(schema.getData()))
+                                .setSchemaId(schemaId)
+                                .setUser(schema.getUser())
+                                .setDeleted(false)
+                                .setTimestamp(clock.millis())
+                                .addAllProps(toPairs(schema.getProps()))
+                                .build();
+                        long start = startTime > 0 ? startTime : 
this.clock.millis();
+                        CompletableFuture<SchemaVersion> persistentFuture = 
new CompletableFuture<>();
+                        schemaStorage
+                                .put(schemaId, info.toByteArray(), context)
+                                .thenAccept(sv -> {
+                                    if (log.isDebugEnabled()) {
+                                        log.debug("[{}] Schema is successfully 
added", schemaId);
+                                    }
+                                    this.stats.recordPutLatency(schemaId, 
this.clock.millis() - start);
+                                    persistentFuture.complete(sv);
+                                })
+                                .exceptionally(t -> {
+                                    if (t.getCause() instanceof 
AlreadyExistsException
+                                            || t.getCause() instanceof 
BadVersionException) {
+                                        // retry if put schemaLocator to zk 
failed caused by race condition
+                                        int retryTime = retry > 0 ? retry + 1 
: 1;
+                                        if (log.isDebugEnabled()) {
+                                            log.debug("[{}] Put schema failed 
because of {}, retry {} times",
+                                                    t.getCause().getMessage(), 
schemaId, retryTime);
+                                        }
+                                        this.stats.recordPutRetry(schemaId);
+                                        putSchemaIfAbsent(schemaId, schema, 
strategy, start, retryTime)
+                                                
.thenAccept(persistentFuture::complete)
+                                                .exceptionally(ex2 -> {
+                                                    
persistentFuture.completeExceptionally(ex2);
+                                                    return null;
+                                                });
+
+                                    } else {
+                                        log.error("[{}] Put schema failed", 
schemaId, t.getCause());
+                                        this.stats.recordPutFailed(schemaId);
+                                        
persistentFuture.completeExceptionally(t);
+                                    }
+                                    return null;
+                                });

Review Comment:
   I'll try this, thanks for your suggestion



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

Reply via email to