dlg99 commented on code in PR #16664:
URL: https://github.com/apache/pulsar/pull/16664#discussion_r925031166
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/api/MetadataStoreException.java:
##########
@@ -61,15 +85,18 @@ public InvalidImplementationException(String msg) {
*/
public static class NotFoundException extends MetadataStoreException {
public NotFoundException() {
- super((Throwable) null);
+ super(makeBkFriendlyException(
Review Comment:
I changed this to be more lightweight (no new exception created, static
final exception set as a cause, traversing exception chain is fast).
There are reasons why decided to take this route:
With all Java's love for checked exceptions, CompletableFuture in the API
can be completed with any exception, hence BK's API implemented in Pulsar
returns exceptions that BK cannot handle properly. So there is no way for
compiler to strictly enforce API contract that suits BK.
As result, removeLedgerMetadata() just returns whatever exception
store.delete() produces etc.
While I can remap the exception there into BK-specific, it can break some
Pulsar code (like the callbacks that rely on ex.getCause().getCause() being
MetadataStoreException). I'd very much prefer not to go through all code base
tracking all possible gotchas as I cannot guarantee that tests will all the
cases.
Alternatively I'd have to inject cause the same way I do now but with more
steps.
Plus there is MetadataStoreException.unwrap which recreates exception with
the message / without the original exception.
Current approach communicates appropriate error to BK so we are no longer
getting UnexpectedConditionException in obvious cases and can properly handle
basic errors, does not add overhead (again, traversing exception chain is
fast), fool-proof enough so we don't have to worry about breaking it all in
case LedgerManager is extended or changed, and it does not add mental overhead
for Pulsar developers (no need to think about BK errors.
let me know if I am missing something obvious / other way to make this work.
--
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]
