[GitHub] [storm] Ethanlm commented on a change in pull request #3286: [STORM-3651] Give producerTasks in ExecutorTransferMultiThreadingTest.testExecutorTransfer more time to finish

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

Ethanlm commented on a change in pull request #3286:
URL: https://github.com/apache/storm/pull/3286#discussion_r438957156



##########
File path: storm-client/test/jvm/org/apache/storm/executor/ExecutorTransferMultiThreadingTest.java
##########
@@ -126,14 +126,15 @@ public void testExecutorTransfer() throws InterruptedException {
         }
 
         //give producers enough time to insert messages into the queue
-        executorService.awaitTermination(100, TimeUnit.MILLISECONDS);
+        executorService.awaitTermination(1000, TimeUnit.MILLISECONDS);

Review comment:
       This test intentionally submits 10 threads with a 5 thread pool to create race condition between threads. 100ms was just fine in my laptop or VM. On travis, I guess the resource is much limited so the thread contention is more observable. 1s maximum should be good enough. We should be able to lower it but since this value is just a maximum, 1s should be harmless.




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