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 prefer 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. It is not just for pipelined region scheduling.
   There once was a ticket/PR 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.
   
   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