masteryhx commented on code in PR #20965:
URL: https://github.com/apache/flink/pull/20965#discussion_r990748412
##########
flink-state-backends/flink-statebackend-common/src/main/java/org/apache/flink/state/common/PeriodicMaterializationManager.java:
##########
@@ -251,6 +254,8 @@ private void asyncMaterializationPhase(
target.handleMaterializationResult(
snapshotResult,
materializationID, upTo);
metrics.reportCompletedMaterialization();
+
metrics.reportMaterializationDuration(
Review Comment:
I think we should report not only the successed materialization but also the
failed and one not be called actually due to no state update.
the metric name is "lastMaterializationDuration" not
"lastSuccessedMaterializationDuration" or
"lastCompletedMaterializationDuration".
WDYT?
##########
docs/content.zh/docs/ops/metrics.md:
##########
@@ -1581,6 +1581,11 @@ Note that the metrics are only available via reporters.
<td>The number of failed materializations.</td>
<td>Counter</td>
</tr>
+ <tr>
+ <td>lastMaterializationDuration</td>
Review Comment:
A minor suggestion: the name could be consistent with the metric of the size
of materialization. like "lastDurationOfMaterialization" ?
##########
docs/content.zh/docs/ops/metrics.md:
##########
@@ -1581,6 +1581,11 @@ Note that the metrics are only available via reporters.
<td>The number of failed materializations.</td>
<td>Counter</td>
</tr>
+ <tr>
Review Comment:
The "rowspan" should also be updated so that the format of the table could
be showed correctly.
--
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]
