scwhittle commented on code in PR #34283:
URL: https://github.com/apache/beam/pull/34283#discussion_r1995469504
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/client/AbstractWindmillStream.java:
##########
@@ -236,14 +236,20 @@ protected final void executeSafely(Runnable runnable) {
}
}
- public final synchronized void maybeSendHealthCheck(Instant
lastSendThreshold) {
- if (!clientClosed && debugMetrics.getLastSendTimeMs() <
lastSendThreshold.getMillis()) {
- try {
- sendHealthCheck();
- } catch (Exception e) {
- logger.debug("Received exception sending health check.", e);
- }
- }
+ public final void maybeSendHealthCheck(Instant lastSendThreshold) {
Review Comment:
I think that we might want to have the check if a health-check should be
scheduled before scheduling on the executor.
Otherwise there may be more churn on the executor than necessary maybe
adding to contention if maybeSendHealthCheck is called more often. We probably
want to keep track if there is an active health-check with a member boolean as
well to avoid piling up in the executor since maybeSendHealthCheck is no longer
blocking and may be called more frequently than actually sending the health
checks takes.
--
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
