xintongsong commented on a change in pull request #15249:
URL: https://github.com/apache/flink/pull/15249#discussion_r595734474
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/ClusterResourceStatisticsProvider.java
##########
@@ -50,4 +52,7 @@
/** Get profile of total pending resources. */
ResourceProfile getPendingResource();
+
+ /** Get allocate slots information from the TaskManager with the given
instance id. */
+ Collection<TaskManagerSlotInformation> getAllocatedSlotsOf(InstanceID
instanceId);
Review comment:
This is not **statistics**.
I'm not entirely sure whether we need to add this interface. Given that it
is only used in `FineGrainedSlotManager#getAllocatedSlotsOf`, we may consider
to move the implementation of
`FineGrainedTaskManagerTracker#getAllocatedSlotsOf` to `FineGrainedSlotManager`.
I'm also fine with adding `getAllocatedSlotsOf` as an shortcut interface. In
that case, we might consider to 1) move it to
`TaskManagerResourceInfoProvider`, and 2) provide a default implementation
basing on `getRegisteredTaskManager`.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/DeclarativeSlotManager.java
##########
@@ -717,6 +717,12 @@ public ResourceProfile getFreeResourceOf(InstanceID
instanceID) {
return taskExecutorManager.getTotalFreeResourcesOf(instanceID);
}
+ @Override
+ public Collection<TaskManagerSlotInformation>
getAllocatedSlotsOf(InstanceID instanceID) {
+ // This information is only valid for fine-grained resource management.
Review comment:
I would say it's currently not supported for this slot manager. The
information should always be valid.
Same for `SlotManagerImpl`.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/slotmanager/SlotManager.java
##########
@@ -70,6 +71,8 @@
ResourceProfile getFreeResourceOf(InstanceID instanceID);
+ Collection<TaskManagerSlotInformation> getAllocatedSlotsOf(InstanceID
instanceID);
Review comment:
This interface exposes `TaskManagerSlotInformation`, which should be
internal to slot manager.
I'd suggest to return `SlotInfo` here.
----------------------------------------------------------------
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]
