zhuzhurk commented on a change in pull request #12278:
URL: https://github.com/apache/flink/pull/12278#discussion_r436540565



##########
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 had another thought and prefers to not make it configurable to fulfill 
request in request order.
   The FIFO order works for any scheduling strategy and is even a improvement 
for lazy from source scheduling. There was a ticket for the same purpose 
"[FLINK-13165] Complete slot requests in request order" although it did not 
fully make it at last. `LinkedHashMap` was introduced by it and eases this PR. 
`SlotPoolRequestCompletionTest` was also introduced by it and can be reused.
   This PR is more alike a fix to FLINK-13165.
   
   The orphaned allocation remapping also ensures that the fail-fast route will 
not break with this change.
   
   What do you think?




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to