Jackie-Jiang commented on PR #19054: URL: https://github.com/apache/pinot/pull/19054#issuecomment-5055372034
@J-HowHuang thanks — your paused-table scenario was spot on and showed the consuming-segment-based gate was insufficient: segments added mid-rebalance by a minion task would not have been re-collocated. The PR has been reworked to gate on instance reassignment instead. A newly added segment (new consuming segment or uploaded segment) is placed from the current instance partitions, collocated with its partition. So when the rebalance is not reassigning instances (`reassignInstances`/`bootstrap` both false — e.g. the `SegmentRelocator` flow), the instance partitions are read-only for the job and any segment added mid-rebalance is placed consistently with the target, making the cheap path safe. With instance reassignment enabled, the conservative full recompute is kept, matching prior behavior. In your scenario the config-change rebalance either reassigns instances (full recompute, re-collocated) or does not (added segments are placed from the same fixed instance partitions the target uses), so both cases are handled. @xiangfu0 the reworked PR no longer touches `getMovingConsumingSegments()`, so the first-replica inference and the removed test case are both back to the original. PR description is updated to match. -- 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]
