jiajunwang commented on a change in pull request #1119:
URL: https://github.com/apache/helix/pull/1119#discussion_r459114542
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -1221,21 +1234,84 @@ private void reconnect() {
}
}
+
+
+ private void doAsyncSync(final ZooKeeper zk, final String path, final long
startT,
+ final ZkAsyncCallbacks.SyncCallbackHandler cb) {
+ zk.sync(path, cb,
+ new ZkAsyncRetryCallContext(_asyncCallRetryThread, cb, _monitor,
startT, 0, true) {
+ @Override
+ protected void doRetry() throws Exception {
+ doAsyncSync(zk, path, System.currentTimeMillis(), cb);
+ }
+ });
+ }
+
+ /*
+ * Note, issueSync takes a ZooKeeper (client) object and pass it to
doAsyncSync().
+ * The reason we do this is that we want to ensure each new session event
is preceded with exactly
+ * one sync() to server. The sync() is to make sure the server would not
see stale data.
+ *
+ * ZooKeeper client object has an invariant of each object has one session.
With this invariant
+ * we can achieve each one sync() to server upon new session establishment.
The reasoning is:
+ * issueSync() is called when fireNewSessionEvents() which in under
eventLock of ZkClient. Thus
+ * we are guaranteed the ZooKeeper object passed in would have the new
incoming sessionId. If by
+ * the time sync() is invoked, the session expires. The sync() would fail
with a stale session.
+ * This is exactly what we want. The newer session would ensure another
fireNewSessionEvents.
+ */
+ private boolean issueSync(String sessionId, ZooKeeper zk) throws
ZkInterruptedException {
Review comment:
I still think Zookeeper is not required here. Please see my reply to the
original thread. If we put the check logic here, it will work.
The reason I insist on doing this is that, when you put the old zk object
here, if any new session created, then the old zk object will throw exceptions
when you call it. That would be a ZK exception. And then we rely on our
exception handling logic to ensure everything works fine.
If we check the current zk object with the session Id, then we can end this
operation with explicit logic and with a very clean warning or error log.
Moreover, as I mentioned in the first comment, the session Id and the input
zk contain redundant information. You only need one of them.
----------------------------------------------------------------
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]
