Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow @ResourceLock and @Isolated to work across processes #4211

Open
Arthurm1 opened this issue Dec 19, 2024 · 10 comments
Open

Allow @ResourceLock and @Isolated to work across processes #4211

Arthurm1 opened this issue Dec 19, 2024 · 10 comments

Comments

@Arthurm1
Copy link

@ResourceLock is great for ensuring tests run with as much concurrency as possible while protecting exclusive access to resources for the duration of a single test method/class.

My issue is that it only protects the resource within that single JUnit test run. If I execute a number of different test runs simultaneously (in different processes) and those test runs have tests that use @ResourceLock("foo") then foo can be accessed at the same time from the different test runs.

This happens for me as I use Gradle and have many subprojects. Gradle allows running subprojects tests in parallel or sequentially. Running tests in parallel means each project will run its tests in its own process and so @ResourceLock("foo") will not be respected across subprojects.

Because of this I have to run each subproject's test task sequentially which is slower.

Deliverables

Ideally there would be an option on @ResourceLock to specify the level of the lock e.g. @ResourceLock(level = LOCAL | MACHINE).

For @ResourceLock("Foo", level = LOCAL), the locking would behave as it currently is and this would be the default.
For @ResourceLock("Foo", level = MACHINE) the lock on Foo would prevent any junit runs on that machine from simultaneously accessing Foo. (Perhaps using java.nio.channels.FileLock)

It would be good if this could be extended to @Isolated(level = LOCAL | MACHINE) to allow only a single test to run concurrently even during multiple simultaneous test runs.

Or is there already a way to do this?

@marcphilipp
Copy link
Member

You are right in pointing out that there is currently no way to achieve that and it would certainly be non-trivial to implement. Can you elaborate more on your use case? What kind of external resource do you require exclusive access to across subprojects?

@Arthurm1
Copy link
Author

In my case it's access to certain tables in a test database and a custom data server. I could always spin up another test DB and another data server but they can be a pain to manage.

I had a different idea on implementation.
Instead of changing the API, there could be a new configuration parameter...
junit.jupiter.execution.lock.location=/tmp/20241220-1150/

If specified, then Junit would use java.nio.channels.FileLock to take a lock on a Foo.junit file within this dir (if @ResourceLock("Foo") had been specified).
This would allow the user to decide which test runs should share locking as a different dir could be used per test run group. It would also mean JUnit wouldn't have to worry about clearing up locks left around if test runs have been interrupted as the dir location would be different next time tests are run.
Resource locking would probably be much slower if files are used but that's the trade off for added functionality. If the config param wasn't used then the current locking implementation would be used.
If the user specified file locking then it would also have the downside of forcing every use of @ResourceLock to use file locking which may not be needed. An API change like above could be made to mitigate this but whether that's necessary depends on the performance hit of using file locks.

@marcphilipp
Copy link
Member

I think using a separate file lock for each resource can lead to deadlock. For example, when two processes A and B hold the global read lock (which is done on the engine level), A has a read lock for Foo, B request a write lock for Foo while A requests the global write lock.

@mpkorstanje
Copy link
Contributor

In my case it's access to certain tables in a test database and a custom data server. I could always spin up another test DB and another data server but they can be a pain to manage.

This happens for me as I use Gradle and have many subprojects.

With both a external resource that can't be shared, and a project with independent modules that can't communicate you have an architecture with conflicting modes of concurrency.

This makes me think you need to wag the dog a bit and change your architecture. Either by:

  • moving all the tests that use the external resource into their own module,
  • or by duplicating the external resource for each module.

For the latter, without knowing any more specifics, this sounds like a problem TestContainers could solve.

@Arthurm1
Copy link
Author

Do you mean something like...

@ResourceLock("Foo", mode = ResourceAccessMode.READ)
class TestClass {

  @ResourceLock("Foo", mode = ResourceAccessMode.READ_WRITE)
  public void TestMethod() {
    // some test
  }
}

If both process A and B are running this same test then they could both start the class test suite TestClass with read locks on Foo (which is fine) and then both would try and get a write lock for method TestMethod and they'd both wait forever because they can't get a write lock until the read locks have been released.

How does that not deadlock at the moment with JUnit tests running in parallel in a single process? Or does JUnit work out their lock requirements in a pre-run phase and make sure that they're not run in parallel?

I guess if file locking were used in the above scenario, the pre-run phase could identify any mid-test switches from Read to Read/Write and change them to run only as Read/Write. It would cut down on concurrency but it would prevent deadlocks.

If @Isolated were included in this functionality then all @Isolated tests could take a write lock on isolated.junit and all @ResourceLock tests could take a read lock on that same file. So @ResourceLock would be locking 2 files.

@Arthurm1
Copy link
Author

  • or by duplicating the external resource for each module.

Setting up multiple duplicate test databases is another option and probably what I'll have to do if this comes to nothing.

Ideally JUnit would handle this - I don't think inter-process resource locking is out of scope for unit testing (especially when that resource is a database) but I haven't seen any requests for it in the issues before so maybe it is a niche requirement.

@mpkorstanje
Copy link
Contributor

How does that not deadlock at the moment with JUnit tests running in parallel in a single process? Or does JUnit work out their lock requirements in a pre-run phase and make sure that they're not run in parallel?

That is correct. For your example the READ_WRITE will cause all tests in TestClass to be run on the same thread.

Ideally JUnit would handle this - I don't think inter-process resource locking is out of scope for unit testing

I don't think it is possible for n arbitrary Java processes to coordinate usage of an arbitrary number of resources without either a n+1 process to arbitrate or by electing a leader from the n processes.

The first isn't possible, the second rather complicated. Probably out of scope for a test framework.

@Arthurm1
Copy link
Author

Arthurm1 commented Dec 20, 2024

That is correct. For your example the READ_WRITE will cause all tests in TestClass to be run on the same thread.

Ah - and it's done this way because purely using locks would lead to a situation where you could have all but 1 of your threads waiting on the resource to be unlocked? So you could massively underuse your CPU.

I guess that does make it difficult to have inter-process locks without some process mediating.

Are there other reasons for choosing this design of forcing tests using a Read/Write resourceLock onto a single thread and ensuring that tests using a Read resourceLock didn't run concurrently with that thread?
I wonder if virtual threads might make thread blocking and CPU underutilisation a non-issue and would simplify code in this area. Obvs it's not compatible with earlier JVMs and there are still thread pinning issues so certain types of test code could be problematic. Are there any plans for virtual thread use or are there problems with it? Ah - I see Virtual Threads: #3442

@Arthurm1
Copy link
Author

@marcphilipp I see you're ex-Gradle.

The reason this issue hit me is that I enabled the Gradle configuration cache. Although it's not enabled by default, it seems like it's eventually going to be.
Currently the cache causes all subprojects test tasks to be run in parallel regardless of other Gradle concurrency settings. I don't know if this will change (see gradle/gradle#23138) but if it doesn't then I could see other Gradle users running into this kind of problem.

@marcphilipp
Copy link
Member

I don't think it is possible for n arbitrary Java processes to coordinate usage of an arbitrary number of resources without either a n+1 process to arbitrate or by electing a leader from the n processes.

The first isn't possible, the second rather complicated. Probably out of scope for a test framework.

I agree with @mpkorstanje on this.

The reason this issue hit me is that I enabled the Gradle configuration cache. Although it's not enabled by default, it seems like it's eventually going to be.
Currently the cache causes all subprojects test tasks to be run in parallel regardless of other Gradle concurrency settings. I don't know if this will change (see gradle/gradle#23138) but if it doesn't then I could see other Gradle users running into this kind of problem.

That's a good point. However, resource locks were not intended to provide cross-process synchronization. You'd have the same problem in a single project if you set maxParallelForks on Gradle's Test task to a value greater than one.

Using separate test databases would be my recommendation for addressing this. If there's only a few tasks and you're not using maxParallelForks, you might be able to use Gradle's mustRunAfter to force the test tasks to run sequentially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants