rkhachatryan commented on code in PR #22772:
URL: https://github.com/apache/flink/pull/22772#discussion_r1234865147
##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/TaskIOMetricGroup.java:
##########
@@ -67,10 +70,12 @@ public class TaskIOMetricGroup extends
ProxyMetricGroup<TaskMetricGroup> {
private final Meter mailboxThroughput;
private final Histogram mailboxLatency;
private final SizeGauge mailboxSize;
+ private final Gauge<Long> initializingTime;
Review Comment:
1. Why is it a `Gauge` and not a `Counter`? Per my understanding,
initialization duration either grows or stays the same. And we can start
reporting some non-zero value as soon as `taskInitializeTime` is set.
2. Should it be `initializationDuration` or something like that, instead of
time?
##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/TaskIOMetricGroup.java:
##########
@@ -196,6 +204,23 @@ public void markTaskStart() {
this.taskStartTime = System.currentTimeMillis();
}
+ public void markTaskInitialize() {
+ this.taskInitializeTime = System.currentTimeMillis();
Review Comment:
It's not trivial to follow all the code paths inside `StreamTask` that could
lead to this call (`restoreInternal` can be called from `invoke`).
Should we make this update conditional - i.e. only update the field if it
wasn't already updated?
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java:
##########
@@ -695,6 +695,7 @@ void restoreInternal() throws Exception {
}
isRestoring = true;
closedOperators = false;
+
getEnvironment().getMetricGroup().getIOMetricGroup().markTaskInitialize();
Review Comment:
This marks the **start** of the initialization, right?
Should we call it something like `markTaskInitializationStarted`? (I was
confused at first)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
