flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zentol <...@git.apache.org>
Subject [GitHub] flink pull request #5687: [FLINK-8934] [flip6] Properly cancel slot requests...
Date Tue, 13 Mar 2018 09:58:49 GMT
Github user zentol commented on a diff in the pull request:

    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolTest.java
    @@ -505,17 +513,21 @@ public void testFulfillingSlotRequestsWithUnusedOfferedSlots() throws
     			} 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);
     			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)


View raw message