You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/08/20 22:54:45 UTC
[GitHub] [helix] mgao0 opened a new issue #1296: Router completely reset when receives FINALIZE event
mgao0 opened a new issue #1296:
URL: https://github.com/apache/helix/issues/1296
### Describe the bug
When handle child changes in CallbackHandler, under certain condition we will remove the listener.
@Override
public void handleChildChange(String parentPath, List<String> currentChilds) {
if (logger.isDebugEnabled()) {
logger.debug("Data change callback: child changed, path: {} , current child count: {}",
parentPath, currentChilds == null ? 0 : currentChilds.size());
}
try {
updateNotificationTime(System.nanoTime());
if (parentPath != null && parentPath.startsWith(_path)) {
if (currentChilds == null && parentPath.equals(_path)) {
// _path has been removed, remove this listener
// removeListener will call handler.reset(), which in turn call invoke() on FINALIZE type
_manager.removeListener(_propertyKey, _listener);
} else {
if (!isReady()) {
// avoid leaking CallbackHandler
logger.info("Callbackhandler {} with path {} is in reset state. Stop subscription to ZK client to avoid leaking",
this, parentPath);
return;
}
NotificationContext changeContext = new NotificationContext(_manager);
changeContext.setType(NotificationContext.Type.CALLBACK);
changeContext.setPathChanged(parentPath);
changeContext.setChangeType(_changeType);
subscribeForChanges(changeContext.getType(), _path, _watchChild);
enqueueTask(changeContext);
}
}
} catch (Exception e) {
String msg = "exception in handling child-change. instance: " + _manager.getInstanceName()
+ ", parentPath: " + parentPath + ", listener: " + _listener;
ZKExceptionHandler.getInstance().handle(msg, e);
}
}
This will send a FINALIZE event to the routing table provider which is listening on this path.
When routing table provider handles this event, it calls reset() and the entire routing table will be reset.
`if (changeContext.getType() == NotificationContext.Type.FINALIZE) {
reset();
}
`
It caused problems in customized view feature, that when an aggregation enabled customized state type is dropped from customized view, a FINALIZE event is sent to the routing table provider, and triggers the reset of routing table. If there is on more events incoming, we will see an empty routing table.
### To Reproduce
Initialize a routing table provider listening to customized state type A and B, set aggregation enabled customized state types in controller to be A andB, update some customized states for both types. Then set aggregation enabled customized state types in controller to be only A, and get routing table snapshot from routing table provider, and the routing table is empty.
### Expected behavior
We can fix this in two ways:
1. Fix callback handler, so it doesn't enqueue FINALIZE event for the change. For example, in the above piece of code, for CurrentState, when dropping a resource, the callback handler enqueue a CALLBACK typed event, so the routing table provider won't be reset unexpectedly.
2. Add granularity into routing table provider, so we don't shut down the entire routing table provider when getting FINALIZE event on one path.
### Additional context
Add any other context about the problem here.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 commented on issue #1296: Router completely reset when receives FINALIZE event
Posted by GitBox <gi...@apache.org>.
kaisun2000 commented on issue #1296:
URL: https://github.com/apache/helix/issues/1296#issuecomment-679423710
It seems that currentState based RP has one callbackhandler with all currentstate as child znode. Thus reset would refresh all currentstate only once. This is fine.
CustomizedView for each type has one callbackHandler. Thus, one removal of callbackhandler parent path would cause other types of customizedView be reset() too.
In sum, current design of routingProvider accomodate only one callbackhandler. (External view, currentstate). However customized view RP has callbackhandler per type. This does not fit into the current design of RP.
@jiajunwang , @zhangmeng916, is the above description right?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 edited a comment on issue #1296: Router completely reset when receives FINALIZE event
Posted by GitBox <gi...@apache.org>.
kaisun2000 edited a comment on issue #1296:
URL: https://github.com/apache/helix/issues/1296#issuecomment-679423710
It seems that currentState based RP has one callbackhandler with all currentstate as child znode. Thus reset would refresh all currentstate only once. This is fine.
CustomizedView for each type has one callbackHandler. Thus, one removal of callbackhandler parent path would cause other types of customizedView be reset() too. That is not what we want.
In sum, current design of routingProvider accommodates only one callbackhandler. (External view, currentstate). However customized view RP has callbackhandler per type. This does not fit into the current design of RP.
@jiajunwang , @zhangmeng916, is the above description right?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org