mgao0 commented on pull request #1000: URL: https://github.com/apache/helix/pull/1000#issuecomment-635528578
> I just saw this diff. In fact, this is a totally different from what it was before. I have quite some reservation to add periodical refresh in CallbackHandler. > > CallbackHandler is the interface with zkclient managing the lifecycle of a specific Helix property path. Specifically they handle notification (further calling back to application logic) and registering further notification logic. > > The part actually already had some notable issues (lose notification, or installing "leaked path etc) that should be addressed. We should not complicate this part with further here. Doing periodic refresh in callback handler can actually resolve the issue that you mentioned such as lost notification or missing watcher. This periodic refresh logic is very independent, it's a separate thread, and it put an event in the event queue only if there is no event processed for a while. That being said, it the callback handler is functioning well and events are processed normally, it won't add much burden to the performance since the periodic refresh event won't be generated. Is there any specific concern you have on the periodic refresh? ---------------------------------------------------------------- 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]
