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

POST /api/v1/recordings of uploaded file consumes more memory than expected #740

Closed
andrewazores opened this issue Oct 27, 2021 · 0 comments · Fixed by #742
Closed

POST /api/v1/recordings of uploaded file consumes more memory than expected #740

andrewazores opened this issue Oct 27, 2021 · 0 comments · Fixed by #742
Assignees
Labels
bug Something isn't working

Comments

@andrewazores
Copy link
Member

andrewazores commented Oct 27, 2021

Description of issue: cryostatio/cryostat#667 (comment)

Suspected cause: https://github.com/cryostatio/cryostat/blob/b5c547a7163d4b90b0869c53e8349f8160076716/src/main/java/io/cryostat/net/web/http/api/v1/RecordingsPostHandler.java#L226

In the RecordingsPostHandler, we invoke:

                        JfrLoaderToolkit.loadEvents(
                                new File(recordingFile));

in an attempt to validate that the uploaded file we have received is really a JFR binary file.

The implementation of this class for JMC7 is here: https://github.com/JDKMissionControl/jmc/blob/ffcd895e728b168f5b8d313890088c4ae3190d6d/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/JfrLoaderToolkit.java#L113

This ends up creating an InputStream out of the file, then passing that along to a parser and loader. The loader attempts to allocate a bunch of memory for a thread pool to use while parsing and loading chunk data ( https://github.com/JDKMissionControl/jmc/blob/develop/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/internal/FlightRecordingLoader.java#L201 ). This memory allocation may also not work as intended when running inside a container with memory limits different than the host operating system.

This JfrLoaderToolkit call should be removed and replaced with a simpler and lighter file validation. At the least, the parallelized parsing and loading done should be replaced by a simple serial parse, and each parsed chunk can be discarded after it is successfully parsed as a JFR chunk.

Current Vert.x-web version is 3.9.7, which uses Netty 4.1.60.Final.

netty/netty#11188

Vert.x-web should be updated to a version that uses Netty 4.1.64.Final or later:

https://github.com/vert-x3/wiki/wiki/3.9.8-Release-Notes
https://netty.io/news/2021/05/19/4-1-65-Final.html

@andrewazores andrewazores added the bug Something isn't working label Oct 27, 2021
@andrewazores andrewazores self-assigned this Oct 27, 2021
@andrewazores andrewazores changed the title POST /api/v1/recordings validation of uploaded file consumes more memory than expected POST /api/v1/recordings of uploaded file consumes more memory than expected Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant