vvivekiyer commented on code in PR #16164:
URL: https://github.com/apache/pinot/pull/16164#discussion_r2219869399
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/maker/InstancePlanMakerImplV2.java:
##########
@@ -305,6 +306,8 @@ private void applyQueryOptions(QueryContext queryContext) {
@Override
public PlanNode makeSegmentPlanNode(SegmentContext segmentContext,
QueryContext queryContext) {
rewriteQueryContextWithHints(queryContext,
segmentContext.getIndexSegment());
+ // Sample to track usage of query planning, since it can be expensive for
large segment lists.
+ Tracing.ThreadAccountantOps.sampleAndCheckInterruption();
Review Comment:
Keep all the sampling in a single place without moving it inside the
planning/pruning classes.. That way it's easier to reason about it.
For example: can we add both the pruning and planning sampling in
ServerQueryExecutorV1Impl.
- Pruning sampling in `executeInternal()` after the selectedSegmentsInfo is
computed.
- Planning in `executeQuery` before the `planCombineQuery` is called.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -298,7 +298,6 @@ protected BrokerResponse handleRequestThrowing(long
requestId, String query, Sql
try (QueryEnvironment.CompiledQuery compiledQuery =
compileQuery(requestId, query, sqlNodeAndOptions, httpHeaders,
queryTimer)) {
-
Review Comment:
(nit) remove spurious changes.
--
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]
