Enable opt-in concurrency for tests#4301
Draft
ScottDugas wants to merge 25 commits into
Draft
Conversation
This updates our configuration so that test classes annotated with `@Execution(ExecutionMode.CONCURRENT)` will run the tests in parallel, which, right now is 15 classes. I hope to change the default to run concurrently, but there are a variety of tests failing under that configuration, for a variety of reasons that would need to be addressed.
The actual logs get super intermingled making it impossible to know what an individual test was doing
It looks like it was getting transaction-too-old. Basic hypothesis: With concurrent work fdb takes just a little bit longer, and we can't insert the batches of 100 at the end within 5 seconds, so it gets transaction-too-old. The runAsync retries a few times, but with the other load it can't succeed. By shrinking the batch size when it gets a bit higher it succeeds.
This reverts commit 1203ed1.
Instead of hardcoding the size universally at 100 for batches, this adds a very, very simplistic work reduction to TestHelpers.basicInsertBatch. To make this a bit cleaner, it also encapsulate that method in one that just takes a list of vectors, which covers all of OperationsTest.
ScottDugas
force-pushed
the
test-concurrency-opt-in
branch
from
6503204 to
dceae4f
Compare
This was useful in debugging the failures, and we may want to revisit something similar in the future, but for now, reduce the churn in the PR
ScottDugas
force-pushed
the
test-concurrency-opt-in
branch
from
dceae4f to
e3d4977
Compare
ScottDugas
commented
Jun 30, 2026
| // When JUnit's parallel execution is enabled, lifecycle methods and the test body can run on | ||
| // different worker threads, leaving the test body without a debugger installed. Pin everything in | ||
| // this class to a single thread so the thread-local survives between @BeforeEach and @Test. | ||
| @Execution(ExecutionMode.SAME_THREAD) |
Collaborator
Author
There was a problem hiding this comment.
Note: This isn't strictly necessary at this point because we still default to SAME_THREAD. The changes below were necessary, and this future-proofs us for enabling parallelism by default.
I _think_ these ended up being unnecessary.
This reverts commit 9f7fbd1. It turns out this is necessary, all the OperationsTests (or most) timed out after 5 minutes, causing the overall build to timeout after 40. I believe this is because we have a lot of futures, and if we are doing everything in a thread pool, joining on a future directly impacts parallelism, so way less work will be done, and it could potentially run out of threads.
This reverts commit e3d4977. I'm getting test timeouts, so need more info....
In case this is what is causing the tests to pass....
The tests around Cached read version can get messed up by other tests running concurrently. I have a fix elsewhere, but want to minimize the changes in this PR.
This reverts commit eaf53c2. All the tests timed out... Maybe it's independently flaky...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates our configuration so that test classes annotated with
@Execution(ExecutionMode.CONCURRENT)will run the tests in parallel, which, right now is 15 classes.I hope to change the default to run concurrently, but there are a variety of tests failing under that configuration, for a variety of reasons that would need to be addressed.