hudi-agent commented on code in PR #18813:
URL: https://github.com/apache/hudi/pull/18813#discussion_r3285942982
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/partitioner/index/TestRecordLevelIndexBackend.java:
##########
@@ -152,6 +156,38 @@ public void testLazyEvictUsesAccessOrder() throws
Exception {
}
}
+ @Test
+ public void testConstructorPreSeedsMetricsForNullSafety() throws Exception {
+ // Without pre-seeding, bootstrapPartition would NPE before the bucket
assign operator
+ // wires the real MetricGroup via registerMetrics(MetricGroup).
+ try (RecordLevelIndexBackend backend = createBackend()) {
+ assertNotNull(backend.getMetrics());
+ }
+ }
+
+ @Test
+ public void testRegisterMetricsRegistersRemoteRliHistograms() throws
Exception {
+ CapturingMetricGroup metricGroup = new CapturingMetricGroup();
+ try (RecordLevelIndexBackend backend = createBackend()) {
+ backend.registerMetrics(metricGroup);
+
+ assertNotNull(metricGroup.getHistogram("remoteIndexLookupLatency"));
+ assertNotNull(metricGroup.getHistogram("remoteLookupKeysNum"));
+ }
+ }
+
+ @Test
+ public void testRegisterMetricsReplacesPreSeededMetrics() throws Exception {
+ try (RecordLevelIndexBackend backend = createBackend()) {
+ FlinkIndexBackendMetrics preSeeded = backend.getMetrics();
+ backend.registerMetrics(new CapturingMetricGroup());
+
+ assertNotNull(backend.getMetrics());
+ // The second call must rewire the field so the active metrics publish
to the real metric group.
Review Comment:
🤖 nit: `assertFalse(a == b)` tests reference inequality but reads a bit
awkwardly — `assertNotSame(preSeeded, backend.getMetrics())` is the idiomatic
JUnit 5 form for this and makes the intent immediately obvious.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/partitioner/index/RecordLevelIndexBackend.java:
##########
@@ -74,6 +77,8 @@ public class RecordLevelIndexBackend implements
PartitionedIndexBackend {
private final long maxCacheSizeInBytes;
private final BootstrapFilter bootstrapFilter;
private HoodieTableMetadata metadataTable;
+ @Getter
Review Comment:
🤖 nit: `@Getter` generates a `public` accessor, but `getMetrics()` only
appears in test code. Could you scope this down with
`@Getter(AccessLevel.PACKAGE)` to avoid leaking the internal metrics object
beyond what's needed?
<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]
