walterddr commented on code in PR #10006:
URL: https://github.com/apache/pinot/pull/10006#discussion_r1061941643
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/combine/CombineSlowOperatorsTest.java:
##########
@@ -194,17 +197,19 @@ private void
testCancelCombineOperator(BaseCombineOperator combineOperator, Coun
} finally {
combineExecutor.shutdownNow();
}
- TestUtils.waitForCondition((aVoid) -> exp.get() instanceof
QueryCancelledException, 10_000,
- "Should have been cancelled");
+ TestUtils.waitForCondition((aVoid) -> exp.get() instanceof
QueryCancelledException
+ || exp.get() instanceof EarlyTerminationException, 10_000, "Should
have been cancelled");
assertEquals(exp.get().getMessage(), errMsg);
Review Comment:
@Jackie-Jiang we need to catch both. previously early termination exception
is unlikely to throw b/c we enter the base nextBlock() method first. then start
the threading; now we start the threading first then calls nextBlock() thus we
can potentially receive an EarlyTerminationExcpetion from the BaseOperator
is this desirable?
--
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
