[GitHub] [pinot] mqliang commented on a change in pull request #8067: Wire EmptySegmentPruner to routing config

[GitBox <gi...@apache.org>]

mqliang commented on a change in pull request #8067:
URL: https://github.com/apache/pinot/pull/8067#discussion_r792153434



##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SegmentPrunerFactory.java
##########
@@ -128,21 +132,32 @@ private static TimeSegmentPruner getTimeSegmentPruner(TableConfig tableConfig,
     return new TimeSegmentPruner(tableConfig, propertyStore);
   }
 
-  private static List<SegmentPruner> sortSegmentPruners(List<SegmentPruner> pruners) {
-    // If there's multiple pruners, move time range pruners to the front。
+  private static void sortSegmentPruners(List<SegmentPruner> pruners) {
+    // If there's multiple pruners, always prune empty segments first. After that, pruned based on time range, and
+    // followed by partition pruners.
     // Partition pruner run time is proportional to input # of segments while time range pruner is not,
     // Prune based on time range first will have a smaller input size for partition pruners, so have better performance.
-    List<SegmentPruner> sortedPruners = new ArrayList<>();
-    for (SegmentPruner pruner : pruners) {
-      if (pruner instanceof TimeSegmentPruner) {
-        sortedPruners.add(pruner);
+    int left = 0;
+    int right = pruners.size() - 1;
+    int current = 0;
+    while (current <= right) {
+      SegmentPruner currentPruner = pruners.get(current);
+      // if currentPruner is EmptySegmentPruner, move it to front by swapping with the left pointer
+      if (currentPruner instanceof EmptySegmentPruner) {
+        pruners.set(current, pruners.get(left));
+        pruners.set(left, currentPruner);
+        left++;
       }
-    }
-    for (SegmentPruner pruner : pruners) {
-      if (!(pruner instanceof TimeSegmentPruner)) {
-        sortedPruners.add(pruner);
+      // if current is PartitionSegmentPruner, move it to end by swapping with right pointer
+      if (currentPruner instanceof PartitionSegmentPruner) {
+        pruners.set(current, pruners.get(right));
+        pruners.set(right, currentPruner);
+        right--;
+        // may have swapped an EmptySegmentPruner/PartitionSegmentPruner from the end of list that requires extra
+        // processing, so decrement the current index to account for it.
+        current--;
       }
+      current++;

Review comment:
       done




-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org