DomGarguilo commented on code in PR #5853:
URL: https://github.com/apache/accumulo/pull/5853#discussion_r2325462134
##########
core/src/test/java/org/apache/accumulo/core/crypto/CryptoTest.java:
##########
@@ -539,14 +540,18 @@ private void testMultipleThreads(Scope scope) throws
Exception {
var executor = Executors.newCachedThreadPool();
- List<Future<Boolean>> verifyFutures = new ArrayList<>();
+ final int numTasks = 32;
+ List<Future<Boolean>> verifyFutures = new ArrayList<>(numTasks);
+ CountDownLatch startLatch = new CountDownLatch(numTasks);
Review Comment:
Okay there are a few things here.
1. The deadlock does occur on a `Executors.newFixedThreadPool(X)` when
waiting for a task count greater than X. This deadlock does not present itself
on a `Executors.newCachedThreadPool` since its able to grow in size to meet
demand.
2. `CompressionTest` is indeed not actually doing anything. It is skipping
over the test code completely when checking `if (isSupported.get(al) != null &&
isSupported.get(al))`. This was present in the test before I made any changes.
I am not familiar with whats going on here so maybe another separate ticket
should be opened if this is a bug.
3. @ctubbsii, your points are definitely valid. You touched on this but your
suggested approach does not ensure that all tasks are actually at the same
point in their operation like the current approach does. The current approach
ensures that all (or the configured number) of tasks are started and waiting at
the designated spot in their operation before releasing them. Your suggestion
is definitely simpler and more robust but may function similar to not using a
latch at all and just starting the threads.
--
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]
