This is an automated email from the ASF dual-hosted git repository.
jiajunwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git
The following commit(s) were added to refs/heads/master by this push:
new 1a37d5c Get rid of non-official ZK error code to avoid NPEs. (#1778)
1a37d5c is described below
commit 1a37d5c692c85db20d0c61e2f72659f3724e125e
Author: Jiajun Wang <[email protected]>
AuthorDate: Wed Jun 2 16:13:22 2021 -0700
Get rid of non-official ZK error code to avoid NPEs. (#1778)
This PR removes the non-official ZK error codes that are returned by the
Helix ZkClient. The original design was made for differentiating the failure
cases. However, that design caused unexpected NPE when most of Helix's logic
relies on the Zookeeper KeeperException.Code class to identify the error types.
---
.../apache/helix/zookeeper/zkclient/ZkClient.java | 22 ++++++---------------
.../zkclient/callback/ZkAsyncCallbacks.java | 13 +-----------
.../zookeeper/impl/client/TestRawZkClient.java | 4 ++--
.../impl/client/TestZkClientAsyncRetry.java | 23 +++++++++++-----------
4 files changed, 20 insertions(+), 42 deletions(-)
diff --git
a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
index 8b742da..cbbdbbd 100644
---
a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
+++
b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
@@ -1276,7 +1276,7 @@ public class ZkClient implements Watcher {
});
} catch (RuntimeException e) {
// Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.UNKNOWN_RET_CODE, path,
+ cb.processResult(KeeperException.Code.APIERROR.intValue(), path,
new ZkAsyncCallMonitorContext(_monitor, startT, 0, true));
throw e;
}
@@ -1960,14 +1960,9 @@ public class ZkClient implements Watcher {
});
return null;
});
- } catch (ZkSessionMismatchedException e) {
- // Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.ZK_SESSION_MISMATCHED_CODE, path,
- new ZkAsyncCallMonitorContext(_monitor, startT, 0, false), null);
- throw e;
} catch (RuntimeException e) {
// Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.UNKNOWN_RET_CODE, path,
+ cb.processResult(KeeperException.Code.APIERROR.intValue(), path,
new ZkAsyncCallMonitorContext(_monitor, startT, 0, false), null);
throw e;
}
@@ -2003,14 +1998,9 @@ public class ZkClient implements Watcher {
});
return null;
});
- } catch (ZkSessionMismatchedException e) {
- // Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.ZK_SESSION_MISMATCHED_CODE, path,
- new ZkAsyncCallMonitorContext(_monitor, startT, 0, false), null);
- throw e;
} catch (RuntimeException e) {
// Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.UNKNOWN_RET_CODE, path,
+ cb.processResult(KeeperException.Code.APIERROR.intValue(), path,
new ZkAsyncCallMonitorContext(_monitor, startT, 0, false), null);
throw e;
}
@@ -2031,7 +2021,7 @@ public class ZkClient implements Watcher {
});
} catch (RuntimeException e) {
// Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.UNKNOWN_RET_CODE, path,
+ cb.processResult(KeeperException.Code.APIERROR.intValue(), path,
new ZkAsyncCallMonitorContext(_monitor, startT, 0, true), null,
null);
throw e;
}
@@ -2052,7 +2042,7 @@ public class ZkClient implements Watcher {
});
} catch (RuntimeException e) {
// Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.UNKNOWN_RET_CODE, path,
+ cb.processResult(KeeperException.Code.APIERROR.intValue(), path,
new ZkAsyncCallMonitorContext(_monitor, startT, 0, true), null);
throw e;
}
@@ -2073,7 +2063,7 @@ public class ZkClient implements Watcher {
});
} catch (RuntimeException e) {
// Process callback to release caller from waiting
- cb.processResult(ZkAsyncCallbacks.UNKNOWN_RET_CODE, path,
+ cb.processResult(KeeperException.Code.APIERROR.intValue(), path,
new ZkAsyncCallMonitorContext(_monitor, startT, 0, false));
throw e;
}
diff --git
a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java
b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java
index c188e7e..7f662cf 100644
---
a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java
+++
b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java
@@ -35,13 +35,6 @@ import org.slf4j.LoggerFactory;
public class ZkAsyncCallbacks {
private static Logger LOG = LoggerFactory.getLogger(ZkAsyncCallbacks.class);
- public static final int UNKNOWN_RET_CODE = 255;
-
- // Whenever there's a mismatch between the expected Zookeeper session ID and
the actual
- // Zookeeper session ID for the async operation, the ZkClient which performs
the async
- // operation passes this return code to the async callback indicate that it
has caught
- // a ZkSessionMismatchedException.
- public static final int ZK_SESSION_MISMATCHED_CODE = 127;
public static class GetDataCallbackHandler extends DefaultCallback
implements DataCallback {
public byte[] _data;
@@ -182,7 +175,7 @@ public class ZkAsyncCallbacks {
*/
public static abstract class DefaultCallback implements
CancellableZkAsyncCallback {
AtomicBoolean _isOperationDone = new AtomicBoolean(false);
- int _rc = UNKNOWN_RET_CODE;
+ int _rc = KeeperException.Code.APIERROR.intValue();
public void callback(int rc, String path, Object ctx) {
if (rc != 0 && LOG.isDebugEnabled()) {
@@ -282,10 +275,6 @@ public class ZkAsyncCallbacks {
* @return true if the error is transient and the operation may succeed
when being retried.
*/
protected boolean needRetry(int rc) {
- if (rc == ZK_SESSION_MISMATCHED_CODE) {
- LOG.error("Actual session ID doesn't match with expected session ID.
Skip retrying.");
- return false;
- }
try {
switch (Code.get(rc)) {
/** Connection to the server has been lost */
diff --git
a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
index e078367..79efd12 100644
---
a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
+++
b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
@@ -887,7 +887,7 @@ public class TestRawZkClient extends ZkTestBase {
// Ensure the async callback is cancelled because of the exception
Assert.assertTrue(createCallback.waitForSuccess(), "Callback operation
should be done");
- Assert.assertEquals(createCallback.getRc(),
ZkAsyncCallbacks.ZK_SESSION_MISMATCHED_CODE);
+ Assert.assertEquals(createCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
}
Assert.assertFalse(zkClient.exists(path));
@@ -915,7 +915,7 @@ public class TestRawZkClient extends ZkTestBase {
// Ensure the async callback is cancelled because of the exception
Assert.assertTrue(setDataCallback.waitForSuccess(), "Callback operation
should be done");
- Assert.assertEquals(setDataCallback.getRc(),
ZkAsyncCallbacks.ZK_SESSION_MISMATCHED_CODE);
+ Assert.assertEquals(setDataCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
}
TestHelper.verify(() -> zkClient.delete(path), TestHelper.WAIT_DURATION);
diff --git
a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestZkClientAsyncRetry.java
b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestZkClientAsyncRetry.java
index 6845aa9..e55f8e6 100644
---
a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestZkClientAsyncRetry.java
+++
b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestZkClientAsyncRetry.java
@@ -42,7 +42,6 @@ import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
-import static
org.apache.helix.zookeeper.zkclient.callback.ZkAsyncCallbacks.UNKNOWN_RET_CODE;
import static org.apache.zookeeper.KeeperException.Code.CONNECTIONLOSS;
/**
@@ -143,7 +142,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
}
ZkAsyncCallbacks.CreateCallbackHandler createCallback =
new ZkAsyncCallbacks.CreateCallbackHandler();
- Assert.assertEquals(createCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(createCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
testZkClient.setAsyncCallRC(code.intValue());
if (code == CONNECTIONLOSS || code ==
KeeperException.Code.SESSIONEXPIRED
|| code == KeeperException.Code.SESSIONMOVED) {
@@ -190,7 +189,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// 1. Test async set retry
ZkAsyncCallbacks.SetDataCallbackHandler setCallback =
new ZkAsyncCallbacks.SetDataCallbackHandler();
- Assert.assertEquals(setCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(setCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
tmpRecord.setSimpleField("test", "data");
testZkClient.setAsyncCallRC(CONNECTIONLOSS.intValue());
@@ -214,7 +213,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// 2. Test async delete
ZkAsyncCallbacks.DeleteCallbackHandler deleteCallback =
new ZkAsyncCallbacks.DeleteCallbackHandler();
- Assert.assertEquals(deleteCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(deleteCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
testZkClient.setAsyncCallRC(CONNECTIONLOSS.intValue());
// Async delete will be pending due to the mock error rc is retryable.
@@ -254,7 +253,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// 1. Test async create retry
ZkAsyncCallbacks.CreateCallbackHandler createCallback =
new ZkAsyncCallbacks.CreateCallbackHandler();
- Assert.assertEquals(createCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(createCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
tmpRecord.setSimpleField("test", "data");
testZkClient.setAsyncCallRC(CONNECTIONLOSS.intValue());
@@ -280,7 +279,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// 1. Test async set retry
ZkAsyncCallbacks.SetDataCallbackHandler setCallback =
new ZkAsyncCallbacks.SetDataCallbackHandler();
- Assert.assertEquals(setCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(setCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
tmpRecord.setSimpleField("test", "data");
testZkClient.setAsyncCallRC(CONNECTIONLOSS.intValue());
@@ -317,7 +316,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// 1. Test async exist check
ZkAsyncCallbacks.ExistsCallbackHandler existsCallback =
new ZkAsyncCallbacks.ExistsCallbackHandler();
- Assert.assertEquals(existsCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(existsCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
testZkClient.setAsyncCallRC(CONNECTIONLOSS.intValue());
// Async exist check will be pending due to the mock error rc is
retryable.
@@ -339,7 +338,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// 2. Test async get
ZkAsyncCallbacks.GetDataCallbackHandler getCallback =
new ZkAsyncCallbacks.GetDataCallbackHandler();
- Assert.assertEquals(getCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(getCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
testZkClient.setAsyncCallRC(CONNECTIONLOSS.intValue());
// Async get will be pending due to the mock error rc is retryable.
@@ -423,7 +422,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// Test asyncGet failure
ZkAsyncCallbacks.GetDataCallbackHandler getCallback =
new ZkAsyncCallbacks.GetDataCallbackHandler();
- Assert.assertEquals(getCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(getCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
// asyncGet should fail because the return code is APIERROR
testZkClient.setAsyncCallRC(KeeperException.Code.APIERROR.intValue());
testZkClient.asyncGetData(NODE_PATH, getCallback);
@@ -445,7 +444,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// Test asyncExists failure
ZkAsyncCallbacks.ExistsCallbackHandler existsCallback =
new ZkAsyncCallbacks.ExistsCallbackHandler();
- Assert.assertEquals(existsCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(existsCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
// asyncExists should fail because the return code is APIERROR
testZkClient.setAsyncCallRC(KeeperException.Code.APIERROR.intValue());
testZkClient.asyncExists(NODE_PATH, existsCallback);
@@ -467,7 +466,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// Test asyncSet failure
ZkAsyncCallbacks.SetDataCallbackHandler setCallback =
new ZkAsyncCallbacks.SetDataCallbackHandler();
- Assert.assertEquals(setCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(setCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
// asyncSet should fail because the return code is APIERROR
testZkClient.setAsyncCallRC(KeeperException.Code.APIERROR.intValue());
testZkClient.asyncSetData(NODE_PATH, tmpRecord, -1, setCallback);
@@ -481,7 +480,7 @@ public class TestZkClientAsyncRetry extends ZkTestBase {
// Test asyncDelete failure
ZkAsyncCallbacks.DeleteCallbackHandler deleteCallback =
new ZkAsyncCallbacks.DeleteCallbackHandler();
- Assert.assertEquals(deleteCallback.getRc(), UNKNOWN_RET_CODE);
+ Assert.assertEquals(deleteCallback.getRc(),
KeeperException.Code.APIERROR.intValue());
// asyncDelete should fail because the return code is APIERROR
testZkClient.setAsyncCallRC(KeeperException.Code.APIERROR.intValue());
testZkClient.asyncDelete(NODE_PATH, deleteCallback);
