Skip to content

Commit

Permalink
Fix computed value of maxSegmentsToMove when coordinator period is ve…
Browse files Browse the repository at this point in the history
…ry low (apache#16984)

Bug: When coordinator period is less than 30s, `maxSegmentsToMove` is always computed as 0
irrespective of number of available threads.

Changes:
- Fix lower bound condition and set minimum value to 100.
- Add new test which fails without this fix
  • Loading branch information
kfaraz authored Sep 2, 2024
1 parent 32e8e07 commit a734944
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class SegmentToMoveCalculator
{
/**
* At least this number of segments must be picked for moving in every cycle
* to keep the cluster well balanced.
* to keep the cluster well-balanced.
*/
private static final int MIN_SEGMENTS_TO_MOVE = 100;

Expand Down Expand Up @@ -150,7 +150,7 @@ public static int computeMaxSegmentsToMovePerTier(
int maxComputationsInThousands = (numBalancerThreads * num30sPeriods) << 20;
int maxSegmentsToMove = (maxComputationsInThousands / totalSegments) * 1000;

if (upperBound < lowerBound) {
if (upperBound < lowerBound || maxSegmentsToMove < lowerBound) {
return Math.min(lowerBound, totalSegments);
} else {
return Math.min(maxSegmentsToMove, upperBound);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ public void testMaxSegmentsToMove1Thread()
@Test
public void testMaxSegmentsToMoveIncreasesWithCoordinatorPeriod()
{
Assert.assertEquals(100, computeMaxSegmentsToMoveInPeriod(200_000, Duration.millis(0)));
Assert.assertEquals(100, computeMaxSegmentsToMoveInPeriod(200_000, Duration.millis(10_000)));
Assert.assertEquals(100, computeMaxSegmentsToMoveInPeriod(200_000, Duration.millis(20_000)));

Assert.assertEquals(5_000, computeMaxSegmentsToMoveInPeriod(200_000, Duration.millis(30_000)));
Assert.assertEquals(10_000, computeMaxSegmentsToMoveInPeriod(200_000, Duration.millis(60_000)));
Assert.assertEquals(15_000, computeMaxSegmentsToMoveInPeriod(200_000, Duration.millis(90_000)));
Expand Down

0 comments on commit a734944

Please sign in to comment.