jiajunwang commented on a change in pull request #970:
URL: https://github.com/apache/helix/pull/970#discussion_r418732749
##########
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:
@alirezazamani I understand your concern. But there are many many ways
to make it happen. I won't be able to comment about every case. Moreover,
strictly speaking, this final check is not like an NPE check, this is not a
failure case. This is just meaning that the caller does not want a retry. And
this is not something we have to avoid, if logic requires, we shall still do
it. It is just not happening, and a little bit strang for the current logic.
But we shall not discourage people to use it in a different way in the future.
At that moment, if this log becomes too verbose, we can downgrade it or remove
it.
----------------------------------------------------------------
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]
