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

High WebSocket memory usage in Jetty 10 #6696

Closed
SerCeMan opened this issue Sep 5, 2021 · 4 comments
Closed

High WebSocket memory usage in Jetty 10 #6696

SerCeMan opened this issue Sep 5, 2021 · 4 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@SerCeMan
Copy link
Contributor

SerCeMan commented Sep 5, 2021

Jetty version(s)

10.0.6

Java version/vendor (use: java -version)

$ java -version
openjdk version "11.0.12" 2021-07-20 LTS
OpenJDK Runtime Environment Zulu11.50+19-CA (build 11.0.12+7-LTS)
OpenJDK 64-Bit Server VM Zulu11.50+19-CA (build 11.0.12+7-LTS, mixed mode)

OS type/version

Mac/Ubuntu

Description

Hi, Jetty maintainers!

I got back to trying to reproduce #6328. While I wasn't able to reproduce the referenced issue yet, I seem to have found the main issue that we experienced with the upgrade to Jetty 10, which was related to memory usage.

Compared to Jetty 9, the memory usage per open WebSocket connection has increased significantly, making it problematic for using when servers maintain a large number of connections. I've put together a minimal reproducer project here: jetty10-memory-test, which implements two minimal WebSocket echo servers on Jetty 9, and Jetty 10, both run with Xmx1G and a test that opens 10k connections.

When running the tests against a bare-bones jetty 9 server, the memory usage does not increase much.
Screen Shot 2021-09-05 at 12 13 56 pm

However, when running the tests against Jetty 10, the memory usage grows significantly, and the server dies with an out of memory error after reaching ~9k connections.
Screen Shot 2021-09-05 at 12 15 31 pm

The OOM heapdump seems to match the heapdump from the load tests which discovered the issue, and it hints at org.eclipse.jetty.websocket.core.server.internal.RFC6455Negotiation.
Screen Shot 2021-09-05 at 12 19 25 pm

A heap dump can also be taken by uncommenting a line in start.sh. The heapdump points at the fact that RFC6455Negotiation is referenced from a number of places through upgradeRequest and upgradeResponse.

Screen Shot 2021-09-05 at 12 33 01 pm

When looking at a single instance of RFC6455Negotiation, one can notice that the biggest memory consumer is the baseRequest.
Screen Shot 2021-09-05 at 12 32 31 pm

One potential fix would be to set baseRequest to null after the request upgrade, for instance at the end of org.eclipse.jetty.websocket.core.server.internal.AbstractHandshaker#upgradeRequest. As far as I can tell, it's the only reference. However, I haven't tested the hypothesis. I'm not sure if that's the best solution, but happy to raise a PR if it sounds good to you. Thanks!

How to reproduce?

I've put together a README that describes running the tests here: https://github.com/SerCeMan/jetty10-memory-test.

@SerCeMan SerCeMan added the Bug For general bugs on Jetty side label Sep 5, 2021
@joakime
Copy link
Contributor

joakime commented Sep 5, 2021

I'm a bit confused.
You state the issue is in version 10.0.6, but then reference the work in issue #6328.

Are you using 10.0.6 (public release)? or 10.0.7-SNAPSHOT (from snapshots repo)? or are you building and testing the jetty-10.0.x branch HEAD?

@SerCeMan
Copy link
Contributor Author

SerCeMan commented Sep 6, 2021

Hi, @joakime! As far as I can tell, the issue affects all versions on the 10.x branch, the reproducer is built on the latest stable 10.x release, 10.0.6, build.gradle#L17-L22.

@lachlan-roberts
Copy link
Contributor

Interestingly enough the test at https://github.com/SerCeMan/jetty10-memory-test fails on 10.0.6 but not on the latest 10.0.x branch. It now appears to perform the same as 9.4 in this test. This may be due to the improvements from PR #6635.

Either way we can cleanup the WebSocketNegotiation to not store the original Request object after the upgrade.

lachlan-roberts added a commit that referenced this issue Sep 6, 2021
…fter upgrade

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Sep 7, 2021
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts self-assigned this Sep 7, 2021
lachlan-roberts added a commit that referenced this issue Sep 8, 2021
…adeRequest

Issue #6696 - don't keep Request object in the WebSocketNegotiation after upgrade
@lachlan-roberts
Copy link
Contributor

Closing as this seems to be fixed in 10.0.x and PR #6698 has been merged to remove the reference to the original Request object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

No branches or pull requests

3 participants