[GitHub] flink pull request #5687: [FLINK-8934] [flip6] Properly cancel slot requests...
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/5687 ---
[GitHub] flink pull request #5687: [FLINK-8934] [flip6] Properly cancel slot requests...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5687#discussion_r174060373
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolTest.java
---
@@ -505,17 +513,21 @@ public void
testFulfillingSlotRequestsWithUnusedOfferedSlots() throws Exception
} catch (ExecutionException ee) {
// expected
assertTrue(ExceptionUtils.stripExecutionException(ee) instanceof
FlinkException);
-
}
- final SlotOffer slotOffer = new SlotOffer(allocationId,
0, ResourceProfile.UNKNOWN);
+ assertEquals(allocationId1,
canceledSlotRequests.take());
+
+ final SlotOffer slotOffer = new
SlotOffer(allocationId1, 0, ResourceProfile.UNKNOWN);
slotPoolGateway.registerTaskManager(taskManagerLocation.getResourceID()).get();
assertTrue(slotPoolGateway.offerSlot(taskManagerLocation, taskManagerGateway,
slotOffer).get());
// the slot offer should fulfill the second slot request
- assertEquals(allocationId,
slotFuture2.get().getAllocationId());
+ assertEquals(allocationId1,
slotFuture2.get().getAllocationId());
+
+ // check that the second slot request has been canceled
--- End diff --
Let's see if i understood the scenario here correctly:
We request the allocation of 2 slots. We cancel the first allocation
request, but a TaskManager has already offered a slot to fulfill it.
We now have one pending allocation request and one offered slot, so we
re-use the slot for the second request, which we can do since both requests
were for the same job with the same resource requirements.
I think we can improve the wording a bit though, as this comment here says
that the second request has been canceled, when just above it was fulfilled. I
guess it should say that the slot _acquisition_ (i.e. the retrieval of slots
from the RM) has been canceled. (also applies to the canceledSlotRequests
variable)
---
[GitHub] flink pull request #5687: [FLINK-8934] [flip6] Properly cancel slot requests...
GitHub user tillrohrmann opened a pull request: https://github.com/apache/flink/pull/5687 [FLINK-8934] [flip6] Properly cancel slot requests of otherwisely fulfilled requests ## What is the purpose of the change Cancel slot requests at the ResourceManager if they have been completed with a different allocation. cc @zentol ## Verifying this change - Adapted `SlotPoolTest#testFulfillingSlotRequestsWithUnusedOfferedSlots` to check cancellation ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no) - The serializers: (no) - The runtime per-record code paths (performance sensitive): (no) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (no) - If yes, how is the feature documented? (not applicable) You can merge this pull request into a Git repository by running: $ git pull https://github.com/tillrohrmann/flink fixSlotRequestCancellation Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/5687.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #5687 commit 95148aa8d6a217e4002ad823aacb3f7e0aaf4d9e Author: Till Rohrmann Date: 2018-03-13T07:13:44Z [FLINK-8934] [flip6] Properly cancel slot requests of otherwisely fulfilled requests Cancel slot requests at the ResourceManager if they have been completed with a different allocation. ---
