scwhittle commented on code in PR #29879:
URL: https://github.com/apache/beam/pull/29879#discussion_r1450238134
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/WorkUnitClient.java:
##########
@@ -49,4 +51,12 @@ interface WorkUnitClient {
* @return a {@link WorkItemServiceState} (e.g. a new stop position)
*/
WorkItemServiceState reportWorkItemStatus(WorkItemStatus workItemStatus)
throws IOException;
+
+ WorkerMessage
createWorkerMessageFromStreamingScalingReport(StreamingScalingReport report);
+ /**
Review Comment:
blank line above
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java:
##########
@@ -252,8 +253,8 @@ public class StreamingDataflowWorker {
private final Counter<Integer, Integer> totalAllocatedThreads;
private final Counter<Long, Long> outstandingBytes;
private final Counter<Long, Long> maxOutstandingBytes;
- private final Counter<Long, Long> outstandingBundles;
- private final Counter<Long, Long> maxOutstandingBundles;
+ private final Counter<Integer, Integer> outstandingBundles;
Review Comment:
I'm not sure if dataflow supports changing counter type in this way.
Additionally the max may be set higher (effectively unbounded) to some value
over Integer.MAX currently. It seems safer for that reason to revert the
change from Long->Integer and instead just bound to int range when populating
the scaling proto (presuming that is what motivated this change).
##########
runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/DataflowWorkUnitClientTest.java:
##########
@@ -227,6 +231,26 @@ public void testCloudServiceCallMultipleWorkItems() throws
Exception {
client.getWorkItem();
}
+ @Test
+ public void testReportWorkerMessageEmptyResponse() throws Exception {
Review Comment:
nit: remove empty response from test name
--
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]
