yihua commented on code in PR #13768:
URL: https://github.com/apache/hudi/pull/13768#discussion_r2299363463
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestHoodieLockMetrics.java:
##########
@@ -67,4 +73,209 @@ public void testMetricsMisses() {
assertDoesNotThrow(lockMetrics::updateLockAcquiredMetric);
}
+ @Test
+ public void testNewErrorMetrics() {
+ HoodieStorage storage = mock(HoodieStorage.class);
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(true).build();
+ HoodieLockMetrics lockMetrics = new
HoodieLockMetrics(HoodieWriteConfig.newBuilder()
+ .forTable("idk").withPath("/dsfasdf/asdf")
+ .withMetricsConfig(metricsConfig)
+ .build(), storage);
+
+ // Test all the new error metrics methods
+ assertDoesNotThrow(lockMetrics::updateLockAcquiredByOthersErrorMetric,
+ "updateLockAcquiredByOthersErrorMetric should not throw");
+ assertDoesNotThrow(lockMetrics::updateLockStateUnknownMetric,
+ "updateLockStateUnknownMetric should not throw");
+ assertDoesNotThrow(lockMetrics::updateLockAcquirePreconditionFailureMetric,
+ "updateLockAcquirePreconditionFailureMetric should not throw");
+ assertDoesNotThrow(lockMetrics::updateLockProviderFatalErrorMetric,
+ "updateLockProviderFatalErrorMetric should not throw");
+ }
+
+ @Test
+ public void testNewErrorMetricsWithDisabledMetrics() {
+ // Test that the new metrics methods work safely when metrics are disabled
+ HoodieStorage storage = mock(HoodieStorage.class);
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(false).build();
+ HoodieLockMetrics lockMetrics = new
HoodieLockMetrics(HoodieWriteConfig.newBuilder()
+ .forTable("idk").withPath("/dsfasdf/asdf")
+ .withMetricsConfig(metricsConfig)
+ .build(), storage);
+
+ // All methods should be safe to call even when metrics are disabled
+ assertDoesNotThrow(lockMetrics::updateLockAcquiredByOthersErrorMetric,
+ "updateLockAcquiredByOthersErrorMetric should not throw when metrics
disabled");
+ assertDoesNotThrow(lockMetrics::updateLockStateUnknownMetric,
+ "updateLockStateUnknownMetric should not throw when metrics disabled");
+ assertDoesNotThrow(lockMetrics::updateLockAcquirePreconditionFailureMetric,
+ "updateLockAcquirePreconditionFailureMetric should not throw when
metrics disabled");
+ assertDoesNotThrow(lockMetrics::updateLockProviderFatalErrorMetric,
+ "updateLockProviderFatalErrorMetric should not throw when metrics
disabled");
+ }
+
+ @Test
+ public void testCombinedMetricsScenario() {
+ HoodieStorage storage = mock(HoodieStorage.class);
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(true).build();
+ HoodieLockMetrics lockMetrics = new
HoodieLockMetrics(HoodieWriteConfig.newBuilder()
+ .forTable("idk").withPath("/dsfasdf/asdf")
+ .withMetricsConfig(metricsConfig)
+ .build(), storage);
+
+ // Test a realistic scenario combining old and new metrics
+ assertDoesNotThrow(() -> {
+ // Start lock acquisition
+ lockMetrics.startLockApiTimerContext();
+
+ // Lock acquisition failed due to unknown state
+ lockMetrics.updateLockStateUnknownMetric();
+ lockMetrics.updateLockNotAcquiredMetric();
+
+ // Retry - start another acquisition attempt
+ lockMetrics.startLockApiTimerContext();
+
+ // This time failed due to precondition failure
+ lockMetrics.updateLockAcquirePreconditionFailureMetric();
+ lockMetrics.updateLockNotAcquiredMetric();
+
+ // Final attempt - lock acquired by others
+ lockMetrics.startLockApiTimerContext();
+ lockMetrics.updateLockAcquiredByOthersErrorMetric();
+ lockMetrics.updateLockNotAcquiredMetric();
+
+ }, "Combined metrics scenario should not throw");
+ }
+
+ @Test
+ public void testLockInterruptedMetric() {
+ HoodieStorage storage = mock(HoodieStorage.class);
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(true).build();
+ HoodieWriteConfig writeConfig = HoodieWriteConfig.newBuilder()
+ .forTable("testTable").withPath("/test/path")
+ .withMetricsConfig(metricsConfig)
+ .build();
+ HoodieLockMetrics lockMetrics = new HoodieLockMetrics(writeConfig,
storage);
+
+ // Get the metrics registry to verify counter values
+ Metrics metrics = Metrics.getInstance(metricsConfig, storage);
+ MetricRegistry registry = metrics.getRegistry();
+ String metricName = writeConfig.getMetricReporterMetricsNamePrefix() + "."
+ HoodieLockMetrics.LOCK_INTERRUPTED;
+
+ // Test that the interrupted metric can be called
+ assertDoesNotThrow(lockMetrics::updateLockInterruptedMetric,
+ "updateLockInterruptedMetric should not throw");
+
+ // Verify the counter exists and increments
+ Counter interruptedCounter = registry.getCounters().get(metricName);
+ assertNotNull(interruptedCounter, "Lock interrupted counter should exist");
+
+ long initialCount = interruptedCounter.getCount();
+
+ // Call the metric multiple times
+ lockMetrics.updateLockInterruptedMetric();
+ lockMetrics.updateLockInterruptedMetric();
+ lockMetrics.updateLockInterruptedMetric();
+
+ // Verify the counter incremented
+ assertEquals(initialCount + 3, interruptedCounter.getCount(),
+ "Lock interrupted counter should increment by 3");
+ }
+
+ @Test
+ public void testLockExpirationDeadlineGauge() {
+ HoodieStorage storage = mock(HoodieStorage.class);
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(true).build();
+ HoodieWriteConfig writeConfig = HoodieWriteConfig.newBuilder()
+ .forTable("testTable").withPath("/test/path")
+ .withMetricsConfig(metricsConfig)
+ .build();
+ HoodieLockMetrics lockMetrics = new HoodieLockMetrics(writeConfig,
storage);
+
+ // Test that the method doesn't throw
+ assertDoesNotThrow(() ->
lockMetrics.updateLockExpirationDeadlineMetric(5000),
+ "updateLockExpirationDeadlineMetric should not throw");
+
+ // Test multiple updates
+ assertDoesNotThrow(() -> {
+ lockMetrics.updateLockExpirationDeadlineMetric(5000);
+ lockMetrics.updateLockExpirationDeadlineMetric(15000);
+ lockMetrics.updateLockExpirationDeadlineMetric(500);
+ lockMetrics.updateLockExpirationDeadlineMetric(-1000);
+ lockMetrics.updateLockExpirationDeadlineMetric(1);
+ }, "Multiple updateLockExpirationDeadlineMetric calls should not throw");
+
+ // Get the metrics registry to verify gauge exists
+ Metrics metrics = Metrics.getInstance(metricsConfig, storage);
+ MetricRegistry registry = metrics.getRegistry();
+ String metricName = writeConfig.getMetricReporterMetricsNamePrefix() + "."
+ HoodieLockMetrics.LOCK_EXPIRATION_DEADLINE;
+
+ // Verify the gauge exists
+ Gauge<?> deadlineGaugeRaw = registry.getGauges().get(metricName);
+ assertNotNull(deadlineGaugeRaw, "Lock expiration deadline gauge should
exist");
+
+ // Test final value (should be 0 from last update)
+ assertEquals(1, ((Number) deadlineGaugeRaw.getValue()).intValue(), "Final
gauge value should be 1");
+ }
+
+ @Test
+ public void testLockDanglingMetric() {
+ HoodieStorage storage = mock(HoodieStorage.class);
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(true).build();
+ HoodieWriteConfig writeConfig = HoodieWriteConfig.newBuilder()
+ .forTable("testTable").withPath("/test/path")
+ .withMetricsConfig(metricsConfig)
+ .build();
+ HoodieLockMetrics lockMetrics = new HoodieLockMetrics(writeConfig,
storage);
+
+ // Get the metrics registry to verify counter values
+ Metrics metrics = Metrics.getInstance(metricsConfig, storage);
+ MetricRegistry registry = metrics.getRegistry();
+ String metricName = writeConfig.getMetricReporterMetricsNamePrefix() + "."
+ HoodieLockMetrics.LOCK_DANGLING;
+
+ // Test that the dangling metric can be called
+ assertDoesNotThrow(lockMetrics::updateLockDanglingMetric,
+ "updateLockDanglingMetric should not throw");
+
+ // Verify the counter exists and increments
+ Counter danglingCounter = registry.getCounters().get(metricName);
+ assertNotNull(danglingCounter, "Lock dangling counter should exist");
+
+ long initialCount = danglingCounter.getCount();
+
+ // Call the metric multiple times
+ lockMetrics.updateLockDanglingMetric();
+ lockMetrics.updateLockDanglingMetric();
+
+ // Verify the counter incremented
+ assertEquals(initialCount + 2, danglingCounter.getCount(),
+ "Lock dangling counter should increment by 2");
+ }
+
+ @Test
+ public void testNewMetricsWithDisabledLocking() {
+ HoodieStorage storage = mock(HoodieStorage.class);
+ // Test that the new metrics methods work safely when locking metrics are
disabled
+ HoodieMetricsConfig metricsConfig =
HoodieMetricsConfig.newBuilder().withPath("/test")
+
.withReporterType(MetricsReporterType.INMEMORY.name()).withLockingMetrics(false).build();
+ HoodieLockMetrics lockMetrics = new
HoodieLockMetrics(HoodieWriteConfig.newBuilder()
+ .forTable("testTable").withPath("/test/path")
+ .withMetricsConfig(metricsConfig)
+ .build(), storage);
+
+ // All new methods should be safe to call even when locking metrics are
disabled
+ assertDoesNotThrow(lockMetrics::updateLockInterruptedMetric,
+ "updateLockInterruptedMetric should not throw when locking metrics
disabled");
+ assertDoesNotThrow(() ->
lockMetrics.updateLockExpirationDeadlineMetric(5000),
+ "updateLockExpirationDeadlineMetric should not throw when locking
metrics disabled");
+ assertDoesNotThrow(lockMetrics::updateLockDanglingMetric,
+ "updateLockDanglingMetric should not throw when locking metrics
disabled");
+ }
+
Review Comment:
nit: remove new line
--
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]
