azagrebin commented on a change in pull request #12679:
URL: https://github.com/apache/flink/pull/12679#discussion_r440937500
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/metrics/MetricRegistryImplTest.java
##########
@@ -161,6 +161,26 @@ public void testReporterScheduling() throws Exception {
registry.shutdown().get();
}
+ @Test
+ public void
testReporterIntervalParsingErrorDoesNotResultInPartialApplication() throws
Exception {
+ TestReporter3.reportCount = 0;
+
+ MetricConfig config = new MetricConfig();
+
config.setProperty(ConfigConstants.METRICS_REPORTER_INTERVAL_SUFFIX, "1
UNICORN");
+
+ MetricRegistryImpl registry = new MetricRegistryImpl(
+
MetricRegistryConfiguration.defaultMetricRegistryConfiguration(),
+
Collections.singletonList(ReporterSetup.forReporter("test", config, new
TestReporter3())));
+ try {
+ // in a prior implementation the time amount was
applied even if the time unit was invalid
+ // in this case this would imply using 1 SECOND as the
interval (seconds is the default)
+ Thread.sleep(2000);
+ Assert.assertEquals(0, TestReporter3.reportCount);
Review comment:
Although, it is not very probable but possible test instability if the
worker hangs for more than 10 sec.
In general, it would be nice to add a manual `Clock` to
`ManuallyTriggeredScheduledExecutor` and inject it into `MetricRegistryImpl`
constructor but this is an extra effort of course. Maybe a mocking executor
would be easier.
----------------------------------------------------------------
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]
