FrankChen021 commented on code in PR #20314:
URL: https://github.com/apache/druid/pull/20314#discussion_r4104511272
##########
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:
Thanks for the context. I rechecked the current head and all 9 of 9 changed
files. The issue is not limited to the large-IN benchmark: in the pinned
Calcite 1.42, `Programs.of(...)` creates `HepPlanner` with a `null` context,
and `Programs.subQuery(...)` delegates to that helper. `HepPlanner` inherits
`AbstractRelOptPlanner.fireRule`, which calls `checkCancel()`, but the null
context makes it use a private cancellation flag, so it never observes the
per-query `CancelFlag` armed by `DirectStatement`.
`CalciteRulesManager.buildPreProgram` still runs this subquery stage (and other
`Programs.of` Hep stages) before Volcano. Thus a query whose time is spent in
one of those rule loops can keep a Broker planning thread past
`maxPlanningTimeMs`; the observed Volcano-dominated large-IN case does not
cover that path. I’m keeping the existing P2: either pass the query flag into
each Hep planner or narrow the documented timeout guarantee.
<!-- 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]
