Lower memory consumption by streaming request data

[rtyley]

Motivation and Context

Prior to this change, the UrlConnectionHttpClient implementation of SdkHttpClient would unintentionally cause the entire RequestBody to be loaded into memory before being sent, even if the body had been made using software.amazon.awssdk.core.sync.RequestBody.fromInputStream() and thus was a stream of known content-length. Even worse, as the buffering ByteArrayInputStream received the request, it would have to repeatedly grow capacity by allocating new arrays and copying the data over, so peak memory usage would be 1.5x the size of the request data. For instance, streaming 1 GB of data to S3 with putObject would lead to a peak memory allocation of 1.5 GB. This large allocation of memory can be avoided by instead streaming the request data as it is sent.

java.net.HttpURLConnection (the core Java class used by UrlConnectionHttpClient to implement SdkHttpClient) supports streaming the request, but will only do it if setFixedLengthStreamingMode() or setChunkedStreamingMode() has been called - this is true even if the Content-Length header has already been set. You can see this in the implementation of sun.net.www.protocol.http.HttpURLConnection where streaming() must be true to avoid the whole request being read into a PosterOutputStream (a simple extension of ByteArrayInputStream) before being sent.

Description

If Content-Length is known, we're actually free to call setFixedLengthStreamingMode() on the connection to activate streaming, the only question is where & when to do it! Unfortunately software.amazon.awssdk.http.urlconnection.UrlConnectionFactory, which provides a way for end-users of the AWS SDK to customise the creation & configuration of the HttpURLConnection, doesn't have access to the request or it's Content-Length, so can't call setFixedLengthStreamingMode(). In any case, this behaviour probably always makes sense, so it doesn't make sense to leave it to users to customise it. That leaves the best place to do this being the createAndConfigureConnection() method, where the newly created HttpURLConnection is available and the Content-Length can be read if it's present.

Consequently, I've updated createAndConfigureConnection() to call setFixedLengthStreamingMode if the Content-Length is available and above a streaming threshold of 16 MB. It's possible that buffering an entire request in memory is beneficial in some way for short requests, I don't know, but I believe for large request objects avoiding the large memory allocation is definitely better.

Testing

At present, the only way to workaround this issue without a change to the AWS SDK is to create a custom SdkHttpClient that wraps UrlConnectionHttpClient and overrides prepareRequest(). As the connection field is not visible in the ExecutableHttpRequest object (it's defined on the private software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient.RequestCallable class), it's necessary to use reflection to extract the connection field before operating on it. An example of this in Scala code can be seen here:

guardian/ophan-geoip-db-refresher@e2fc370#diff-5780ade773d1bdf8dd5dea40283e6fec88bacdb30c20c5b39955da1faa8e3ade

Running the ophan-geoip-db-refresher code locally, the heap required is:

• 560 MB (-Xmx560m) - without the workaround, request fully in memory
• 32 MB - with the workaround on, and so streaming enabled

Screenshots

These videos show memory usage before and after streaming being enabled:

Demo.Before.NoStreaming.mov

...at 31s in the above video, you can see memory usage suddenly massively increase - because the entire outgoing request has been buffered into memory. In the video below, with streaming enabled, memory consumption stays low for the entire duration of the run:

Demo.After.StreamingForced.mov

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install - fails on main, fails in same way on this branch
• My code follows the code style of this project
• My change requires a change to the Javadoc documentation
• I have updated the Javadoc documentation accordingly
• I have read the README document
• I have added tests to cover my changes
• All new and existing tests passed
• A short description of the change has been added to the CHANGELOG

License

• I confirm that this pull request can be released under the Apache 2 license

[zoewangg]

Hi @rtyley, apologies for the delayed response and thank you for your PR! We will take a look shortly

[rtyley]

Hi @rtyley, apologies for the delayed response and thank you for your PR! We will take a look shortly

Thanks @zoewangg I appreciate it! I can rebase on the master branch to repair the CHANGELOG.md merge conflict if that helps?

[zoewangg]

Thank you for your contribution!

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[zoewangg]

@all-contributors please add @rtyley for code.

[allcontributors]

@zoewangg

I've put up a pull request to add @rtyley! 🎉