fixed feedback

Author: tonyjongyoonan

xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java

@@ -332,10 +332,12 @@ public boolean isEquivalentTo(RoundRobinPicker picker) {
    * Implementation of Static Stride Scheduler, replaces EDFScheduler.
    * <p>
    * The Static Stride Scheduler works by iterating through the list of subchannel weights
-   * and using modular arithmetic to evenly distribute picks and skips, favoring entries with the
-   * highest weight. It generates a practically equivalent sequence of picks as the EDFScheduler.
-   * Albeit needing more bandwidth, the Static Stride Scheduler is more performant than the
-   * EDFScheduler, as it removes the need for a priority queue (and thus mutex locks).
+   * and using modular arithmetic to proportionally distribute picks, favoring entries 
+   * with higher weights. It is based on the observation that the intended sequence generated 
+   * from the EDF scheduler is a periodic one that can be achieved through modular arithmetic. 
+   * This scheduler generates a practically equivalent sequence of picks as the EDFScheduler.
+   * The Static Stride Scheduler is more performant than the EDFScheduler, as it removes 
+   * the need for a priority queue (and thus mutex locks).
    * <p>
    * go/static-stride-scheduler
    * <p>
@@ -359,7 +361,7 @@ static final class StaticStrideScheduler {
       float maxWeight = 0;
       int meanWeight = 0;
       for (float weight : weights) {
-        if (weight > 0.0001) {
+        if (weight > 0) {
           sumWeight += weight;
           maxWeight = Math.max(weight, maxWeight);
           numWeightedChannels++;
@@ -376,7 +378,7 @@ static final class StaticStrideScheduler {
       // scales weights s.t. max(weights) == K_MAX_WEIGHT, meanWeight is scaled accordingly
       int[] scaledWeights = new int[numChannels];
       for (int i = 0; i < numChannels; i++) {
-        if (weights[i] < 0.0001) {
+        if (weights[i] <= 0) {
           scaledWeights[i] = meanWeight;
         } else {
           scaledWeights[i] = (int) Math.round(weights[i] * scalingFactor);
@@ -394,7 +396,7 @@ private long nextSequence() {
       return Integer.toUnsignedLong(sequence.getAndIncrement());
     }
 
-    public long getSequence() {
+    long getSequence() {
       return Integer.toUnsignedLong(sequence.get());
     }
 

xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java

@@ -175,7 +175,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
             }
             });
     wrr = new WeightedRoundRobinLoadBalancer(helper, fakeClock.getDeadlineTicker(),
-        new FakeRandom());
+        new FakeRandom(0));
   }
 
   @Test
@@ -919,15 +919,15 @@ public void testAllInvalidWeightsUseOne() {
   }
 
   @Test
-  public void testLacrgestWeightIndexPickedEveryGeneration() {
+  public void testLargestWeightIndexPickedEveryGeneration() {
     float[] weights = {1.0f, 2.0f, 3.0f};
-    int mean = 2;
+    int largestWeightIndex = 2;
     Random random = new Random();
     StaticStrideScheduler sss = new StaticStrideScheduler(weights, random);
     int largestWeightPickCount = 0;
     int kMaxWeight = 65535;
-    for (int i = 0; i < mean * kMaxWeight; i++) {
-      if (sss.pick() == mean) {
+    for (int i = 0; i < largestWeightIndex * kMaxWeight; i++) {
+      if (sss.pick() == largestWeightIndex) {
         largestWeightPickCount += 1;
       }
     }
@@ -1022,7 +1022,7 @@ public void testManyComplexWeights() {
   @Test
   public void testDeterministicPicks() {
     float[] weights = {2.0f, 3.0f, 6.0f};
-    Random random = new FakeRandom();
+    Random random = new FakeRandom(0);
     StaticStrideScheduler sss = new StaticStrideScheduler(weights, random);
     assertThat(sss.getSequence()).isEqualTo(0);
     assertThat(sss.pick()).isEqualTo(1);
@@ -1042,7 +1042,7 @@ public void testDeterministicPicks() {
   @Test
   public void testImmediateWraparound() {
     float[] weights = {1.0f, 2.0f, 3.0f, 4.0f, 5.0f};
-    Random random = new FakeRandomWraparound();
+    Random random = new FakeRandom(-1);
     StaticStrideScheduler sss = new StaticStrideScheduler(weights, random);
     double totalWeight = 15;
     Map<Integer, Integer> pickCount = new HashMap<>();
@@ -1059,7 +1059,7 @@ public void testImmediateWraparound() {
   @Test
   public void testWraparound() {
     float[] weights = {1.0f, 2.0f, 3.0f, 4.0f, 5.0f};
-    Random random = new FakeRandomWraparound2();
+    Random random = new FakeRandom(-500);
     StaticStrideScheduler sss = new StaticStrideScheduler(weights, random);
     double totalWeight = 15;
     Map<Integer, Integer> pickCount = new HashMap<>();
@@ -1076,7 +1076,7 @@ public void testWraparound() {
   @Test
   public void testDeterministicWraparound() {
     float[] weights = {2.0f, 3.0f, 6.0f};
-    Random random = new FakeRandomWraparound();
+    Random random = new FakeRandom(-1);
     StaticStrideScheduler sss = new StaticStrideScheduler(weights, random);
     assertThat(sss.getSequence()).isEqualTo(0xFFFF_FFFFL);
     assertThat(sss.pick()).isEqualTo(1);
@@ -1106,25 +1106,17 @@ private static class FakeSocketAddress extends SocketAddress {
   }
 
   private static class FakeRandom extends Random {
-    @Override
-    public int nextInt() {
-      // return constant value to disable init deadline randomization in the scheduler
-      // sequence will be initialized to 0
-      return 0;
-    }
-  }
+    private int nextInt;
 
-  private static class FakeRandomWraparound extends Random {
-    @Override
-    public int nextInt() {
-      return -1;
+    public FakeRandom(int nextInt) {
+      this.nextInt = nextInt;
     }
-  }
 
-  private static class FakeRandomWraparound2 extends Random {
     @Override
     public int nextInt() {
-      return -500;
+      // return constant value to disable init deadline randomization in the scheduler
+      // sequence will be initialized to 0
+      return nextInt;
     }
   }
 }