yashmayya commented on code in PR #15071:
URL: https://github.com/apache/pinot/pull/15071#discussion_r1959480962
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/executor/OpChainSchedulerService.java:
##########
@@ -49,10 +49,9 @@ public void register(OpChain operatorChain) {
Future<?> scheduledFuture = _executorService.submit(new TraceRunnable() {
@Override
public void runJob() {
- boolean isFinished = false;
TransferableBlock returnedErrorBlock = null;
Throwable thrown = null;
- try {
+ try (OpChain closeMe = operatorChain) {
Review Comment:
nit: let's add a Javadoc to this `register` method stating that the
`OpChain` will be closed after execution completes?
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafStageTransferableBlockOperator.java:
##########
@@ -182,6 +183,26 @@ protected TransferableBlock getNextBlock()
}
}
+ @Override
+ protected void earlyTerminate() {
+ super.earlyTerminate();
+ cancelSseTasks();
+ }
+
+ @Override
+ public void cancel(Throwable e) {
+ super.cancel(e);
+ cancelSseTasks();
+ }
+
+ @VisibleForTesting
+ protected void cancelSseTasks() {
+ Future<Void> executionFuture = _executionFuture;
Review Comment:
Why do we need to copy this here? `_executionFuture` is only set once right?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
