Jackie-Jiang opened a new pull request, #19054: URL: https://github.com/apache/pinot/pull/19054
## Summary During a tier-relocation rebalance of a strict realtime table (upsert/dedup), `TableRebalancer`'s convergence loop reads the `IdealState` at version `V`, recomputes the target assignment, then issues a version-checked update expecting `V`. On a continuously-ingesting table, consuming-segment commits bump the `IdealState` version between the read and the write, so the update loses the compare-and-set and retries. For strict realtime segment assignment, the recompute on a version change was **unconditionally** a full `rebalanceTable`, which reads segment ZK metadata per partition (`MultiTierStrictRealtimeSegmentAssignment.getExistingAssignment`). That made every retry expensive and widened the window in which the version can change, so a tier relocation could make very little progress while ingestion continued. The full recompute is only necessary when the rebalance actually **moves consuming segments** (e.g. rebalancing the consuming tier). When it only moves completed segments (a tier relocation), a consuming segment added to the `IdealState` mid-rebalance keeps its placement in both the current and target assignment, so the previously computed target can be reused via the existing cheap path — the same one non-strict assignment already uses when no moving segment changed state. ## Change - Gate the forced full recompute on `isStrictRealtimeSegmentAssignment && hasMovingConsumingSegments(...)` instead of `instanceof BaseStrictRealtimeSegmentAssignment`. For a tier relocation (no consuming segments moving), the loop now takes the cheap path on a version change, narrowing the update window and letting the rebalance make progress under sustained ingestion. - The segment `tier` ZK metadata is persisted up front by `TableRebalanceManager.updateTargetTier` before the convergence loop, so it does not change per iteration — reusing the target during the loop is safe. - Simplify `getMovingConsumingSegments` to a single pass that reads the first entry of each segment's (uniform) instance-state map, and add a short-circuiting `hasMovingConsumingSegments` for the existence check the gate uses. - Add `testHasMovingConsumingSegments` and update `testGetMovingConsumingSegments` to realistic uniform-state inputs. ## Notes - This is the first of two changes. A follow-up will add rebase-on-conflict to the versioned `IdealState` update, so a lost compare-and-set can retry against the newest version without another ExternalView wait; this PR only reduces the cost and likelihood of the conflict. - Trade-off: a tier relocation now skips the mid-loop instance-partition recompute, so an instance-config change *during* a relocation is picked up on the next rebalance run rather than mid-flight. -- 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]
