azagrebin commented on a change in pull request #12278:
URL: https://github.com/apache/flink/pull/12278#discussion_r430441131
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolImpl.java
##########
@@ -112,6 +114,9 @@
/** The requests that are waiting for the resource manager to be
connected. */
private final LinkedHashMap<SlotRequestId, PendingRequest>
waitingForResourceManager;
+ /** Maps a request to its allocation. */
+ private final BiMap<SlotRequestId, AllocationID> requestedAllocations;
Review comment:
Looking into the implementation of `DualKeyLinkedMap` for
`waitingForResourceManager`, it seems we can just remove the first matching
`SlotRequestId` and then remap the orphaned `SlotRequestId` to its
`AllocationID`. The original insertion ordering should not suffer in
`DualKeyLinkedMap.aMap`. If so, we could remove `requestedAllocations`.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/slotpool/SlotPoolImpl.java
##########
@@ -648,26 +648,8 @@ boolean offerSlot(
slotOffer.getResourceProfile(),
taskManagerGateway);
- // check whether we have request waiting for this slot
- PendingRequest pendingRequest =
pendingRequests.removeKeyB(allocationID);
Review comment:
I am not sure about all consequences of this change for the existing
scheduling. I mean that we do not respect SlotRequestId->AllocationID by
accepting the slot offer. Would it make sense to keep this behaviour
configurable for now depending on scheduling strategy? Or this complication is
not needed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
