[GitHub] [geode] kohlmu-pivotal commented on a change in pull request #5691: GEODE-8672: No need in token mode if concurrencyChecksEnabled

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

kohlmu-pivotal commented on a change in pull request #5691:
URL: https://github.com/apache/geode/pull/5691#discussion_r516206148



##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/control/RebalanceOperationDistributedTest.java
##########
@@ -2820,68 +2823,58 @@ private void doRebalance() throws TimeoutException, InterruptedException {
 
   private void doConcurrentPutInTx(String s) throws Exception {
     for (int i = 0; i < totalNumberOfBuckets; i++) {
-      synchronized (list) {
-        list.add(i);
-      }
-    }
-
-    Thread[] threads = new Thread[totalNumberOfBuckets];
-    for (int i = 0; i < totalNumberOfBuckets; i++) {
-      threads[i] = new Thread(() -> doPutOpInTx(s));
+      queue.add(i);
     }
 
+    ExecutorService pool = Executors.newCachedThreadPool();
+    Future<?>[] futures = new Future<?>[totalNumberOfBuckets];
     for (int i = 0; i < totalNumberOfBuckets; i++) {
-      threads[i].start();
+      futures[i] = pool.submit(() -> doPutOpInTx(s));

Review comment:
       I was more thinking in the line of using the `invokeAll` rather than the single `submit`. With `invokeAll` you can create all the operations up front and then invoke all of them at the same time.
   
   Are we not concerned that if we do it this way, that there is a possibility that the concurrency issue is possibly not ever hit?




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