zhuzhurk commented on a change in pull request #12278:
URL: https://github.com/apache/flink/pull/12278#discussion_r433607269
##########
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 think it's a good idea to make it configurable.
Besides the benefit to reduce risk for streaming and DataSet jobs, another
benefit is that we can also drop the change to remap orphaned allocations. This
is because the remapping is for fail-fast of pending requests in
`failAllocation(...)` which makes difference only if it is a streaming job.
Actually I'm thinking whether we need to keep the fail-fast mechanism in
`failAllocation(...)` in the future. It requires the slot pool to differentiate
streaming requests and batch requests. And in the future if a slotpool contains
both batch slots(occupied temporarily) and streaming slots(occupied
indefinitely), a failed allocation for streaming request does not need to fail
immediately if it is still fulfillable, just like how we currently deal with
batch requests.
What do you think of dropping the commit to remap orphaned allocations?
----------------------------------------------------------------
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
