Vamsi-klu commented on PR #19083: URL: https://github.com/apache/pinot/pull/19083#issuecomment-5235777533
Pushed `80cc4f3`. All of the red CI came from one root cause: the five doc blocks this PR adds used `/** */` and master now requires the `///` JEP 467 form, so checkstyle failed the `pinot-controller` build and took Linter, Quickstart and the four compat jobs down with it. Converted all five and merged latest master. @noob-se7en on your point about force-committing partial offline segments, that is a fair concern and I do not want to paper over a genuinely bad server. What is there today: the feature is off by default, the commit is partition scoped so only the partition holding the OFFLINE replica commits rather than the whole table, there is an age gate of five minutes so young segments are never touched, and a once-per-segment guard stops repeats. The case I am targeting is the one from #15897, where a transient stream blip leaves one replica OFFLINE indefinitely and the partition runs under-replicated until somebody force commits by hand. Would it address your concern if the trigger also required the partial OFFLINE state to persist across N consecutive validation runs? Or would you rather this repair used the existing OFFLINE to CONSUMING reset instead of a force commit? I am happy to go either way. One thing worth flagging whichever way we go: a controller leadership change resets the in-memory once-per-segment tracking, so a ZK visible guard would be stronger than what I have now. -- 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]
