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


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/BookkeeperSchemaStorage.java:
##########
@@ -126,33 +128,54 @@ public CompletableFuture<StoredSchema> get(String key, 
SchemaVersion version) {
 
     @Override
     public CompletableFuture<List<CompletableFuture<StoredSchema>>> 
getAll(String key) {
+        return getAll(key, null);
+    }
+
+    public CompletableFuture<List<CompletableFuture<StoredSchema>>> 
getAll(String key,
+            LocatorEntry locator) {

Review Comment:
   ```suggestion
       public CompletableFuture<List<CompletableFuture<StoredSchema>>> 
getAll(String key, LocatorEntry locator) {
   ```



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/SchemaRegistryServiceImpl.java:
##########
@@ -180,57 +189,101 @@ public 
CompletableFuture<List<CompletableFuture<SchemaAndMetadata>>> getAllSchem
     @Override
     @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);
-                            }
-                        });
+            SchemaCompatibilityStrategy strategy) {
+
+        return putSchemaIfAbsent(schemaId, schema, strategy, 0, 0);
+    }
 
+    @NotNull
+    public CompletableFuture<SchemaVersion> putSchemaIfAbsent(String schemaId, 
SchemaData schema,
+            SchemaCompatibilityStrategy strategy, long startTime, int retry) {
+        BookkeeperSchemaStorage bkSchemaStorage = (BookkeeperSchemaStorage) 
schemaStorage;

Review Comment:
   The `schemaRegistryStorageClassName` is configurable. Users can have their 
implementations. 
   
   ```
   ### --- Schema storage --- ###
   # The schema storage implementation used by this broker
   
schemaRegistryStorageClassName=org.apache.pulsar.broker.service.schema.BookkeeperSchemaStorageFactory
   ```



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/BookkeeperSchemaStorage.java:
##########
@@ -126,33 +128,54 @@ public CompletableFuture<StoredSchema> get(String key, 
SchemaVersion version) {
 
     @Override
     public CompletableFuture<List<CompletableFuture<StoredSchema>>> 
getAll(String key) {
+        return getAll(key, null);
+    }
+
+    public CompletableFuture<List<CompletableFuture<StoredSchema>>> 
getAll(String key,
+            LocatorEntry locator) {
         CompletableFuture<List<CompletableFuture<StoredSchema>>> result = new 
CompletableFuture<>();
-        getLocator(key).thenAccept(locator -> {
-            if (log.isDebugEnabled()) {
-                log.debug("[{}] Get all schemas - locator: {}", key, locator);
-            }
 
-            if (!locator.isPresent()) {
-                result.complete(Collections.emptyList());
-                return;
-            }
+        if (log.isDebugEnabled()) {
+            log.debug("[{}] Get all schemas - locator: {}", key, locator);
+        }
+        List<CompletableFuture<StoredSchema>> list = new ArrayList<>();
+        if (locator == null) {

Review Comment:
   We can just add 
   
   ```java
   CompletableFuture<Optional<LocatorEntry>> locatorFuture = 
                   locator == null ? getLocator(key) : 
CompletableFuture.completedFuture(Optional.of(locator));
   locatorFuture.thenAccept(...)
   ```
   to reduce the duplicated code.



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