leventov commented on a change in pull request #7174: Change reservoir sampling 
of segments to spliterator approach
URL: https://github.com/apache/incubator-druid/pull/7174#discussion_r262738218
 
 

 ##########
 File path: 
server/src/main/java/org/apache/druid/server/coordinator/ReservoirSegmentSampler.java
 ##########
 @@ -19,36 +19,93 @@
 
 package org.apache.druid.server.coordinator;
 
+import com.google.common.math.IntMath;
+import org.apache.druid.client.ImmutableDruidServer;
 import org.apache.druid.timeline.DataSegment;
 
+import java.math.RoundingMode;
+import java.util.Collection;
 import java.util.List;
+import java.util.Spliterator;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 
 final class ReservoirSegmentSampler
 {
+  private static final int SPLITERATOR_SIZE_THRESHOLD = 25;
 
   static BalancerSegmentHolder getRandomBalancerSegmentHolder(final 
List<ServerHolder> serverHolders)
   {
-    ServerHolder fromServerHolder = null;
+    ServerHolder fromServerHolder = reservoirSampleServer(serverHolders);
+    if (fromServerHolder == null) {
+      return null;
+    }
+
     DataSegment proposalSegment = null;
-    int numSoFar = 0;
-
-    for (ServerHolder server : serverHolders) {
-      for (DataSegment segment : server.getServer().getSegments()) {
-        int randNum = ThreadLocalRandom.current().nextInt(numSoFar + 1);
-        // w.p. 1 / (numSoFar+1), swap out the server and segment
-        if (randNum == numSoFar) {
-          fromServerHolder = server;
-          proposalSegment = segment;
+    Collection<DataSegment> segments = 
fromServerHolder.getServer().getSegments();
+
+    if (segments.size() == 1) {
+      proposalSegment = segments.iterator().next();
+    } else {
+      Spliterator<DataSegment> chosenSpliterator = segments.spliterator();
+      if (chosenSpliterator == null) {
+        return null;
+      }
+
+      for (int i = 0; i < IntMath.log2(segments.size(), RoundingMode.UP); i++) 
{
 
 Review comment:
   This is right. The loop doesn't need to be bounded at all. Also, a single 
`randomBits = nextInt()` can be computed before the loop and checking 
`((randomBits >>= 1) & 1) == 0` instead of calling into TLR every time.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to