[GitHub] [beam] suztomo commented on a change in pull request #14833: [BEAM-11227] Upgrade vendored gRPC to 1.36.0 with test fix

[GitBox <gi...@apache.org>]

suztomo commented on a change in pull request #14833:
URL: https://github.com/apache/beam/pull/14833#discussion_r636122911



##########
File path: runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/fn/logging/BeamFnLoggingServiceTest.java
##########
@@ -141,13 +144,16 @@ public void testMultipleClientsFailingIsHandledGracefullyByServer() throws Excep
                               .withOnError(waitForTermination::countDown)
                               .build());
               outboundObserver.onNext(createLogsWithIds(instructionId, -instructionId));
-              outboundObserver.onError(new RuntimeException("Client " + instructionId));
-              waitForTermination.await();
+              outboundObservers.add(outboundObserver);
               return null;
             });
       }
       ExecutorService executorService = Executors.newCachedThreadPool();
       executorService.invokeAll(tasks);
+      for (int i = 1; i <= 3; ++i) {
+        outboundObservers.take().onError(new RuntimeException("Client " + i));

Review comment:
       @kileys @kennknowles Would you share your thought process to come up this solution?
   
   Looking at this now, I'm feeling that 1 thread waiting for 1 latch is better (less likely to hit the rare random concurrency bug) than 3 threads waiting for 3 latches. Did you think this way?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org