dasahcc commented on a change in pull request #970:
URL: https://github.com/apache/helix/pull/970#discussion_r416922187
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -1758,50 +1776,66 @@ public void asyncSetData(final String path, Object
datat, final int version,
data = serialize(datat, path);
} catch (ZkMarshallingError e) {
cb.processResult(KeeperException.Code.MARSHALLINGERROR.intValue(), path,
- new ZkAsyncCallbacks.ZkAsyncCallContext(_monitor, startT, 0, false),
null);
+ new ZkAsyncCallMonitorContext(_monitor, startT, 0, false), null);
return;
}
+ doAsyncSetData(path, data, version, startT, cb);
+ }
+
+ private void doAsyncSetData(final String path, byte[] data, final int
version, final long startT,
+ final ZkAsyncCallbacks.SetDataCallbackHandler cb) {
retryUntilConnected(() -> {
((ZkConnection) getConnection()).getZookeeper().setData(path, data,
version, cb,
- new ZkAsyncCallbacks.ZkAsyncCallContext(_monitor, startT,
- data == null ? 0 : data.length, false));
+ new ZkAsyncRetryCallContext(_asyncCallRetryThread, cb, _monitor,
startT,
+ data == null ? 0 : data.length, false) {
+ @Override
+ protected void doRetry() {
+ doAsyncSetData(path, data, version, System.currentTimeMillis(),
cb);
Review comment:
Is this recursively self calling OK?
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java
##########
@@ -122,44 +121,66 @@ public void handle() {
}
/**
- * Default callback for zookeeper async api
+ * Default callback for zookeeper async api.
*/
- public static abstract class DefaultCallback {
- AtomicBoolean _lock = new AtomicBoolean(false);
- int _rc = -1;
+ public static abstract class DefaultCallback implements
CancellableZkAsyncCallback {
+ AtomicBoolean _isOperationDone = new AtomicBoolean(false);
+ int _rc = UNKNOWN_RET_CODE;
public void callback(int rc, String path, Object ctx) {
if (rc != 0 && LOG.isDebugEnabled()) {
LOG.debug(this + ", rc:" + Code.get(rc) + ", path: " + path);
}
- if (ctx != null && ctx instanceof ZkAsyncCallContext) {
- ZkAsyncCallContext zkCtx = (ZkAsyncCallContext) ctx;
- if (zkCtx._monitor != null) {
- if (zkCtx._isRead) {
- zkCtx._monitor.record(path, zkCtx._bytes, zkCtx._startTimeMilliSec,
- ZkClientMonitor.AccessType.READ);
- } else {
- zkCtx._monitor.record(path, zkCtx._bytes, zkCtx._startTimeMilliSec,
- ZkClientMonitor.AccessType.WRITE);
- }
- }
+ if (ctx != null && ctx instanceof ZkAsyncCallMonitorContext) {
+ ((ZkAsyncCallMonitorContext) ctx).recordAccess(path);
}
_rc = rc;
- handle();
- synchronized (_lock) {
- _lock.set(true);
- _lock.notify();
+ // If retry is requested by passing the retry callback context, do retry
if necessary.
+ if (needRetry(rc)) {
+ if (ctx != null && ctx instanceof ZkAsyncRetryCallContext) {
+ try {
+ if (((ZkAsyncRetryCallContext) ctx).requestRetry()) {
+ // The retry operation will be done asynchronously. Once it is
done, the same callback
+ // handler object shall be triggered to ensure the result is
notified to the right
+ // caller(s).
+ return;
+ } else {
+ LOG.warn(
+ "Cannot request to retry the operation. The retry request
thread may have been stopped.");
+ }
+ } catch (Throwable t) {
+ LOG.error("Failed to request to retry the operation.", t);
+ }
+ } else {
+ LOG.warn(
Review comment:
Would this be to many introduced for log?
----------------------------------------------------------------
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]
