Abacn commented on code in PR #33187:
URL: https://github.com/apache/beam/pull/33187#discussion_r1858712636
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/streaming/harness/FanOutStreamingEngineWorkerHarness.java:
##########
@@ -195,7 +199,10 @@ static FanOutStreamingEngineWorkerHarness forTesting(
getWorkBudgetDistributor,
dispatcherClient,
workCommitterFactory,
- getDataMetricTracker);
+ getDataMetricTracker,
+ // Run the workerMetadataConsumer on the direct calling thread to
make testing more
+ // deterministic.
Review Comment:
"to make testing more deterministic" gives an impression that the change
just fix tests, however the test code path then diverts from the real one.
Please provide more information in this comment why the race observed in the
test does not affect production, for future reference.
If this indeed could happen in production then we should fix the code.
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/client/grpc/stubs/ChannelCache.java:
##########
@@ -85,7 +86,9 @@ static ChannelCache forTesting(
notification -> {
shutdownChannel(notification.getValue());
onChannelShutdown.run();
- });
+ },
+ // Run the removal on the calling thread for better determinism in
tests.
Review Comment:
same here
--
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]
