gaoran10 commented on code in PR #18293:
URL: https://github.com/apache/pulsar/pull/18293#discussion_r1015212888
##########
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) {
Review Comment:
Could the exception be null?
##########
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)
+
.thenAccept(persistentFuture::complete)
+
.exceptionally(ex2 -> {
+
persistentFuture.completeExceptionally(ex2);
+ return
null;
+ });
+
+ } else {
+ log.error("[{}]
Put schema failed", schemaId);
Review Comment:
Maybe we can get the original error and print the stack, and it seems that
the `persistentFuture` should be completed exceptionally here.
--
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]
