FrankChen021 commented on code in PR #20314:
URL: https://github.com/apache/druid/pull/20314#discussion_r4093912233
##########
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 the cancel flag to Hep planning stages
**Finding:** The new framework context exposes the per-query CancelFlag to
Calcite's top-level planner, but the current CalciteRulesManager still builds
the pre/subquery/reduction/pre-Volcano/cleanup Hep planners through Programs.of
and Programs.subQuery, which pass a null Context. In Calcite 1.42 those Hep
planners therefore have private cancellation flags; a query that spends its
budget in one of those rule loops can keep the Broker planning thread busy past
maxPlanningTimeMs, and the later deadline check can only reject the result
after that work completes. The author's large-IN testing does not cover these
stages.
**Suggestion:** Build every Hep stage with the query CancelFlag, including
both subquery stages, or explicitly narrow the documented timeout guarantee,
and add a regression test for a timeout in 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]
