tillrohrmann commented on a change in pull request #7568: [FLINK-11417] Make
access to ExecutionGraph single threaded from JobMaster main thread
URL: https://github.com/apache/flink/pull/7568#discussion_r252261775
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ExecutionTest.java
##########
@@ -478,22 +489,25 @@ public void testEagerSchedulingFailureReturnsSlot()
throws Exception {
slotRequestId);
slotProvider.complete(slotRequestId,
singleLogicalSlot);
},
- executorService);
-
- final CompletableFuture<Void> schedulingFuture =
execution.scheduleForExecution(
- slotProvider,
- false,
- LocationPreferenceConstraint.ANY,
- Collections.emptySet());
-
- try {
- schedulingFuture.get();
- // cancel the execution in case we could
schedule the execution
- execution.cancel();
- } catch (ExecutionException ignored) {
- }
-
- assertThat(returnedSlotFuture.get(),
is(equalTo(slotRequestIdFuture.get())));
+ executorService).thenRunAsync(() -> {
+ try {
+ final CompletableFuture<Void>
schedulingFuture = execution.scheduleForExecution(
+ slotProvider,
+ false,
+
LocationPreferenceConstraint.ANY,
+ Collections.emptySet());
+
+ try {
+ schedulingFuture.get();
+ // cancel the execution
in case we could schedule the execution
+ execution.cancel();
+ } catch (ExecutionException
ignored) {
+ }
+
+
assertThat(returnedSlotFuture.get(), is(equalTo(slotRequestIdFuture.get())));
+ } catch (Exception ex) {
+ }
+ }, testMainThreadUtil.getMainThreadExecutor());
Review comment:
Chaining these two calls renders this test useless, because the second call
back triggers the first one. I would recommend to try again to get rid of the
concurrency here. What the test should test is that in non-queued scheduling
that the scheduling should fail if it gets an uncompleted future back and that
a later completion of this future with a slot will return this slot back to the
`SlotOwner`. We should be able to do this with having a `SlotProvider` which
returns a configurable future which we can control when it is completed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
