RocMarshal commented on code in PR #25218:
URL: https://github.com/apache/flink/pull/25218#discussion_r1759680801
##########
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/allocator/DefaultSlotAssigner.java:
##########
@@ -48,14 +50,30 @@ public Collection<SlotAssignment> assignSlots(
allGroups.addAll(createExecutionSlotSharingGroups(vertexParallelism,
slotSharingGroup));
}
- Iterator<? extends SlotInfo> iterator = freeSlots.iterator();
+ Iterator<? extends SlotInfo> iterator =
+ selectSlotsInMinimalTaskExecutors(freeSlots, allGroups,
Collections.emptyList())
+ .iterator();
Review Comment:
After discussion offline:
If it is in application mode and the number of slots for each TM is equal，
the commented is a good idea to optimize it.
However, there are the following situations for compatibility here:
1. Deployment mode: application & session;
2. Some user configurations may cause the number of slots on TM to be
different in session mode
- For the application mode
Sort the TM in reverse order according to the number of slots, and then
start using slots, which is a good choice because it can use the minimum number
of TMs (at job scope side/vision).
- For the session mode:
Sorting TM in ascending order based on the number of slots and then starting
to use slots is a good choice, as it allows for the minimum number of TM((at
`ResourceManager`/`session-cluster` scope side/vision)) to be used for multiple
jobs are running. It's mentioned that although this solution may not achieve
maximum benefits（the number of slots for each TM is not equal） in application
mode, it can still have quite good results(when the number of slots for each TM
is equal).
So, sorting TM in reverse order based on the number of slots and selecting
its may be a good choice.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
