From issues-return-158164-archive-asf-public=cust-asf.ponee.io@flink.apache.org Tue Mar 13 10:58:51 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id D66FB18064F for ; Tue, 13 Mar 2018 10:58:50 +0100 (CET) Received: (qmail 5319 invoked by uid 500); 13 Mar 2018 09:58:49 -0000 Mailing-List: contact issues-help@flink.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@flink.apache.org Delivered-To: mailing list issues@flink.apache.org Received: (qmail 5309 invoked by uid 99); 13 Mar 2018 09:58:49 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 13 Mar 2018 09:58:49 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 94614ED499; Tue, 13 Mar 2018 09:58:49 +0000 (UTC) From: zentol To: issues@flink.apache.org Reply-To: issues@flink.apache.org References: In-Reply-To: Subject: [GitHub] flink pull request #5687: [FLINK-8934] [flip6] Properly cancel slot requests... Content-Type: text/plain Message-Id: <20180313095849.94614ED499@git1-us-west.apache.org> Date: Tue, 13 Mar 2018 09:58:49 +0000 (UTC) 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) ---