noob-se7en commented on PR #19083: URL: https://github.com/apache/pinot/pull/19083#issuecomment-5236332991
> 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. We never/rarely ran into that issue after https://github.com/apache/pinot/pull/17062 was merged. I still don't think there is a strong need for this fix. And I am also not sure if forceCommitting is the best design here, adding new code behind default off config adds more to the config burden. If we can find out strong reason/latest production incident which re-surfaces a problem this PR addresses that will help. -- 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]
