flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tillrohrmann <...@git.apache.org>
Subject [GitHub] flink pull request #4887: [FLINK-7870] [runtime] Cancel slot allocation to R...
Date Fri, 03 Nov 2017 11:01:15 GMT
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4887#discussion_r148754465
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/SlotManager.java
---
    @@ -302,7 +302,12 @@ public boolean unregisterSlotRequest(AllocationID allocationId) {
     		PendingSlotRequest pendingSlotRequest = pendingSlotRequests.remove(allocationId);
     
     		if (null != pendingSlotRequest) {
    -			cancelPendingSlotRequest(pendingSlotRequest);
    +			if (pendingSlotRequest.isAssigned()) {
    +				cancelPendingSlotRequest(pendingSlotRequest);
    +			}
    +			else {
    +				resourceActions.cancelResourceAllocation(pendingSlotRequest.getResourceProfile());
    --- End diff --
    
    Then this should be added as a separate feature because it is not strictly required by
this PR here.
    
    Furthermore, I'm not sure whether this shouldn't be the responsibility of the `ResourceManager`.
E.g. we could think about adding a timeout for container requests after which we cancel them.
Additionally, if we add support for starting machines with multiple slots, then we shouldn't
release a requested resource if only a single of it slots is no longer needed. That is something
else to consider before adding the cancel resource allocation method.


---

Mime
View raw message