ctubbsii commented on code in PR #5853:
URL: https://github.com/apache/accumulo/pull/5853#discussion_r2337824534
##########
test/src/main/java/org/apache/accumulo/test/functional/FateStarvationIT.java:
##########
@@ -83,20 +85,26 @@ public void run() throws Exception {
List<Text> splits = new ArrayList<>(TestIngest.getSplitPoints(0, 100000,
67));
- List<Future<?>> futures = new ArrayList<>();
+ int numTasks = 100;
+ List<Future<?>> futures = new ArrayList<>(numTasks);
var executor = Executors.newCachedThreadPool();
+ // wait for a portion of the tasks to be ready
+ CountDownLatch startLatch = new CountDownLatch(32);
Review Comment:
also here
##########
test/src/main/java/org/apache/accumulo/test/WriteAfterCloseIT.java:
##########
@@ -141,12 +142,24 @@ public void testWriteAfterClose(TimeType timeType,
boolean killTservers, long ti
try (AccumuloClient c = Accumulo.newClient().from(props).build()) {
c.tableOperations().create(table, ntc);
- List<Future<?>> futures = new ArrayList<>();
-
- for (int i = 0; i < 100; i++) {
- futures.add(
- executor.submit(createWriteTask(i * 1000, c, table, timeout,
useConditionalWriter)));
+ int numTasks = 100;
+ List<Future<?>> futures = new ArrayList<>(numTasks);
+ // synchronize start of a portion of the tasks
+ CountDownLatch startLatch = new CountDownLatch(32);
Review Comment:
Should assert that the `32 < numTasks` or couple them in some way like
`numTasks / 4` or similar.
##########
test/src/main/java/org/apache/accumulo/test/UniqueNameAllocatorIT.java:
##########
@@ -68,10 +69,14 @@ void testConcurrentNameGeneration() throws Exception {
var executorService = Executors.newCachedThreadPool();
List<Future<Integer>> futures = new ArrayList<>();
+ // start a portion of threads at the same time
+ CountDownLatch startLatch = new CountDownLatch(32);
Review Comment:
Should couple the latch initial value to the number of tasks in some way
(`numTasks/2`) instead of using literals for here and for the loop.
##########
core/src/test/java/org/apache/accumulo/core/clientImpl/ClientTabletCacheImplTest.java:
##########
@@ -1989,8 +1990,10 @@ public void testMultithreadedLookups() throws Exception {
var executor = Executors.newCachedThreadPool();
final int lookupCount = 128;
final int roundCount = 8;
+ final int numTasks = roundCount * lookupCount;
- List<Future<CachedTablet>> futures = new ArrayList<>(roundCount *
lookupCount);
+ List<Future<CachedTablet>> futures = new ArrayList<>(numTasks);
+ CountDownLatch startLatch = new CountDownLatch(32); // start a portion of
threads at once
Review Comment:
Could add a test assertion that `32 <= numTasks` in the test in case
somebody modifies the values. Gotta make sure that somebody doesn't tweak the
numbers so that it falls below the minimum barrier to make progress.
Alternatively,
```suggestion
CountDownLatch startLatch = new CountDownLatch(numTasks / 32); // start
a portion of threads at once
```
--
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]
