quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.77k stars 2.68k forks source link

Eclipse Maven Update "Too Many Files" with 2.7.1 #23655

Closed jimbogithub closed 2 years ago

jimbogithub commented 2 years ago

Describe the bug

Upgraded an existing 2.5.1 project to 2.7.1 and then in Eclipse, ran Maven Update. The Maven Update failed with IO errors for "Too Many Files".

On 2.5.1 lsof -p <PID> | wc -l on the Eclipse process shows just over 1,000 open files for my project. On 2.7.1 it blows through the 10,420 limit and fails. Examining the raw list of files from lsof -p <PID> shows many duplicate references to the same files which does not occur for 2.5.1. Looks like it's loading each dependent jar once for each and every project/module.

Expected behavior

Maven Update should succeed.

Actual behavior

Maven Update fails.

How to Reproduce?

Create a multi-module Maven project. Import into Eclipse. Perform "Maven Update" in Eclipse. Use lsof -p <PID> to observe loaded files.

You may not hit the "Too Many Files" error unless you add enough modules, but you'll still see the increased file count and duplicates versus the 2.5.1 behaviour.

P.S. Can be useful to open Eclipse with the -clean option between tests to ensure there aren't cached files included (mine was showing some 2.7.1 files even after switching back to 2.5.1).

Output of uname -a or ver

Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_ARM64_T6000 arm64

Output of java -version

Java(TM) SE Runtime Environment (build 17.0.2+8-LTS-86)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.7.1

Build tool (ie. output of mvnw --version or gradlew --version)

Maven 3.8.4

Additional information

Also tested on non Apple Silicon, same result.

quarkus-bot[bot] commented 2 years ago

/cc @quarkusio/devtools

geoand commented 2 years ago

cc @Postremus @aloubyansky

jimbogithub commented 2 years ago

FYI, this is still an issue in 2.7.2 and is also not resolved by upgrading the Eclipse EMBEDDED Maven to 3.8.4 (was 3.8.1) which I believe is the one used by Eclipse to resolve dependencies.

jimbogithub commented 2 years ago

Here are the lists of open files for two different copies of https://github.com/jimbogithub/sse (a reproducer I have for a different issue).

Both lists obtained from distinct newly created Eclipse Workspaces.

jimbogithub commented 2 years ago

2.6.3 does not have this issue.

famod commented 2 years ago

@Postremus FWIW, jars are loaded multiple times in that 2.7.2 dump, e.g.:

Those are loaded at most 2 times in the 2.5.1 dump.

But you probably already noticed that.

gsmet commented 2 years ago

This is concerning. @Postremus do you think you will have the time to look at it before tomorrow evening? If not, we will need to have someone else having a look.

Postremus commented 2 years ago

@gsmet My plan was to start looking at this issue in detail today. But I honestly don't think that I am able to fix this in todays remaining 3 hours. I will dig into this a bit though.

Yesterday, I did some tests with only quarkus-fs-util, found nothing so far. Calling close() on the opened fs correctly closes the ZipFileSystem. The FileSystem path itself is also not locked, i.e. I can rename the zip file.

Postremus commented 2 years ago

I guess I figured it out.

During a "Maven Update" in eclipse, the following goals are invoked:

The generate-code goal uses a QuarkusClassLoader (the dev mode classloader), which internally opens a lot of OpenArchivePathTree instances. This kind of Tree keeps the backing FileSystem open, until an explicit close invoked.

This never happens though during this goal invocation.

For cli uses, this does not matter, the whole maven process shuts down at the end of execution, and all locks are freed. In eclipse however, the Mojo instance is kept by m2e for the runtime of eclipse, meaning no OpenArchivePathTree`s will ever be closed.

This is at only my first assumption at this point. I am unable to confirm this as the root cause, since my fedora installation is out of order.

Let me prepare a fix though, which someone on linux can verify.

/cc @gsmet @aloubyansky

jimbogithub commented 2 years ago

Nice work. 2.7.3 is now showing a little over half the files open that even 2.5.1 had. Thanks.

aloubyansky commented 2 years ago

Great! Thanks for checking @jimbogithub

jimbogithub commented 2 years ago

@aloubyansky Same problem is occurring when building (mvn clean install). More mojos impacted? Raise another issue or re-open this one?

aloubyansky commented 2 years ago

mvn clean install on the Quarkus repo? From an IDE?

jimbogithub commented 2 years ago

mvn clean install on the Quarkus repo? From an IDE?

On the command line whilst building multiple Quarkus based apps as part of a multi-module build. In fact, the exact same set of projects as the original issue. I didn't notice earlier as I didn't hit the open file limit but a colleague had more things already running on his machine and hit the limit. Running lsof | grep java > open_files.txt during the build shows the same symptoms, i.e. multiple copies of jars with 2.7.x, far fewer files with 2.5.x.

aloubyansky commented 2 years ago

@jimbogithub would you be able to build Quarkus from a custom branch and test it in your builds?

jimbogithub commented 2 years ago

@jimbogithub would you be able to build Quarkus from a custom branch and test it in your builds?

Yes I could.

aloubyansky commented 2 years ago

@jimbogithub before I ask you to give it a try, do you need generate-code/generate-code-tests goals in your builds? Are they present in your configs? E.g. do you have the gRPC extension in your apps? If you don't need them, you could try removing them and see whether it makes a significant difference. If it's troublesome, I'll create a branch with an optimization/cleanup for those goals that you could try.

gsmet commented 2 years ago

These goals are in the default generated project. So we will need them to work OK.

aloubyansky commented 2 years ago

Absolutely. It's just before asking @jimbogithub to build the thing and test it I thought we could try to narrow down the issue.

jimbogithub commented 2 years ago

We do have generate-code/generate-code-tests for the reasons @gsmet points out but we don't use gRPC so if that means we can remove them then I could try that on our main build.

In the meantime I've added two branches to the https://github.com/jimbogithub/sse reproducer from earlier on. The with_sleep branch simply sleeps for 30 seconds (via a test) at the end of the build so that you have time to capture the list of open files. The with_sleep_no_generate branch removes the generate-code/generate-code-tests goals as you suggested and does appear to significantly close the gap in the number of open files. You can tweak the version in the root pom to see the differences.

jimbogithub commented 2 years ago

Removed generate-code/generate-code-tests from our main build. That appears to resolve the issue. Running while true; do lsof | grep java | wc -l && echo "" && sleep 2; done at the same time as the build showed a background count of about 300 (no build running), peaks of around 2000 when running QuarkusTests, troughs of around 700 while compiling. No obvious sign of leakage except that the troughs did bump up to around 980 towards the end of the build, but possibly due to the later modules having some extra/different libraries.

Without those removed the counts go into the many 1,000s.

Can I safely leave those out then? I always thought they were mandatory.

aloubyansky commented 2 years ago

https://github.com/quarkusio/quarkus/pull/24173 should help then. Hopefully it'll make it in 2.7.4.Final scheduled to be released tomorrow. AFAIR, only the gRPC extension relies on the generate-code goals. Kogito extensions also had plans for them. Otherwise, they aren't required.

aloubyansky commented 2 years ago

Thanks a lot for testing @jimbogithub

famod commented 2 years ago

@aloubyansky maybe those mojos should log something as INFO if they are not needed.

aloubyansky commented 2 years ago

I am not sure about that, tbh.