gortiz opened a new pull request, #19907: URL: https://github.com/apache/pinot/pull/19907
## Summary `TransformBlockValSet` now checks for query termination before each evaluation of its transform function. A caller that evaluates a transform many times in one block can no longer run past the query deadline, and a terminated or cancelled query stops at the next evaluation. ## Background The query engine checks for termination at block boundaries (`BaseOperator.nextBlock()`) and every 8,192 keys in the group-by combine merge. There is no check inside the processing of one block. If the work for one block is unexpectedly large, the query timeout and the cancellation cannot stop it. The combine operator then waits for that worker in `stopProcess()` before it releases the segments. `TransformBlockValSet` does not cache values. Each `get*Values*()` call evaluates the transform on the whole block again. #19905 fixes one case where this went wrong: `ANY_VALUE` in a group-by called `getStringValuesSV()` once per row, so a block of N rows evaluated the transform N times. With `jsonExtractScalar` on about 1.6 KB of JSON, one 10,000-row block took 15 to 40 minutes on a production cluster. The query failed at the broker, but the server kept the worker threads busy for that time. This change is the generic guard for that class of problem. It does not replace #19905. ## Change - `TransformBlockValSet` calls `QueryThreadContext.checkTermination()` before each `get*Values*()` evaluation. The hash getters and `getNumMVEntries()` go through these methods, so they are covered too. - The scope supplier is created once per value set, so the check does not allocate. - The check costs one volatile read, one `Thread.interrupted()` call and one `System.currentTimeMillis()` call per transform evaluation. One evaluation processes up to 10,000 rows, so the cost is negligible. - If no `QueryThreadContext` is set (for example, in some tests), the check does nothing, as before. In a normal query, this adds one check per block for each transform expression that the operator reads. After the deadline, the query stops within the cost of one transform evaluation on one block. Not covered by this change: - Work inside a single transform evaluation. One evaluation is bounded by the block size. - Nested transforms. In `f(g(x))`, `f` calls `g` through `transformTo*()` directly, not through a `TransformBlockValSet`. - Expression filters. `ExpressionFilterOperator` calls the transform function directly. A check in `BaseTransformFunction` could cover the last two, but it touches every transform function. I left it out of this change. ## Tests `TransformBlockValSetTerminationTest` covers these cases: | Case | Result | |---|---| | Active query | The transform runs. | | No `QueryThreadContext` | The transform runs. | | Query terminated (`QUERY_CANCELLATION`) | `TerminationException`. The transform does not run. | | Deadline passed | `QueryException` with `EXECUTION_TIMEOUT`. The transform does not run. | | Thread interrupted | `EarlyTerminationException`. The transform does not run. | `TransformEvaluationDeadlineQueriesTest` runs a group-by query on one segment of one block, with ten `SUM(sleep(...))` aggregations. Each transform evaluation takes 100 ms, so the block takes 1 s. The deadline is 200 ms and starts after planning. - With this change, the query fails with `EXECUTION_TIMEOUT` from inside the block (`Timing out on: transform function: sleep`), and the test finishes in less than 1 s. - Without this change, the test fails. The block runs to the end, and the timeout only comes from the next block boundary (`Timing out on: Transform`). The test uses `GROUP BY` because `DefaultGroupByExecutor.process()` handles a whole block in one call, which is the path of the original problem. All the query, transform, combine and group-by tests in `pinot-core` pass (2,405 tests). A local experiment (not part of this PR) ran the `ANY_VALUE(jsonExtractScalar(...))` group-by from #19905, without that fix, on one segment of 8,000 rows. Without this change, the query runs for 238 s: nothing inside the block checks the deadline, so a deadline does not change that time. With this change and a 2 s deadline, the query stopped after 2,005 ms with `Timing out on: transform function: jsonExtractScalar`. -- 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]
