yashmayya opened a new pull request, #19709: URL: https://github.com/apache/pinot/pull/19709
## The bug If planning of a multi-stage query takes longer than the query timeout, the broker returns `BrokerTimeoutError: Timed out while planning query`. It also calls `Future.cancel(true)`, which interrupts the planning thread. But Calcite ignores interrupts, so the thread keeps planning to the end. When the query is terminated (`QueryExecutionContext.terminate`), the same thing happens, because this call also interrupts the thread. Each of these planning threads is one of the `availableProcessors / 2` threads of the broker compile executor. A small number of slow queries can use all of them. Then new multi-stage queries wait in the queue and also fail with `BrokerTimeoutError`. Example: a query with 300 `UNION ALL` branches (`SELECT col1, col3 + <i> FROM a WHERE col3 > <i> AND col2 = '<i>'`) plans for about 16 s. With `SET timeoutMs=1000`, `MultiStageBrokerRequestHandler` returned the timeout error after 1.2 s, and its compile thread planned for 17.6 s more. ## The fix Calcite stops planning only if the `CancelFlag` of the planner `Context` is set. It reads this flag in `RelOptPlanner.checkCancel()`, before it fires each rule. Now, if the thread is interrupted, `LogicalPlanner` also throws `EarlyTerminationException` from `checkCancel()`. `LogicalPlanner` is the `HepPlanner` that runs the Pinot rule programs. A `CancelFlag` alone does not correct this. The code that cancels planning (the broker timeout and `QueryExecutionContext.terminate`) interrupts the thread, and it has no access to a flag. With this change, the thread in the example stopped 1 ms after the timeout. The broker startup warmup (`warmupCompile`) waits 5 s for a `SELECT 1` compile. After the wait, it called `shutdownNow()`, but Calcite ignored this interrupt, so a slow warmup continued in the background. Now the warmup calls `shutdown()`, so a slow warmup continues to its end as before. ## Limits Planning stops only before the next rule of a `LogicalPlanner` program fires. These steps still run to their end after an interrupt: - A single slow rule. - Validation, SQL-to-rel conversion, decorrelation (Calcite creates plain `HepPlanner`s for it) and field trimming. For example, the conversion of a very large `IN` list inside a `CASE` is slow. - The Pinot planning steps after Calcite: the v2 physical optimizer, `planQuery` and `explain`. ## Testing - `QueryPlanningCancellationTest` plans a query with an added test rule. The rule blocks on its first firing and ignores interrupts, as Calcite code does. The test cancels the planning task (as the broker does on timeout), or terminates the query. Then it makes sure that no other rule fires and that planning fails. Without this change, both tests fail: the rule fires 3 times and planning succeeds. - `LogicalPlannerTest` covers `checkCancel()` with an interrupted thread and with a set `CancelFlag`. - The new tests passed in 200 repeated runs. - All 1953 `pinot-query-planner` tests and all 511 `pinot-broker` tests pass. `spotless`, `checkstyle` and `license` are clean. -- 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]
