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

Netty4ClientHttpRequestFactory always allocates "maxRequestSize" bytes for each request [SPR-12623] #17224

Closed
spring-issuemaster opened this issue Jan 13, 2015 · 1 comment

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jan 13, 2015

Francisco Lozano opened SPR-12623 and commented

There are two inter-related issues here:

  1. The maxRequestSize parameter name is incorrect, as it also affects the maximum response size (it's passed to HttpObjectAggregator):
private Bootstrap getBootstrap() {
     if (this.bootstrap == null) {
          Bootstrap bootstrap = new Bootstrap();
          bootstrap.group(this.eventLoopGroup).channel(NioSocketChannel.class)
                        .handler(new ChannelInitializer<SocketChannel>() {
                                 @Override
                                 protected void initChannel(SocketChannel channel) throws Exception {
                                          ChannelPipeline pipeline = channel.pipeline();
                                          if (sslContext != null) {
                                                   pipeline.addLast(sslContext.newHandler(channel.alloc()));
                                          }
                                          pipeline.addLast(new HttpClientCodec());
                                          pipeline.addLast(new HttpObjectAggregator(maxRequestSize));
                                 }
                        });

-Way to solve this would be to add a separate maxResponseSize parameter and use it in HttpObjectAggregator.-

  1. Because of the issue above, when you increase the maxRequestSize parameter (to be able to process maxRequestSize-big responses), even if the request to be processed is just a few kB's, it will always allocate maxRequestSize for it.

This can easily cause OOM errors - but it's not a leak, it's just an abusive use of memory.

java.lang.OutOfMemoryError: Java heap space
        at io.netty.buffer.UnpooledHeapByteBuf.<init>(UnpooledHeapByteBuf.java:45) ~[netty-all-4.0.24.Final.jar:4.0.24.Final]
        at io.netty.buffer.UnpooledByteBufAllocator.newHeapBuffer(UnpooledByteBufAllocator.java:43) ~[netty-all-4.0.24.Final.jar:4.0.24.Final]
        at io.netty.buffer.AbstractByteBufAllocator.heapBuffer(AbstractByteBufAllocator.java:136) ~[netty-all-4.0.24.Final.jar:4.0.24.Final]
        at io.netty.buffer.AbstractByteBufAllocator.heapBuffer(AbstractByteBufAllocator.java:127) ~[netty-all-4.0.24.Final.jar:4.0.24.Final]
        at io.netty.buffer.Unpooled.buffer(Unpooled.java:118) ~[netty-all-4.0.24.Final.jar:4.0.24.Final]
        at org.springframework.http.client.Netty4ClientHttpRequest.<init>(Netty4ClientHttpRequest.java:68) ~[spring-web-4.1.4.RELEASE.jar:4.1.4.RELEAS
E]
        at org.springframework.http.client.Netty4ClientHttpRequestFactory.createRequestInternal(Netty4ClientHttpRequestFactory.java:148) ~[spring-web-
4.1.4.RELEASE.jar:4.1.4.RELEASE]
        at org.springframework.http.client.Netty4ClientHttpRequestFactory.createAsyncRequest(Netty4ClientHttpRequestFactory.java:144) ~[spring-web-4.1
.4.RELEASE.jar:4.1.4.RELEASE]
        at org.springframework.http.client.support.AsyncHttpAccessor.createAsyncRequest(AsyncHttpAccessor.java:76) ~[spring-web-4.1.4.RELEASE.jar:4.1.
4.RELEASE]
        at org.springframework.web.client.AsyncRestTemplate.doExecute(AsyncRestTemplate.java:533) ~[spring-web-4.1.4.RELEASE.jar:4.1.4.RELEASE]
        at org.springframework.web.client.AsyncRestTemplate.execute(AsyncRestTemplate.java:512) ~[spring-web-4.1.4.RELEASE.jar:4.1.4.RELEASE]
        at org.springframework.web.client.AsyncRestTemplate.exchange(AsyncRestTemplate.java:456) ~[spring-web-4.1.4.RELEASE.jar:4.1.4.RELEASE]

The issue is here:

public Netty4ClientHttpRequest(Bootstrap bootstrap, URI uri, HttpMethod method, int maxRequestSize) {
     this.bootstrap = bootstrap;
     this.uri = uri;
     this.method = method;
     this.body = new ByteBufOutputStream(Unpooled.buffer(maxRequestSize));
}

The method used allocates always the max capacity:

/**
 * Creates a new big-endian Java heap buffer with the specified {@code capacity}, which
 * expands its capacity boundlessly on demand.  The new buffer's {@code readerIndex} and
 * {@code writerIndex} are {@code 0}.
 */
public static ByteBuf buffer(int initialCapacity) {
    return ALLOC.heapBuffer(initialCapacity);
}

Instead, this allocation method should be used:

/**
 * Creates a new big-endian Java heap buffer with the specified
 * {@code initialCapacity}, that may grow up to {@code maxCapacity}
 * The new buffer's {@code readerIndex} and {@code writerIndex} are
 * {@code 0}.
 */
public static ByteBuf buffer(int initialCapacity, int maxCapacity) {
    return ALLOC.heapBuffer(initialCapacity, maxCapacity);
}

I set this bug as critical because, with current status, the client is unusable for any meaningful load.

Aside from this, it'd be great if it was possible to provide the memory allocator, instead of using always the static Unpooled one...


Affects: 4.1.4

Reference URL: #719

Issue Links:

  • #17232 Netty4ClientHttpRequestFactory buffers (aggregates) all requests/responses

Referenced from: commits d9d8a79, fd426aa

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 2, 2015

Rossen Stoyanchev commented

The maxRequestSize property is now deprecated as of 4.1.5 and is already removed in master for 4.2. While still present in 4.1.5 I've made sure it's used as expected, i.e. Unpooled.buffer(1024, maxRequestSize).

Meanwhile a new maxResponseSize property has been added that limits the size of the response through HttpObjectAggregator(maxResponseSize).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.