FrankChen021 commented on code in PR #20724:
URL: https://github.com/apache/druid/pull/20724#discussion_r4370808955


##########
server/src/test/java/org/apache/druid/server/coordinator/balancer/ReservoirSegmentSamplerTest.java:
##########
@@ -279,15 +279,32 @@ public void 
testSegmentsFromMorePopulousServerAreMoreLikelyToBePicked()
 
       // Number of segments picked from server0 are ~40% of total and
       // number of segments picked from other servers are each ~20% of total
-      double error = totalSegmentsPicked * 0.02;
-      Assertions.assertEquals(totalSegmentsPicked * 0.40, 
numSegmentsPickedFromServer[0], error);
+      Assertions.assertEquals(
+          totalSegmentsPicked * 0.40,
+          numSegmentsPickedFromServer[0],
+          allowedError(totalSegmentsPicked, 0.40)
+      );
 
       for (int serverId = 1; serverId < servers.size(); ++serverId) {
-        Assertions.assertEquals(totalSegmentsPicked * 0.20, 
numSegmentsPickedFromServer[serverId], error);
+        Assertions.assertEquals(
+            totalSegmentsPicked * 0.20,
+            numSegmentsPickedFromServer[serverId],
+            allowedError(totalSegmentsPicked, 0.20)
+        );
       }
     }
   }
 
+  /**
+   * Returns 4 standard deviations of a binomial count with {@code numPicked} 
trials
+   * and probability {@code p}. A fixed percentage of {@code numPicked} is too 
tight
+   * for small samples and makes the test flaky.
+   */
+  private static double allowedError(int numPicked, double p)

Review Comment:
   [P2] Apply the statistical tolerance to the equal-server case
   
   **Finding:** The new allowedError helper is only used by the 
more-populous-server test; the equal-server test still uses totalSegmentsPicked 
* 0.02. At the 5% sample size it aggregates 2,500 picks, and each 25%-server 
count has a standard deviation of about 21, so ±50 is only about 2.4 standard 
deviations. One of four assertions can still fail randomly, leaving the 
flaky-test failure mode in the same test class.
   
   **Suggestion:** Apply the statistical or exact hypergeometric margin to the 
equal-server assertions as well, while retaining the weighted-distribution 
check separately.



-- 
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]

Reply via email to