[GitHub] [flink] xintongsong commented on a diff in pull request #20110: [FLINK-28308] Introduce metrics of the accumulated time that a running task is busy / idle / back-pressured

[GitBox <gi...@apache.org>]

xintongsong commented on code in PR #20110:
URL: https://github.com/apache/flink/pull/20110#discussion_r911560015


##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/TaskIOMetricGroup.java:
##########
@@ -169,6 +186,15 @@ public long getBackPressuredTimeMsPerSecond() {
                 + getHardBackPressuredTimePerSecond().getValue();
     }
 
+    public long getAccumulatedBackPressuredTimeMs() {
+        return getSoftBackPressuredTimePerSecond().getAccumulatedCount()
+                + getHardBackPressuredTimePerSecond().getAccumulatedCount();
+    }
+
+    public void markTaskStart() {
+        this.taskStartTime = System.currentTimeMillis();

Review Comment:
   Minor: I'd suggest to add an assertion (`checkState(this.taskStartTime == 0)`) to make sure this can only be called once.



##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/TaskIOMetricGroup.java:
##########
@@ -178,6 +204,17 @@ private double getBusyTimePerSecond() {
         return busyTimeEnabled ? 1000.0 - Math.min(busyTime, 1000.0) : Double.NaN;
     }
 
+    private double getAccumulatedBusyTime() {
+        return busyTimeEnabled
+                ? Math.max(
+                        System.currentTimeMillis()
+                                - taskStartTime

Review Comment:
   Minor: I'd suggest to add an assertion (`checkState(this.taskStartTime != 0)`) to make sure `markTaskStart()` has been called.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org