Terminate the query after plan generation if timeout

[jasperjiaguo]

End query execution early if timeout reached after planning.

[siddharthteotia]

What about timeOut during per segment planNode creation, that is before execute is called and physical operators are created ? Can happen for extremely large number of segments I guess.

Plan queryPlan =
          enableStreaming ? _planMaker.makeStreamingInstancePlan(selectedSegments, queryContext, executorService,
              responseObserver, _serverMetrics) : _planMaker.makeInstancePlan(selectedSegments, queryContext,
              executorService, _serverMetrics);
      planBuildTimer.stopAndRecord();

^^ can potentially timeOut

TimerContext.Timer planExecTimer = timerContext.startNewPhaseTimer(ServerQueryPhase.QUERY_PLAN_EXECUTION);
     DataTable dataTable = queryContext.isExplain() ? processExplainPlanQueries(queryPlan) : queryPlan.execute();

[jasperjiaguo]

What about timeOut during per segment planNode creation, that is before execute is called and physical operators are created ? Can happen for extremely large number of segments I guess.

Plan queryPlan =
          enableStreaming ? _planMaker.makeStreamingInstancePlan(selectedSegments, queryContext, executorService,
              responseObserver, _serverMetrics) : _planMaker.makeInstancePlan(selectedSegments, queryContext,
              executorService, _serverMetrics);
      planBuildTimer.stopAndRecord();

^^ can potentially timeOut

TimerContext.Timer planExecTimer = timerContext.startNewPhaseTimer(ServerQueryPhase.QUERY_PLAN_EXECUTION);
     DataTable dataTable = queryContext.isExplain() ? processExplainPlanQueries(queryPlan) : queryPlan.execute();

The work here I think is to do makeSegmentPlanNode for each segment to create top level plan nodes, shouldn't be too much work?

[siddharthteotia]

Suggest removing printStackTrace. lgtm otherwise

[siddharthteotia]

The work here I think is to do makeSegmentPlanNode for each segment to create top level plan nodes, shouldn't be too much work?

Yes I agree. Not a big deal. I was overthinking it

[codecov-commenter]

Codecov Report

Merging #9386 (6d60383) into master (987480b) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             master    #9386    +/-   ##
==========================================
  Coverage     63.44%   63.44%            
+ Complexity     5069     4762   -307     
==========================================
  Files          1832     1832            
  Lines         98122    98147    +25     
  Branches      15017    15021     +4     
==========================================
+ Hits          62250    62269    +19     
- Misses        31283    31286     +3     
- Partials       4589     4592     +3     

Flag	Coverage Δ
unittests1	66.97% <0.00%> (+0.01%)	⬆️
unittests2	15.36% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...a/org/apache/pinot/core/plan/GlobalPlanImplV0.java	86.66% <0.00%> (-13.34%)	⬇️
.../main/java/org/apache/pinot/minion/MinionConf.java	37.50% <0.00%> (-2.50%)	⬇️
...lix/core/realtime/PinotRealtimeSegmentManager.java	71.20% <0.00%> (-2.10%)	⬇️
...che/pinot/broker/routing/BrokerRoutingManager.java	58.19% <0.00%> (-1.42%)	⬇️
...core/query/reduce/ExplainPlanDataTableReducer.java	82.75% <0.00%> (-0.69%)	⬇️
.../aggregation/function/ModeAggregationFunction.java	87.56% <0.00%> (-0.55%)	⬇️
...va/org/apache/pinot/spi/utils/CommonConstants.java	27.69% <0.00%> (ø)
...g/apache/pinot/common/metrics/ControllerMeter.java	0.00% <0.00%> (ø)
...che/pinot/core/operator/docidsets/AndDocIdSet.java	100.00% <0.00%> (ø)
...ntroller/helix/core/minion/CronJobScheduleJob.java	0.00% <0.00%> (ø)
... and 9 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more