FrankChen021 commented on code in PR #20314:
URL: https://github.com/apache/druid/pull/20314#discussion_r3989311012
##########
sql/src/main/java/org/apache/druid/sql/DirectStatement.java:
##########
@@ -196,20 +197,44 @@ public ResultSet plan()
}
long planningStartNanos = System.nanoTime();
try (DruidPlanner planner = createPlanner()) {
- validate(planner);
- authorize(planner, authorizer());
+ // Bound the wall-clock time spent planning this query. A non-positive
timeout disables this.
+ final long maxPlanningTimeMs =
planner.getPlannerContext().getPlannerConfig().getMaxPlanningTimeMs();
+ try (SqlPlanningTimeout timeout = SqlPlanningTimeout.arm(
Review Comment:
## Follow-up assessment
I re-reviewed all 11 changed files at current head
`fd86a12573d4d1af0afef053f20ef3768098a46a`. The watchdog-queue, Hep
cancellation, nested-view cancellation, late-success, and config-round-trip
findings are addressed. The planner-construction timeout finding remains a P2:
`createPlanner()` still runs before the watchdog is armed, and an
already-exhausted budget is reduced to a 1 ms schedule rather than being marked
timed out immediately. A slow schema/planner setup can therefore exceed the
configured deadline without interruption, and a fast follow-on plan can return
before that 1 ms callback fires.
Please enforce the elapsed deadline before proceeding or arm the effective
deadline before planner construction.
Reviewed 11 of 11 changed files. No other actionable follow-up findings were
identified.
<!-- mergelens:review -->
--
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]
