XComp commented on a change in pull request #13547:
URL: https://github.com/apache/flink/pull/13547#discussion_r506387189
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskExecutor.java
##########
@@ -1776,6 +1776,25 @@ public ResourceID getResourceID() {
return unresolvedTaskManagerLocation.getResourceID();
}
+ public long getUsedManagedMemory() {
Review comment:
I understand what you mean saying that the `TaskExecutor` is already
kind of busy doing other stuff. I'm just not sure whether it makes sense to
move the logic somewhere else. It is actually a property of `TaskExecutor`.
Alternatively, we could think about moving the logic into `TaskSlotTable`.
Semantically, it would fit in there even more. Instead of
`MetricUtils.instantiateManagedMemoryMetrics(MetricGroup, TaskExecutor)` we
could use `MetricUtils.instantiateManagedMemoryMetrics(MetricGroup,
TaskSlotTable)` instead. What do you think about that?
I would object moving the aggregating code into `MetricUtils`. The sole
purpose of `MetricUtils` seem to be that it instantiates metrics. There is no
other code. Hence, I'd like to keep it that way.
----------------------------------------------------------------
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]
