yashmayya commented on code in PR #18928:
URL: https://github.com/apache/pinot/pull/18928#discussion_r3533431116
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/executor/OpChainSchedulerService.java:
##########
@@ -257,7 +257,9 @@ public void onFailure(Throwable t) {
// RejectedExecutionException. When it does, the task never runs and the
directExecutor FutureCallback above
// (which decrements the active-opchain counter and removes the
per-request context entry) never fires. Back
// out that bookkeeping here so the entry — and the
QueryExecutionContext it pins — does not leak until a later
- // cancel. Then rethrow so the caller propagates the failure as a stage
error.
+ // cancel. Then rethrow so the caller propagates the failure as a stage
error. (The caller,
+ // QueryRunner#processQueryBlocking, close()s the op chain on this
rethrow, releasing operator resources that
+ // the never-fired FutureCallback would otherwise have closed.)
Review Comment:
Good catch, thanks — you're right. On the reject path `runJob` never runs,
so neither its `_opChainCache.invalidate(opChainId)` nor the `FutureCallback`
fires, leaving the entry pinning the `rootOperator` tree +
`QueryExecutionContext` until TTL/weight eviction. The caller-side
`opChain.close()` frees the operator's internal resources but not those
references.
Fixed in 127b9517d3: added `_opChainCache.invalidate(opChainId)` next to the
`decrementActiveOpChains()` backout in the submit-reject `catch`, mirroring the
prompt release `cancel()` already does for cancelled entries (and updated the
comment).
--
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]
