dongxiaoman commented on a change in pull request #7243:
URL: https://github.com/apache/pinot/pull/7243#discussion_r681351838
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/SegmentPrunerFactory.java
##########
@@ -84,6 +82,8 @@ private SegmentPrunerFactory() {
}
}
}
+ // Always prune out empty segments first
Review comment:
For this change I am a bit concerned whether this change will break some
of the existing features, that we may end up selecting some empty segments and
later got pruned out, resulting a failed query .
The move is after the below finding:
In our tests, we found that `EmptySegmentPruner` is trying to create a
duplicated copy of all the segment names every time.
It shows up in the jstack outputs, meaning that copying 30k+ segment names
for each query is actually not cheap. Moving it to the bottom may mean better
performance, in our test it is the difference of **60k QPS => 80k QPS**
(in the case if there is empty segment around)
--
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]
