yashmayya opened a new pull request, #18928:
URL: https://github.com/apache/pinot/pull/18928

   ## Problem
   
   `OpChainSchedulerService.register()` can throw *after* the op chain has been 
built, on paths where the op chain is never scheduled to run:
   
   - the query was cancelled or terminated before scheduling 
(`checkTermination`, or the cancelled-query cache), or
   - the executor rejected the task (e.g. under load shedding / when the work 
queue is full → `RejectedExecutionException`).
   
   On these paths the `ListenableFutureTask` never runs, so the 
`FutureCallback` that normally calls `operatorChain.close()` on completion 
(success or failure) never fires, and the op chain's operators are never 
closed. Any operator that releases resources only in `close()` is then leaked 
for the lifetime of the process.
   
   ## Fix
   
   `QueryRunner#processQueryBlocking` now closes the built op chain in its 
`catch` when scheduling fails, before propagating the error as a stage error:
   
   - `opChain` is hoisted out of the `try` and closed only when non-null, so a 
failure during `convert` / `compileLeafStage` (before it is assigned) is 
unaffected.
   - `close()` is idempotent and is wrapped in its own guard, so a failure 
while closing cannot mask the original exception that must be propagated.
   - `OpChainSchedulerService`'s submit-rejection path continues to back out 
its active-op-chain bookkeeping and rethrow; the added `close()` at the caller 
does not affect that bookkeeping (a scheduler-registered chain's finish 
callback is a no-op).
   
   This mirrors the normal completion path, where the `FutureCallback` both 
decrements the active-op-chain counter and calls `operatorChain.close()`.
   
   ## Testing
   
   The affected paths are scheduling-failure races (executor rejection under 
load shedding; cancellation between build and scheduling), which are awkward to 
drive deterministically at the `QueryRunner` level. The change is a small, 
guarded, idempotent `close()` that mirrors the existing `FutureCallback` 
completion path and is a no-op on the success path. Happy to add a targeted 
test if reviewers prefer a particular harness.
   


-- 
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]

Reply via email to