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


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/SchemaRegistryServiceImpl.java:
##########
@@ -181,56 +183,85 @@ public 
CompletableFuture<List<CompletableFuture<SchemaAndMetadata>>> getAllSchem
     @NotNull
     public CompletableFuture<SchemaVersion> putSchemaIfAbsent(String schemaId, 
SchemaData schema,
                                                               
SchemaCompatibilityStrategy strategy) {
-        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);
-                            }
-                        });
-
-            });
-
-        }));
+        CompletableFuture<SchemaVersion> persistentFuture = new 
CompletableFuture<>();
+        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();
+                                        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 != null) {
+                                                        if (t.getCause() 
instanceof AlreadyExistsException
+                                                                || 
t.getCause() instanceof BadVersionException) {
+                                                            // retry if put 
schemaLocator to zk failed caused by
+                                                            // race condition
+                                                            if 
(log.isDebugEnabled()) {
+                                                                
log.debug("[{}] Put schema failed because of {}, retry",
+                                                                        
t.getCause().getMessage(), schemaId);
+                                                            }
+                                                            
putSchemaIfAbsent(schemaId, schema, strategy)

Review Comment:
   +1



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