scwhittle commented on code in PR #35327:
URL: https://github.com/apache/beam/pull/35327#discussion_r2178275673
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/client/grpc/GrpcGetDataStream.java:
##########
@@ -284,8 +374,8 @@ public void
onHeartbeatResponse(List<Windmill.ComputationHeartbeatResponse> resp
}
@Override
- protected void sendHealthCheck() throws WindmillStreamShutdownException {
- if (hasPendingRequests()) {
+ protected synchronized void sendHealthCheck() throws
WindmillStreamShutdownException {
+ if (currentPhysicalStream != null &&
currentPhysicalStream.hasPendingRequests()) {
trySend(HEALTH_CHECK_REQUEST);
Review Comment:
These custom health-checks were more to make sure we werent' stuck waiting
for responses on broken streams. Sending all the time could help detect errors
on streams that are idle, reducing latency if the stream does need reconnecting
and we wait until a request is sent. But for cases with load I don't think it
woudl occur and always sending could increase the service load since I think we
currently have a lot of idle streams (such as GetData that is never used). So
I'd rather not change it unless we have more motivation.
--
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: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
