smattheis commented on a change in pull request #18991:
URL: https://github.com/apache/flink/pull/18991#discussion_r825751243
##########
File path:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/StreamTaskTest.java
##########
@@ -1763,6 +1765,66 @@ public long calculateThroughput() {
}
}
+ /**
+ * Tests mailbox metrics latency and queue size and verifies that (1)
latency measurement is
+ * executed initially once and at least once triggered by timer, (2)
latency mean value is
+ * greater than zero and (3) mailbox size is greater than zero for some
time and eventually
+ * equals to zero.
+ *
+ * @throws Exception on {@link MockEnvironmentBuilder#build()} failure.
+ */
+ @Test
+ public void testMailboxMetrics() throws Exception {
Review comment:
1. The test is stable in the way that any additional delay will not
cause the test to fail. Note that the assertions ask for a minimum delay and
minimum number of latency measurements (>= 2) and `Thread.sleep(iteration == 2
? 10 : 1);` is only to not let the test sleep/run for too long, e.g., total
execution time is currently ~34ms. (The test would be work the same way if
there was only `Thread.sleep(10) or `Thread.sleep(20)`.
2. I don't see the point of testing with testHarness.processMail because 1)
it does not call the StreamTask#invoke method and, hence, won't perform the
initial call of the scheduleMailboxMetrics and 2) I don't see/understand how
the testHarness uses/implements timer-triggered execution of actions which is
relevant part of the code to be tested.
--
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]
