jiajunwang commented on a change in pull request #1540:
URL: https://github.com/apache/helix/pull/1540#discussion_r526532222
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -340,26 +340,29 @@ 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);
- }
-
+ // TODO: Having subscribeForChanges in critical section might be overkill.
+ // Maybe it could be moved out later.
+ synchronized (this) {
Review comment:
I think you already answered the question yourself.
----------------------------------------------------------------
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]
