hudi-agent commented on code in PR #20148:
URL: https://github.com/apache/hudi/pull/20148#discussion_r4151312045
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/StreamWriteOperatorCoordinator.java:
##########
@@ -457,6 +464,8 @@ private void initHiveSync() {
.threadFactory(getThreadFactory("hive-sync"))
.waitForTasksFinish(true).build();
this.hiveSyncContext = HiveSyncContext.create(conf, this.storageConf);
+ this.hiveSyncMetrics = new FlinkHiveSyncMetrics(context.metricGroup());
+ this.hiveSyncMetrics.registerMetrics();
Review Comment:
🤖 I think these metrics can go stale under the AdaptiveScheduler. Flink
reuses the coordinator metric group (`getOrAddOperator` +
`addGroup("coordinator")`) when it rebuilds the ExecutionGraph on a restart or
rescale. When the new coordinator registers here, `addMetric` hits a name
collision and keeps the old, closed coordinator's counters and gauge.
`hiveSyncLastSuccessTimeMs` would then freeze after the first restart. Could
you look into whether a recreated coordinator can reuse the existing metric
objects?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/TestStreamWriteOperatorCoordinator.java:
##########
@@ -1009,6 +1084,34 @@ private static StreamWriteOperatorCoordinator
createCoordinator(Configuration co
return new StreamWriteOperatorCoordinator(conf, coordinatorContext);
}
+ /**
+ * A coordinator metric group that keeps what is registered on it.
+ */
+ private static final class CapturingCoordinatorMetricGroup extends
UnregisteredMetricsGroup
Review Comment:
🤖 nit: this is almost the same as `CapturingMetricGroup` in
`TestFlinkHiveSyncMetrics`. Could we move it into one shared test utility (e.g.
under `org.apache.hudi.utils`) and have both tests use it? Then the metric-name
constants could be public, so this test wouldn't need hardcoded strings like
"hiveSyncFailureCount".
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
