xyuanlu commented on a change in pull request #1540:
URL: https://github.com/apache/helix/pull/1540#discussion_r526506355
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -340,26 +340,28 @@ public void enqueueTask(NotificationContext
changeContext) throws Exception {
public void invoke(NotificationContext changeContext) throws Exception {
Type type = changeContext.getType();
long start = System.currentTimeMillis();
+ if (logger.isInfoEnabled()) {
+ logger.info("{} START: CallbackHandler {}, INVOKE {} listener: {} type:
{}",
+ Thread.currentThread().getId(), _uid, _path, _listener, type);
+ }
- // This allows the listener to work with one change at a time
- synchronized (_manager) {
- if (logger.isInfoEnabled()) {
- logger
- .info("{} START: CallbackHandler {}, INVOKE {} listener: {} type:
{}", Thread.currentThread().getId(),
- _uid, _path, _listener, type);
- }
-
+ synchronized (this) {
if (!_expectTypes.contains(type)) {
logger.warn("Callback handler {} received event in wrong order.
Listener: {}, path: {}, "
+ "expected types: {}, but was {}", _uid, _listener, _path,
_expectTypes, type);
return;
-
}
_expectTypes = nextNotificationType.get(type);
if (type == Type.INIT || type == Type.FINALIZE ||
changeContext.getIsChildChange()) {
subscribeForChanges(changeContext.getType(), _path, _watchChild);
}
+ }
+
+ // This allows the Helix Manager to work with one change at a time
+ // TODO: Maybe we don't need to sync on _manager for all types of
listener. PCould be a
+ // potential improvement candidate.
+ synchronized (_manager) {
Review comment:
Yes In my understanding, I think it is possible that we don't need to
make all different onXXXChange to be sequential. So this `TODO` is added..
----------------------------------------------------------------
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]
