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

Large memory increase after bumping from 4.6.2 to 4.7.7 #11428

Open
brian-mulier-p opened this issue Dec 11, 2024 · 4 comments
Open

Large memory increase after bumping from 4.6.2 to 4.7.7 #11428

brian-mulier-p opened this issue Dec 11, 2024 · 4 comments
Assignees

Comments

@brian-mulier-p
Copy link

brian-mulier-p commented Dec 11, 2024

Expected Behavior

No memory increase

Actual Behaviour

Hello!
It seems there is something that is causing large memory increase after updating to 4.7.x (see the graph below where the high spike is when our pod is rolled out with the new Micronaut version.
Screenshot from 2024-12-11 14-21-07

I believe it's due to something in this PR.
We started to get a lot of OOM all of a sudden following the upgrade. From my searches (I'm not familiar with the whole Micronaut codebase so maybe I'm wrong), this change leads to any content from an HTTP Client that exceeds the maxContentLength to be streamed instead with a buffer size of maxContentLength. In our case we've put the value of this prop to Integer.MAX_VALUE (I believe due to some large content we were downloading that was failing the client).

private BodySizeLimits sizeLimits() {
    return new BodySizeLimits(Long.MAX_VALUE, configuration.getMaxContentLength());
}

Our usecase is that we proxy to a docker registry (simple ProxyHttpClient) and it downloads something more than 2GB.

Playing a bit around and considering the change above, our config was originally setting the micronaut.http.services.registry.max-content-length to 2147483647 (Integer.MAX_VALUE) (I believe due to some prior bugs we had. However it seems to me that since we're now streaming and following above, it's taken as the max buffer size.
Lowering this param seems to lower the memory consumption but I had no good result doing a memory dump as Netty uses direct memory...

I know our configuration of max-content-length was a bit awkward but it can be misleading to introduce such behaviour change and the fact that it fallback to such parameter to choose buffer size upon streaming a response seems unclear.

WDYT? Is my analysis correct?

Thank you!

Steps To Reproduce

a proxyHttpClient.proxy(httpRequest) on a content larger than max-content-length should fallback to streaming ence using this value as the max buffer size

Environment Information

No response

Example Application

No response

Version

4.7.7

@graemerocher
Copy link
Contributor

@yawkat can you have a look?

@yawkat
Copy link
Member

yawkat commented Dec 11, 2024

I tried proxying with code like this:

@ServerFilter("/proxy")
class ProxyFilter(val proxy: ProxyHttpClient) {

    @RequestFilter
    fun filter(): Publisher<MutableHttpResponse<*>>? {
        return proxy.proxy(HttpRequest.GET<Unit>("https://ftp.fau.de/ubuntu-releases/noble/ubuntu-24.04.1-desktop-amd64.iso"))
    }
}

And it downloaded just fine with -Xmx512M -XX:MaxDirectMemorySize=512M. So it doesn't look like it's actually buffering 2G. This is not what I would expect either, it the buffering limit is only relevant if there's something that does actual buffering, such as a filter with @Body. In a simple proxy app, it should stay fully streaming.

Beyond something app-specific, I also see two other possibilities related to the env:

  • Maybe the allocation pattern is different, so if you don't have an Xmx or MaxDirectMemorySize, the JVM may grow memory more quickly, showing the memory behavior you see. In general the JVM does not limit memory below the specified limits.
  • There may be a bug relating to backpressure handling. If backpressure is not applied properly, it may be that the file is loaded more quickly from the upstream than it is forwarded to the downstream. That could lead to buffering. You can exclude this by lowering the maxContentLength drastically, that should lead to an exception if the backpressure does not work properly.

You can try debugging these two things. Otherwise I can only request a test case.

@brian-mulier-p
Copy link
Author

Pretty sure it's the second one, btw locally I don't reproduce neither it's on the remote that it's painful.
Actually one more detail is that it tend to fail and lead to OOM whenever we have someone with poor connection trying to download from the proxy so I'm almost sure as said that it's the second root cause. While I can understand the behaviour, it seems to me like a breaking change or something as we didn't have such issue on the previous version so idk. I could dig more but what was the default max buffer size prior to this change? I have hard time digging into both Micronaut and Netty code searching for that value 😞

@yawkat
Copy link
Member

yawkat commented Dec 11, 2024

Ahh I see an issue, auto read is on. I'll try a patch tomorrow. That could indeed break backpressure.

yawkat added a commit that referenced this issue Dec 13, 2024
ProxyBackpressureTest takes quite long so I made it parallel. Wasn't able to do that with junit unfortunately.

Might fix #11428
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

3 participants