scwhittle commented on code in PR #30693:
URL: https://github.com/apache/beam/pull/30693#discussion_r1577515789
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutorTest.java:
##########
@@ -153,7 +154,6 @@ public void testExecuteMapTaskExecutor() throws Exception {
inOrder.verify(o1).finish();
inOrder.verify(o2).finish();
inOrder.verify(o3).finish();
- inOrder.verify(stateTracker).reset();
Review Comment:
how about adding verification that activate/deactivate is called on the
tracker to this test?
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java:
##########
@@ -321,6 +321,11 @@ public long getNextLullReportMs() {
return nextLullReportMs;
}
+ /** Return the time of the next bundle lull report. */
Review Comment:
This sounds like an absolute time, while this is the duration after the
bundle start that the log should occur. Maybe better name would be
nextBundleLullDurationReportMs for method and variable?
this is just exposed for testing as well so you could remove the public and
have it be package private if that works. or mark with the @VisibleForTesting
annotaiton.
--
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]
