pkuwm commented on a change in pull request #1119:
URL: https://github.com/apache/helix/pull/1119#discussion_r459109582
##########
File path:
zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
##########
@@ -553,6 +554,52 @@ public void testCreateEphemeralWithValidSession()
_zkClient.delete(path);
}
+ /*
+ * This test validates that when ZK_AUTOSYNC_ENABLED_DEFAULT is enabled,
sync() would be issued
+ * before handleNewSession. ZKclient would not see stale data.
+ */
+ @Test
+ public void testAutoSyncWithNewSessionEstablishment() throws Exception {
+ final String path = "/" + TestHelper.getTestMethodName();
+ final String data = "Hello Helix 2";
+
+ // Wait until the ZkClient has got a new session.
+ Assert.assertTrue(TestHelper
+ .verify(() ->
_zkClient.getConnection().getZookeeperState().isConnected(), 1000L));
+
+ final long originalSessionId = _zkClient.getSessionId();
+
+ try {
+ // Create node.
+ _zkClient.create(path, data, CreateMode.PERSISTENT);
+ } catch (Exception ex) {
+ Assert.fail("Failed to create ephemeral node.", ex);
+ }
+
+ // Expire the original session.
+ ZkTestHelper.expireSession(_zkClient);
+
+ // Wait until the ZkClient has got a new session.
+ Assert.assertTrue(TestHelper.verify(() -> {
+ try {
+ // New session id should not equal to expired session id.
+ return _zkClient.getSessionId() != originalSessionId;
+ } catch (ZkClientException ex) {
+ return false;
+ }
+ }, 1000L));
+
+ // Verify the node is created and its data is correct.
+ Stat stat = new Stat();
+ String nodeData = null;
+ try {
+ nodeData = _zkClient.readData(path, stat, true);
+ } catch (ZkException e) {
+ Assert.fail("fail to read data");
Review comment:
Actually this is no need. If exception is thrown here, test will fail.
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -105,6 +111,9 @@
// ZkEventThread. Otherwise the retry request might block the normal event
processing.
protected final ZkAsyncRetryThread _asyncCallRetryThread;
+ private final boolean _syncOnNewSession;
+ private static final String _syncPath = "/";
Review comment:
`_syncPath` -> `SYNC_PATH`?
##########
File path:
zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java
##########
@@ -553,6 +554,52 @@ public void testCreateEphemeralWithValidSession()
_zkClient.delete(path);
}
+ /*
+ * This test validates that when ZK_AUTOSYNC_ENABLED_DEFAULT is enabled,
sync() would be issued
+ * before handleNewSession. ZKclient would not see stale data.
+ */
+ @Test
+ public void testAutoSyncWithNewSessionEstablishment() throws Exception {
+ final String path = "/" + TestHelper.getTestMethodName();
+ final String data = "Hello Helix 2";
+
+ // Wait until the ZkClient has got a new session.
+ Assert.assertTrue(TestHelper
+ .verify(() ->
_zkClient.getConnection().getZookeeperState().isConnected(), 1000L));
+
+ final long originalSessionId = _zkClient.getSessionId();
+
+ try {
+ // Create node.
+ _zkClient.create(path, data, CreateMode.PERSISTENT);
+ } catch (Exception ex) {
+ Assert.fail("Failed to create ephemeral node.", ex);
Review comment:
Same. Actually this is no need. If exception is thrown here, test will
fail.
Realizing this will also help you with later tests implement.
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java
##########
@@ -120,6 +122,39 @@ public void handle() {
}
}
+ public static class SyncCallbackHandler extends DefaultCallback implements
AsyncCallback.VoidCallback {
+ private String _sessionId;
+
+ public SyncCallbackHandler(String sessionId) {
+ _sessionId = sessionId;
+ }
+
+ @Override
+ public void processResult(int rc, String path, Object ctx) {
+ LOG.info("sycnOnNewSession with sessionID {} async return code: {}",
_sessionId, rc);
+ callback(rc, path, ctx);
+ }
+
+ @Override
+ public void handle() {
+ // Make compiler happy, not used.
+ }
+
+ @Override
+ protected boolean needRetry(int rc) {
+ try {
+ // Connection to the server has been lost
+ if (KeeperException.Code.get(rc) == Code.CONNECTIONLOSS) {
+ return true;
+ }
+ return false;
+ } catch (ClassCastException | NullPointerException ex) {
Review comment:
I wonder how `NullPointerException` is thrown?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
