utafrali commented on code in PR #25483:
URL: https://github.com/apache/pulsar/pull/25483#discussion_r3048668475
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/SchemaRegistryServiceImpl.java:
##########
@@ -230,7 +230,7 @@ public CompletableFuture<SchemaVersion>
putSchemaIfAbsent(String schemaId, Schem
}))).whenComplete((v, ex) -> {
var latencyMs = this.clock.millis() - start.longValue();
if (ex != null) {
- log.error("[{}] Put schema failed", schemaId, ex);
+ log.warn("[{}] Put schema failed", schemaId, ex);
Review Comment:
This `whenComplete` catches ALL exceptions from `putSchemaIfAbsent`, not
just `IncompatibleSchemaException`. Downgrading the whole handler to WARN means
that genuine server-side failures (ZooKeeper errors, storage I/O failures,
etc.) will also be silently logged at WARN, which makes them much harder to
detect in production.
The two changes at lines 461 and 498 are correct since those handlers are
exclusively triggered by `IncompatibleSchemaException`. But here the right fix
is to distinguish by exception type:
```java
if (ex instanceof IncompatibleSchemaException) {
log.warn("[{}] Put schema failed due to incompatible schema", schemaId,
ex);
} else {
log.error("[{}] Put schema failed", schemaId, ex);
}
```
This keeps WARN for the expected client-caused case while retaining ERROR
for unexpected server-side failures.
--
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]
