Skip to content

Modify strictReplicaGroup rebalance batching to categorize on current+target instances to partitionId to currentAssignment#15838

Merged
somandal merged 2 commits intoapache:masterfrom
somandal:rebalance-batching-reorder-grouping
May 20, 2025
Merged

Modify strictReplicaGroup rebalance batching to categorize on current+target instances to partitionId to currentAssignment#15838
somandal merged 2 commits intoapache:masterfrom
somandal:rebalance-batching-reorder-grouping

Conversation

@somandal
Copy link
Contributor

@somandal somandal commented May 19, 2025

This PR modifies strictReplicaGroup rebalance batching to categorize on Pair(current instances, target instances) to partitionId to currentAssignment, instead of partitionId -> current instances -> currentAssignment. It also modifies the code to move a full Pair(current instances, target instances) to partitionId at a time rather than a full partitionId at a time to provide more granular batching.

This is to more closely mimic how strictReplicaGroup instance selection is done, which is based on the assignments and knows nothing about the partitions, or whether partitioning is even enabled. The main objective is to assess segment availability and mark instances as a whole as unavailable if even one segment is unavailable for that instance.

For StrictRealtimeSegmentAssignment, the full partitionId is guaranteed to belong to a single Pair(current instances, target instances), since the IdealState is used to update the segments that are newly assigned. Thus the invariant for consistency purposes that the full partition must move as a whole is still maintained, since the full partition will be part of the same pair. Even when choosing whether the segment can be moved based on availability, the partition will be chosen as a whole or be skipped for move as a whole since the assignedInstances is used for the availability tracking and all segments of a partition will have the same assignedInstances and available instances (which is same as the older rebalance code without batching).

For other segment assignment strategies, the code will now move a full Pair(current instances, target instances) to partitionId as a whole, but it can happen that a partitionId is split across multiple Pair(current instances, target instances). It is okay to move them separately since there is no mandate in these segment assignment strategies that the partition must be moved as a whole or assigned based on IdealState (it is instead assigned based on instance partitions if it exists or the tagged servers). It is enough to ensure that the minAvailableReplicas will be honored based on strictReplicaGroup instance selector, which is done by the original rebalance code already (and utilized even with batching).

Testing done:

Used HybridQuickstart to try the following with strictReplicaGroup instance selector enabled:

  • StrictRealtimeSegmentAssignment for REALTIME tables
  • RealtimeSegmentAssignment for REALTIME tables
  • OfflineSegmentAssignment for OFFLINE tables

Verified the grouping happens as expected, and the full Pair(current instances, target instances) to partitionId is moved as a whole. Non-batching works as expected.

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.36%. Comparing base (1a476de) to head (688a8b6).
Report is 106 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15838      +/-   ##
============================================
+ Coverage     62.90%   63.36%   +0.46%     
+ Complexity     1386     1353      -33     
============================================
  Files          2867     2898      +31     
  Lines        163354   165797    +2443     
  Branches      24952    25360     +408     
============================================
+ Hits         102755   105055    +2300     
+ Misses        52847    52807      -40     
- Partials       7752     7935     +183     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.34% <100.00%> (+0.47%) ⬆️
java-21 63.30% <100.00%> (+0.48%) ⬆️
skip-bytebuffers-false ?
skip-bytebuffers-true ?
temurin 63.36% <100.00%> (+0.46%) ⬆️
unittests 63.36% <100.00%> (+0.46%) ⬆️
unittests1 56.39% <ø> (+0.57%) ⬆️
unittests2 33.43% <100.00%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@somandal somandal requested review from Jackie-Jiang, klsince and yashmayya and removed request for klsince and yashmayya May 19, 2025 16:46
@somandal somandal added the segment-rebalance Related to segment rebalancing across servers label May 19, 2025
…rget instances to partitionId to currentAssignment
@somandal somandal force-pushed the rebalance-batching-reorder-grouping branch from 8438509 to 9f8c4bc Compare May 19, 2025 22:19
@somandal somandal requested a review from Jackie-Jiang May 19, 2025 23:10
@somandal somandal merged commit 0f38fd7 into apache:master May 20, 2025
17 checks passed
@somandal somandal deleted the rebalance-batching-reorder-grouping branch May 20, 2025 19:27
songwdfu pushed a commit to songwdfu/pinot that referenced this pull request Jun 3, 2025
…+target instances to partitionId to currentAssignment (apache#15838)

* Fix strictReplicaGroup rebalance batching to categorize on current+target instances to partitionId to currentAssignment

* Address review comment: short-circuit return when rebalance batching is disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

segment-rebalance Related to segment rebalancing across servers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants