scwhittle commented on code in PR #29879:
URL: https://github.com/apache/beam/pull/29879#discussion_r1452166328
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/WorkUnitClient.java:
##########
@@ -49,4 +51,17 @@ interface WorkUnitClient {
* @return a {@link WorkItemServiceState} (e.g. a new stop position)
*/
WorkItemServiceState reportWorkItemStatus(WorkItemStatus workItemStatus)
throws IOException;
+ /**
Review Comment:
blank line above to separate from previous method (I'm suprised
spotlessApply doesn't care but might as well match the rest of the file).
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java:
##########
@@ -586,6 +588,15 @@ public void start() {
options.getWindmillHarnessUpdateReportingPeriod().getMillis(),
TimeUnit.MILLISECONDS);
+ workerMessageReportTimer = executorSupplier.apply("WorkerMessageTimer");
Review Comment:
this needs to be shutdown and awaited in stop() below like the other timers.
You could consider refactoring so that we have a collection of these timers
that we add to after we start them, since the shutdown/await code is repetitive
and I don't think we otherwise refer to them.
--
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]
