kfaraz commented on PR #20825: URL: https://github.com/apache/druid/pull/20825#issuecomment-6601685502
Some queries: > This is an improvement because it allows graceful shutdown of Brokers. IIUC from the patch, we now wait for up to 10s for ongoing queries to finish. But since we start waiting only after calling `ControllerHolder.cancel()` for these queries, can they complete only by failing? Barring the fact that the query cancellation will now ask all the workers to clean up their state at shutdown time itself, is there any inherent drawback to using ZK-based discovery in the `DartWorkerRunner`? Seems like the ZK-listener would also have eventually detected that a Broker has gone missing and it would have cleaned up any existing state, which would be very similar to the new controller reaper (since both are based on the absence of heartbeats). > (Brokers remove themselves from discovery at the beginning of the graceful shutdown window.) I assume this is true because `NodeAnnouncer.stop()` is called before `DartControllerRegistry.stop()`? Probably worth adding a comment that clarifies this. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
