FrankChen021 commented on code in PR #20314:
URL: https://github.com/apache/druid/pull/20314#discussion_r4082348386
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java:
##########
@@ -326,6 +327,10 @@ public SqlConformance conformance()
if (aClass.equals(PlannerContext.class)) {
return (C) plannerContext;
}
+ if (aClass.equals(CancelFlag.class)) {
Review Comment:
[P2] Propagate cancellation to Hep planning stages
**Finding:** This framework context reaches the top-level VolcanoPlanner,
but the current Calcite 1.42 Programs.of and Programs.subQuery helpers
construct their HepPlanners with a null context, so AbstractRelOptPlanner gives
those stages a private cancellation flag instead of the per-query flag. The
watchdog's Thread.interrupt() is not checked by Hep's rule loops. A query that
spends its planning budget in subquery rewriting, expression reduction, or
pre-Volcano/cleanup Hep stages can therefore keep the Broker planning thread
busy past maxPlanningTimeMs; the later deadline check only rejects the result
after that work completes.
**Suggestion:** Retain cancel-aware Hep program construction for every Hep
stage, including both subquery stages, or otherwise pass the per-query
CancelFlag into those HepPlanners, and add a regression test for a timeout
during a Hep stage.
--
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]
