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

WebFlux WebClient loading entire file into direct buffer memory during multipart upload #23518

Closed
matthewwilson opened this issue Aug 26, 2019 · 2 comments
Assignees

Comments

@matthewwilson
Copy link

matthewwilson commented Aug 26, 2019

Affects: 5.1.8-RELEASE

We have an endpoint that we can upload files using multipart and are using WebClient for our uploads.

Our upload client code looks like this

@Component
class UploadClient(
    private val client: WebClient,
) {
    suspend fun upload(filePath: String) =
        client.post()
              .uri("/upload")
              .contentType(MULTIPART_FORM_DATA)   
              .body(BodyInserters.fromMultipartData(generateMultipartBody(filePath)))
              .retrieve()
              .bodyToMono(UploadResult::class.java)
              .awaitFirst()

    private fun generateMultipartBody(filePath: String): MultiValueMap<String, HttpEntity<*>> {
        val builder = MultipartBodyBuilder()
        builder.part("file", FileSystemResource(filePath))
        return builder.build()
    }
}

However when we upload a large file, (1.6gb) we are seeing that this entire file is loaded into direct memory:

As the file is uploaded, the memory is released, then when the next file is uploaded you can see the spike in memory again.

For contrast I tried replacing WebClient with https://github.com/AsyncHttpClient/async-http-client and the memory usage is much lower, ~60mb per upload

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 26, 2019
@poutsma
Copy link
Contributor

poutsma commented Aug 27, 2019

I am guessing that the collect(Collectors.toList()) on this line is the culprit, as it buffers the entire part into a list. @rstoyanchev, what do you think?

@poutsma poutsma self-assigned this Aug 27, 2019
@poutsma
Copy link
Contributor

poutsma commented Aug 28, 2019

After more investigation, it seems that the real culprit is the code that reads from the file (i.e. DataBufferUtils::readAsynchronousFileChannel, which currently does not consider back pressure and reads the entire file into buffers, whether there is demand or not.

I am working on a fix.

poutsma added a commit that referenced this issue Aug 29, 2019
This commit adds support for back pressure in the ReadCompletionHandler,
as used by DataBufferUtils::readAsynchronousFileChannel.

See gh-23518
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants