amaechler commented on code in PR #20322:
URL: https://github.com/apache/druid/pull/20322#discussion_r4315676400
##########
processing/src/test/java/org/apache/druid/frame/processor/RunAllFullyWidgetTest.java:
##########
@@ -467,6 +467,29 @@ public void test_runAllFully_futureCancel() throws
InterruptedException
Assertions.assertEquals(0, exec.cancelableProcessorCount());
}
+ @Test
+ public void test_runAllFully_cancelWhileWaitingForFirstProcessor()
Review Comment:
This covers runnables parked on `processorManager.next()` with the cancel
coming from `exec.cancel(cancellationId)`. The fix also closes a second path
that no test reaches: the result future is canceled directly while the
runnables wait for a bouncer ticket. Before the fix, `finished` stayed null, so
when the ticket was released the listener queued it, resubmitted the runnable,
and launched the processor after cancellation. `exec.cancel` hides this,
because the ticket future is registered under the same cancellation ID and gets
canceled too.
Something like this fails in all 360 parameterizations without the fix and
passes with it:
```java
@Test
public void test_runAllFully_cancelWhileWaitingForTicket() throws Exception
{
final Bouncer localBouncer = new Bouncer(1);
final Bouncer.Ticket heldTicket = localBouncer.ticket().get();
final ListenableFuture<Long> future = exec.runAllFully(
possiblyDelay(
ensureClose(
ProcessorManagers.of(Sequences.simple(List.of(new
SleepyFrameProcessor())))
.withAccumulation(0L, Long::sum)
)
),
maxOutstandingProcessors,
localBouncer,
null
);
Assertions.assertTrue(future.cancel(true));
Assertions.assertEquals(1, closed.get(), "Processor manager closed");
// The released ticket must go back to the bouncer, not to a newly
launched processor.
heldTicket.giveBack();
Assertions.assertEquals(0, localBouncer.getCurrentCount(), "Bouncer
current running count");
}
```
--
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]
