sajjad-moradi commented on a change in pull request #8067:
URL: https://github.com/apache/pinot/pull/8067#discussion_r792091710
##########
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:
The algorithm here is not hard to follow, but it's not as easy to
understand as the older version. Since at most there will be three pruners,
looping through pruners multiple times doesn't affect performance. For the sake
of simplicity, I suggest using the older version.
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
