Updating from quarkus 2.9.2.Final (Java 17) to 3.17.2 (Java 21) heap memory issues #45122
Comments
|
Thanks a lot for reporting.
This is an equivalent change. The apples to apples comparison would be from
This is a lot of changes. I propose doing changes in isolated steps and seeing what the impact is for each one |
|
/cc @FroMage (rest), @stuartwdouglas (rest) |
geoand
added
kind/question
|
I got a chance to perform some isolated changes and :
Maybe im missing something here, but i'm out of ideas right now for more tests that i could do on my side |
|
The next step is to get a heap dump from the OOM and have a look at it with either VisualVM or Eclipse MAT. Also bisecting the version might be useful as it's a huuuuge gap. I would try 2.16 then 3.0, then 3.8, then 3.15 and refine depending on the results. I can help but setting up things can be a bit time consuming and I wouldn't like to shoot in the dark. So if you could provide an easy way to reproduce it with all pieces available, that would already be awesome. I see you have it ready so if you can send me a link at |
I would add 3.2 here also |
|
Hi there, i looked into the heap dump of the real app where this issue appeared first but i'm not very proficient in looking into heap dumps so i could not pinpoint the issue, would you be able to have a look into it? @gsmet i sent you an email with both a mock server that generates the response and the client that asks for it that i'm using to test these scenarios. I tested with java 17 only migrating from quarkus 2.9.2.Final -> 2.16 -> 3.0 -> 3.2 -> 3.8 and when i got to 3.15 it broke (after i changed dependency from quarkus-resteasy to quarkus-rest) If you think it is necessary i can send you a heap dump, but i think you can generate that easily with the projects @gsmet. Tell me if i can perform any more tests to take out some other options, i'm open to suggestions, and thank you for your help |
|
Were you able to replicate it @gsmet? Is there any news on this topic? |
|
Sorry, I completely missed the ping (and the email...). I'll try to have a look later today once I'm done with my current task! |
|
I'll send you a follow up by email. |
|
I was able to reproduce it and it's even worse as I end up having a gazillion of log lines with: I'll dig more and report back. |
|
The initial stack trace was: and then it gets crazy. |
|
The first thing I noticed when I started working on this is that We keep the context (and the duplicate copy of the bytes) in memory until the next request, that might clear Note that the fact that it's stateful is also problematic from a concurrency point of view for the resolution of the There's more to it though, here is a summary of my findings:
Furthermore, the OOM is handled extremely badly:
I'm not sure we can do something about it but it doesn't seem ideal. To illustrate the problem, here are a few elements. What's in the dump when you get an OOM with the original code (see the two full copies of the bytes):
The memory usage when I get an OOM (with the very dirty patch below) - you can see we are far from using all the heap with we get the OOM:
A very dirty patch that improve the situation a bit (but is definitely not something we want to apply). I include it here so that you can quickly see the affected areas of code: diff --git a/extensions/resteasy-reactive/rest-client-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java b/extensions/resteasy-reactive/rest-client-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java
index fd4169f64e0..4f1b7b07635 100644
--- a/extensions/resteasy-reactive/rest-client-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java
+++ b/extensions/resteasy-reactive/rest-client-jackson/runtime/src/main/java/io/quarkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java
@@ -1,14 +1,11 @@
package io.quarkus.rest.client.reactive.jackson.runtime.serialisers;
-import static io.quarkus.rest.client.reactive.jackson.runtime.serialisers.JacksonUtil.getObjectMapperFromContext;
-
import java.io.IOException;
import java.io.InputStream;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
-import java.util.function.Function;
import jakarta.inject.Inject;
import jakarta.ws.rs.WebApplicationException;
@@ -32,7 +29,6 @@ public class ClientJacksonMessageBodyReader extends JacksonBasicMessageBodyReade
private static final Logger log = Logger.getLogger(ClientJacksonMessageBodyReader.class);
private final ConcurrentMap<ObjectMapper, ObjectReader> objectReaderMap = new ConcurrentHashMap<>();
- private RestClientRequestContext context;
@Inject
public ClientJacksonMessageBodyReader(ObjectMapper mapper) {
@@ -58,20 +54,10 @@ public Object readFrom(Class<Object> type, Type genericType, Annotation[] annota
@Override
public void handle(RestClientRequestContext requestContext) {
- this.context = requestContext;
+ //this.context = requestContext;
}
private ObjectReader getEffectiveReader(MediaType responseMediaType) {
- ObjectMapper effectiveMapper = getObjectMapperFromContext(responseMediaType, context);
- if (effectiveMapper == null) {
- return getEffectiveReader();
- }
-
- return objectReaderMap.computeIfAbsent(effectiveMapper, new Function<>() {
- @Override
- public ObjectReader apply(ObjectMapper objectMapper) {
- return objectMapper.reader();
- }
- });
+ return getEffectiveReader();
}
}
diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java
index ea066ae352e..5ba6fd50d5d 100644
--- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java
+++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientSendRequestHandler.java
@@ -1,6 +1,5 @@
package org.jboss.resteasy.reactive.client.handlers;
-import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -41,6 +40,7 @@
import org.jboss.resteasy.reactive.common.core.Serialisers;
import org.jboss.resteasy.reactive.common.util.MultivaluedTreeMap;
+import io.netty.buffer.ByteBufInputStream;
import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.http.HttpHeaderValues;
import io.netty.handler.codec.http.LastHttpContent;
@@ -361,7 +361,7 @@ public void handle(AsyncResult<Buffer> ar) {
try {
if (buffer.length() > 0) {
requestContext.setResponseEntityStream(
- new ByteArrayInputStream(buffer.getBytes()));
+ new ByteBufInputStream(buffer.getByteBuf()));
} else {
requestContext.setResponseEntityStream(null);
}
diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/VertxClientInputStream.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/VertxClientInputStream.java
index 69fa50754a8..b288778becf 100644
--- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/VertxClientInputStream.java
+++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/VertxClientInputStream.java
@@ -24,17 +24,14 @@ class VertxClientInputStream extends InputStream {
private boolean closed;
private boolean finished;
private ByteBuf pooled;
- private final RestClientRequestContext vertxResteasyReactiveRequestContext;
public VertxClientInputStream(HttpClientResponse response, long timeout,
RestClientRequestContext vertxResteasyReactiveRequestContext) {
- this.vertxResteasyReactiveRequestContext = vertxResteasyReactiveRequestContext;
this.exchange = new VertxBlockingInput(response, timeout);
}
public VertxClientInputStream(HttpClientResponse request, long timeout, ByteBuf existing,
RestClientRequestContext vertxResteasyReactiveRequestContext) {
- this.vertxResteasyReactiveRequestContext = vertxResteasyReactiveRequestContext;
this.exchange = new VertxBlockingInput(request, timeout);
this.pooled = existing;
}
diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java
index bdd37464772..480f3b89086 100644
--- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java
+++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java
@@ -87,10 +87,6 @@ public class RestClientRequestContext extends AbstractResteasyReactiveContext<Re
private final ClientRestHandler[] abortHandlerChainWithoutResponseFilters;
private final boolean disableContextualErrorMessages;
- /**
- * Only initialised once we get the response
- */
- private HttpClientResponse vertxClientResponse;
// Changed by the request filter
Map<String, Object> properties;
private HttpClientRequest httpClientRequest;
@@ -235,7 +231,6 @@ public void initialiseResponse(HttpClientResponse vertxResponse) {
for (String i : vertxHeaders.names()) {
headers.addAll(i, vertxHeaders.getAll(i));
}
- this.vertxClientResponse = vertxResponse;
responseStatus = vertxResponse.statusCode();
responseReasonPhrase = vertxResponse.statusMessage();
responseHeaders = headers;
@@ -310,7 +305,7 @@ public CompletableFuture<ResponseImpl> getResult() {
}
public HttpClientResponse getVertxClientResponse() {
- return vertxClientResponse;
+ return null;
}
@Override |
|
Thanks for the thorough analysis!
This would be addressed by #45518 As for rest of the points, I'll have to take a closer look in order to comment |
This is related to quarkusio#45122 and reduce the amount of copy of the content we keep in memory.
This is related to quarkusio#45122 and reduce the amount of copy of the content we keep in memory. (cherry picked from commit 09edb56)
Describe the bug
After the following changes:
A rest client that was consuming a ~350MB json response stopped working and now throws java.lang.OutOfMemoryError: Java heap space.
Same heap memory configurations were used:
In order for the new quarkus version and dependencies to work i needed to raise the Xmx to 2g.
I was not able to upload all the zip files, but i do have the whole setup for testing done, if needed i can send it.
Expected behavior
Client-works.zip
In version 3.17.2 of quarkus, with the new dependencies and java version, the json body should be able to be consumed with similar heap configurations. (Working client with quarkus version 2.9.2.Final on zip file in attachments)
Actual behavior
After changing the quarkus version to 3.17.2, java to version 21, and changing dependency "quarkus-resteasy" to "quarkus-rest-jackson", it is not possible to consume the same json body response with the same heap memory configurations. (Needed to raise Xmx from 300m to 2g)
How to Reproduce?
Output of
uname -aorverNo response
Output of
java -versionNo response
Quarkus version or git rev
No response
Build tool (ie. output of
mvnw --versionorgradlew --version)No response
Additional information
The text was updated successfully, but these errors were encountered: