search

apache / maven-mvnd

Apache Maven Daemon
https://maven.apache.org/
Apache License 2.0
2.9k stars 207 forks source link

Fix daemon connection issues #1071

Closed oehme closed 2 months ago

oehme commented 2 months ago

We have a test suite for a Maven extension that runs around 5000 builds and before this change, on each CI build, about 100 of them would fail with daemon connection issues. The symptom would always be the same - The daemon is waiting for the build request, the client has already sent it, but the daemon never receives it, no matter how long we configured the timeouts.

The reason turned out to be the way that the daemon waits for the request:

The Server was using a SynchrnousQueue to coordinate the main thread and the background thread that receives the request from the client. A SynchronousQueue only allows insertions when a corresponding call to get is in progress. However, since the receiver thread is started before the call to get, there was a short time window, where the call to queue.offer could fail and simply return false. This return code was ignored.

A possible solution would have been to call put instead of offer, but I decided to replace the queue with a Future, since we only wait for a single element.

gnodet commented 2 months ago

It seems much less stable on Windows for some reason...

oehme commented 2 months ago

Yep, looks like I'll have to dig into the Windows behavior some more. Taking back into draft.

oehme commented 2 months ago

After a lot of debugging, I've finally figured out the true cause of this, see the updated description.

I guess my previous attempt had simply changed the timing slightly, reducing the flakiness on my machine, but increasing it in the Windows builds, which explains those failures.

gnodet commented 2 months ago

After a lot of debugging, I've finally figured out the true cause of this, see the updated description.

I guess my previous attempt had simply changed the timing slightly, reducing the flakiness on my machine, but increasing it in the Windows builds, which explains those failures.

Really nice catch.

However, even if guava is a transitive dependencies, it's not really a direct dependency in maven. I would really prefer if we could use a JDK collection somehow, maybe new ArrayBlockingQueue(1) ?

oehme commented 2 months ago

Good point, changed to CompletableFuture

ppalaga commented 2 months ago

Indeed, great catch @oehme 🙏