kaisun2000 commented on a change in pull request #1540:
URL: https://github.com/apache/helix/pull/1540#discussion_r526505380
##########
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:
why sync on _manager?
by the same argument as sync(this) above, it seems that there is no across
_manager race condition, aside from ZK session change. Or am I missing
something?
----------------------------------------------------------------
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]
