You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by eo...@apache.org on 2020/07/27 13:08:04 UTC
[bookkeeper] branch branch-4.11 updated: Track ZooKeeper errors as
causes of ZKException
This is an automated email from the ASF dual-hosted git repository.
eolivelli pushed a commit to branch branch-4.11
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/branch-4.11 by this push:
new dca3412 Track ZooKeeper errors as causes of ZKException
dca3412 is described below
commit dca3412f860da91a09b3527faf9be551288931f9
Author: Enrico Olivelli <eo...@gmail.com>
AuthorDate: Mon Jul 27 15:06:29 2020 +0200
Track ZooKeeper errors as causes of ZKException
Descriptions of the changes in this PR:
### Motivation
Every time a problem with ZK arises you don't see it in the exception chain of the BKException (in this case ZKException) and you end up with errors like:
```
org.apache.bookkeeper.client.BKException$ZKException: Error while using ZooKeeper
at org.apache.bookkeeper.client.SyncCallbackUtils.finish(SyncCallbackUtils.java:83)
at org.apache.bookkeeper.client.SyncCallbackUtils$SyncAddCallback.addComplete(SyncCallbackUtils.java:251)
at org.apache.bookkeeper.client.AsyncCallback$AddCallback.addCompleteWithLatency(AsyncCallback.java:91)
at org.apache.bookkeeper.client.PendingAddOp.submitCallback(PendingAddOp.java:430)
at org.apache.bookkeeper.client.LedgerHandle.errorOutPendingAdds(LedgerHandle.java:1784)
at org.apache.bookkeeper.client.LedgerHandle$5.safeRun(LedgerHandle.java:574)
```
### Changes
Add a "cause" to every ZKException.
### Notes
There are very few places that cannot be fixed because they are still using the old callback based mechanism without CompletableFuture. Those points are not changed in order to make the patch simple but still useful.
Reviewers: Jia Zhai <zh...@apache.org>
This closes #2384 from eolivelli/fix/zkexception-chain
(cherry picked from commit b96a5a75403f3260b564ca8c5d5889af51fe9872)
Signed-off-by: Enrico Olivelli <en...@diennea.com>
---
.../org/apache/bookkeeper/client/BKException.java | 4 ++++
.../bookkeeper/discover/ZKRegistrationClient.java | 2 +-
.../bookkeeper/meta/AbstractZkLedgerManager.java | 22 +++++++++++++++-------
.../meta/AbstractZkLedgerManagerTest.java | 1 +
4 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
index a3f03c2..438dd95 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java
@@ -306,6 +306,10 @@ public abstract class BKException extends org.apache.bookkeeper.client.api.BKExc
public ZKException() {
super(BKException.Code.ZKException);
}
+
+ public ZKException(Throwable cause) {
+ super(BKException.Code.ZKException, cause);
+ }
}
/**
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
index 6995a14..367e9ac 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
@@ -289,7 +289,7 @@ public class ZKRegistrationClient implements RegistrationClient {
CompletableFuture<Versioned<Set<BookieSocketAddress>>> future = FutureUtils.createFuture();
zk.getChildren(regPath, watcher, (rc, path, ctx, children, stat) -> {
if (Code.OK != rc) {
- ZKException zke = new ZKException();
+ ZKException zke = new ZKException(KeeperException.create(KeeperException.Code.get(rc), path));
future.completeExceptionally(zke.fillInStackTrace());
return;
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
index 0b42c3d..8ecb9dd 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
@@ -295,7 +295,7 @@ public abstract class AbstractZkLedgerManager implements LedgerManager, Watcher
} else {
LOG.error("Could not validate node for ledger {} after LedgerExistsException", ledgerId,
exception);
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(new BKException.ZKException(exception));
}
return null;
});
@@ -306,7 +306,8 @@ public abstract class AbstractZkLedgerManager implements LedgerManager, Watcher
} else {
LOG.error("Could not create node for ledger {}", ledgerId,
KeeperException.create(Code.get(rc), path));
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(
+ new BKException.ZKException(KeeperException.create(Code.get(rc), path)));
}
}
};
@@ -365,7 +366,8 @@ public abstract class AbstractZkLedgerManager implements LedgerManager, Watcher
}
FutureUtils.complete(promise, null);
} else {
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(
+ new BKException.ZKException(KeeperException.create(Code.get(rc), path)));
}
}
};
@@ -444,12 +446,15 @@ public abstract class AbstractZkLedgerManager implements LedgerManager, Watcher
if (rc != KeeperException.Code.OK.intValue()) {
LOG.error("Could not read metadata for ledger: " + ledgerId,
KeeperException.create(KeeperException.Code.get(rc), path));
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(
+ new BKException.ZKException(KeeperException.create(Code.get(rc), path)));
return;
}
if (stat == null) {
LOG.error("Could not parse ledger metadata for ledger: {}. Stat object is null", ledgerId);
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(new BKException.ZKException(
+ new Exception("Could not parse ledger metadata for ledger: "
+ + ledgerId + " . Stat object is null").fillInStackTrace()));
return;
}
@@ -459,7 +464,9 @@ public abstract class AbstractZkLedgerManager implements LedgerManager, Watcher
promise.complete(new Versioned<>(metadata, version));
} catch (Throwable t) {
LOG.error("Could not parse ledger metadata for ledger: {}", ledgerId, t);
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(new BKException.ZKException(
+ new Exception("Could not parse ledger metadata for ledger: "
+ + ledgerId, t).fillInStackTrace()));
}
}
}, null);
@@ -498,7 +505,8 @@ public abstract class AbstractZkLedgerManager implements LedgerManager, Watcher
promise.completeExceptionally(new BKException.BKNoSuchLedgerExistsOnMetadataServerException());
} else {
LOG.warn("Conditional update ledger metadata failed: {}", KeeperException.Code.get(rc));
- promise.completeExceptionally(new BKException.ZKException());
+ promise.completeExceptionally(
+ new BKException.ZKException(KeeperException.create(Code.get(rc), path)));
}
}
}, null);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
index b54b7d9..e2c6a12 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/AbstractZkLedgerManagerTest.java
@@ -216,6 +216,7 @@ public class AbstractZkLedgerManagerTest extends MockZooKeeperTestCase {
assertTrue(e instanceof BKException);
BKException bke = (BKException) e;
assertEquals(Code.ZKException, bke.getCode());
+ assertTrue(bke.getCause() instanceof KeeperException);
}
}