xyuanlu commented on a change in pull request #1540:
URL: https://github.com/apache/helix/pull/1540#discussion_r526505856
##########
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);
- }
-
+ // TODO: Having subscribeForChanges here might be overkill. Maybe it could
be moved out later.
Review comment:
I guess it is possible for callBackHandler gets `Finalize` and a
`callBack` at the same time. So the type checking and the subscribe should be
in the same synced block..?
----------------------------------------------------------------
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]
