fapaul commented on a change in pull request #15975:
URL: https://github.com/apache/flink/pull/15975#discussion_r636879461
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/slotmanager/DeclarativeSlotManagerTest.java
##########
@@ -1450,6 +1454,34 @@ public void testClearRequirementsClearsResourceTracker()
throws Exception {
}
}
+ @Test
+ public void testMetricsUnregisteredWhenSuspending() throws Exception {
+ testAccessMetricValueDuringItsUnregister(SlotManager::suspend);
+ }
+
+ @Test
+ public void testMetricsUnregisteredWhenClosing() throws Exception {
+ testAccessMetricValueDuringItsUnregister(AutoCloseable::close);
+ }
+
+ private void testAccessMetricValueDuringItsUnregister(
+ ThrowingConsumer<SlotManager, Exception> closeFn) throws Exception
{
+ final AtomicInteger registeredMetrics = new AtomicInteger();
+ final MetricRegistry metricRegistry =
+ TestingMetricRegistry.builder()
+ .setRegisterConsumer((a, b, c) ->
registeredMetrics.incrementAndGet())
+ .setUnregisterConsumer((a, b, c) ->
registeredMetrics.decrementAndGet())
+ .build();
+
+ final DeclarativeSlotManager slotManager =
+ createDeclarativeSlotManagerBuilder()
+ .setSlotManagerMetricGroup(
+ SlotManagerMetricGroup.create(metricRegistry,
"localhost"))
+ .buildAndStartWithDirectExec();
Review comment:
Nit: Assert that metrics number is greater than 0.
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/slotmanager/FineGrainedSlotManagerTest.java
##########
@@ -948,4 +954,41 @@ private void testMaxTotalResourceExceededRegisterResource(
}
};
}
+
+ @Test
+ public void testMetricsUnregisteredWhenSuspending() throws Exception {
+ testAccessMetricValueDuringItsUnregister(SlotManager::suspend);
+ }
+
+ @Test
+ public void testMetricsUnregisteredWhenClosing() throws Exception {
+ testAccessMetricValueDuringItsUnregister(AutoCloseable::close);
+ }
+
+ private void testAccessMetricValueDuringItsUnregister(
+ ThrowingConsumer<SlotManager, Exception> closeFn) throws Exception
{
+ final AtomicInteger registeredMetrics = new AtomicInteger();
+ final MetricRegistry metricRegistry =
+ TestingMetricRegistry.builder()
+ .setRegisterConsumer((a, b, c) ->
registeredMetrics.incrementAndGet())
+ .setUnregisterConsumer((a, b, c) ->
registeredMetrics.decrementAndGet())
+ .build();
+
+ final Context context = new Context();
+ context.setSlotManagerMetricGroup(
+ SlotManagerMetricGroup.create(metricRegistry, "localhost"));
+
+ context.runTest(
+ () -> {
+ context.runInMainThreadAndWait(
+ () -> {
+ try {
+ closeFn.accept(context.getSlotManager());
Review comment:
Nit: Assert that the number of registered metrics is greater than 0
before calling the closing method.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
