eclipse.jdt.ls icon indicating copy to clipboard operation
eclipse.jdt.ls copied to clipboard

Only fetch checksums for final gradle releases during build

Open mfussenegger opened this issue 2 years ago • 6 comments

This is a follow up to https://github.com/eclipse-jdtls/eclipse.jdt.ls/pull/2775 I frequently experience build failures due to connection errors.

After some debugging with wireshark, netstat and -Djdk.httpclient.HttpClient.log=all I suspect that I'm getting rate limited.

Somewhere between 150 and 200 requests, the connections either get closed by the remote or almost stall.

To mitigate the issue, this excludes nightly, snapshot, rc and broken gradle releases. The biggest impact has cutting out RC releases:

total versions: 386 without nightly: 385 without RCs: 173 without broken: 172

Note that this still doesn't completely fix the issue for me, but it helps reduce the chance for it happening.

I also raised an issue in the gradle repo: https://github.com/gradle/gradle/issues/26793 (I doubt they'll address this soon, if at all, but it's worth a shot)

mfussenegger avatar Oct 20 '23 11:10 mfussenegger

I suspected this might be a bit controversial. Part of my reasoning was that people using pre-release versions should kinda know what they're doing and should be fine with accepting the unknown checksum.

Using -Declipse.jdt.ls.skipGradleChecksums works for me, but others could run into the build failures too - making this a hurdle to contributions. It looks like the files are behind a cloudflare CDN, which could make this a region dependant issue.

mfussenegger avatar Oct 20 '23 14:10 mfussenegger

I'm open to this change. It definitely improves the code by handling the redirects. It's just a question of whether we have some guard for enable/disable nightly/snapshot/rcs at build-time or just get rid of them.

rgrunber avatar Oct 20 '23 17:10 rgrunber

Or what about introduce a flag to disable parallel downloading? Since it looks like the parallel downloading works fine in the CI machines.

jdneo avatar Oct 22 '23 11:10 jdneo

Somewhere between 150 and 200 requests, the connections either get closed by the remote or almost stall.

@mfussenegger Is the rate limit per minute, hour or day? What is the use case that requires you to build the jdt.ls from code frequently?

I don't think I like this approach as it reduces what versions of Gradle we would support where supporting them all requires just some build-time processing. Users might still be interest in running the nightlies/snapshots/rcs for the latest upcoming release so it might be nice to include those.

I have similar concern about excluding some gradle version explicitly, this definitely has impact on end-user experience.

testforstephen avatar Oct 23 '23 02:10 testforstephen

Is the rate limit per minute, hour or day? What is the use case that requires you to build the jdt.ls from code frequently?

Probably per minute. I get it sometimes on the first build, after not having built for several days or even weeks. I don't build it that frequently. Maybe once a week or less. Mostly to ensure changes haven't broken anything in nvim-jdtls and if they do, that I can address them before a new eclipse.jdt.ls release.

If the RC releases are really such a concern I don't mind closing this and create a new PR for the .followRedirects(Redirect.NORMAL) change. That's unrelated anyway.

I ended up adding -Declipse.jdt.ls.skipGradleChecksums for me, but I still think that this is a potential stepping stone for contributors. Although given that the ~170 downloads are sometimes still too much, maybe another option would be to default to sequential download, and have a flag to opt-in to the concurrent download?

That would ensure people who check this out for the first time have a reliable build, and regulars should know how to tune it.

mfussenegger avatar Oct 23 '23 15:10 mfussenegger

@mfussenegger , I'm ok with switching to sequential builds (by default) again as long as we have a way to re-enable the parallel workflow (via. system property) in client-side setups like https://github.com/redhat-developer/vscode-java/blob/master/gulpfile.js#L163 . Is that possible with the current code ?

rgrunber avatar Oct 23 '23 22:10 rgrunber