yashmayya commented on PR #16008: URL: https://github.com/apache/pinot/pull/16008#issuecomment-2961262362
I realized that there is actually a pretty major flaw in disabling retries for `SegmentRelocator` rebalances while enabling ZK-based progress tracking. If a controller dies in the midst of an in-progress `SegmentRelocator` initiated rebalance, we will never recover from this state automatically and all subsequent rebalances (user initiated or system triggered) will be rejected until the in-progress but stuck rebalance is manually cancelled by the user. Thinking about this some more, is it really a bad idea to allow the RebalanceChecker to do its thing even on rebalances initiated by `SegmentRelocator`? It would help avoid such situations since it aborts stuck rebalances (no progress update for a time duration longer than the heartbeat timeout). And when it does initiate a retry for failed / stuck rebalances, the next run of `SegmentRelocator` will simply be a no-op for the table assuming the retried rebalance wasn't completed by then (it will basically just log that the newly initiated table rebalance failed because one is already in progress); and the next run after the retried rebalance completes will complete normally. Since we have a random initial delay for controller periodic tasks, it's also fairly unlikely that we'll encounter a situation where the `SegmentRelocator` initiates a rebalance and the `RebalanceChecker` retries a failed / stuck rebalance at exactly the same time (remember that we currently disallow concurrent rebalances based on the ZK rebalance progress stats - so a race condition is indeed possible). I guess another option would be to allow the `RebalanceChecker` to abort, but not retry stuck rebalances initiated by `SegmentRelocator`. Note that the possible, but unlikely race condition mentioned above is still possible - the `RebalanceChecker` could theoretically still retry a user-initiated failed / stuck rebalance at the same time as the `SegmentRelocator` initiating a rebalance for the same table. Importantly, though, none of these are newly introduced issues and were always possible cases. I'm closing this PR until we discuss and decide the best way to avoid the newly introduced issue of stuck `SegmentRelocator` initiated rebalances blocking all future rebalances until manually cancelled. -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
