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

"Response body is too large" / "we actually read too much content in coreSizeLimitHandler for request size limit" #85

Closed
stickfigure opened this issue Dec 12, 2023 · 11 comments
Assignees

Comments

@stickfigure
Copy link
Contributor

I'm trying to get a Java8 app migrated to one of the modern Java runtimes and having the worst kind of problem imaginable.

After several hours running in the new runtime, my application starts behaving extremely erratically. I've tried the Java17 and Java21 runtimes, which behave slightly different but still eventually fail all requests.

With the Java17 runtime, I start seeing this:

Request failed: Unexpected Error: org.eclipse.jetty.http.HttpException$RuntimeException: 500: Response body is too large: 335544292061>33554432

With the Java21 runtime, I got some sort of 413 error and saw a lot of this in the logs (verbatum):

we actually read too much content in coreSizeLimitHandler for request size limit

Unfortunately, there's nothing else useful in the logs. I'm using the cloud logging logback adapter and I've correlated the logs, but the logs don't show any actual errors. I see 500 responses in the request log, but with normal (316 bytes) response size and normal application logging output, as if my code is returning success (316 bytes!).

Is this a problem in the runtime? That number in the error message is over 300 GB, which makes no sense to me.

Unfortunately, this only seems to happen under production load - the sandbox deployment of my app doesn't show the same behavior. And it only happens after several hours. I am totally at a loss as to how to go about debugging this, especially without further abusing my poor users :-(

@ludoch
Copy link
Collaborator

ludoch commented Dec 12, 2023

Will look...
Anyway to get a minimal repro app?
Relevant java21 code is around

@stickfigure
Copy link
Contributor Author

Thanks Ludo. So far it's happened twice, in production, once under Java21 and once under Java17. I can try to come up with a load generator for sandbox but making a load profile that looks like real users will be difficult. I'm not even sure where to start with a minimal repro app since I don't have an easy way of making it happen against the full app.

I saw that handler, but I don't have enough context to understand why it would think that it's getting 300GB of data. (assuming that - the 300GB message came from Java17 which doesn't log that "we actually read too much..." and the Java21 runtime was giving me 413 errors, I didn't see better text).

@ludoch
Copy link
Collaborator

ludoch commented Dec 12, 2023

We are preparing a PR. Not sure how long to prod as we are approaching prod freeze for the long break. If you are stuck, let me know and I can provide a way to deploy the runtime jars with your app!

@stickfigure
Copy link
Contributor Author

stickfigure commented Dec 12, 2023

That would be useful to know! We have a fair bit of development on this branch and backporting it to the java8 branch would be a big pain. So yes, please.

Alternatively, if there's something we can do that will avoid tickling this particular issue, that would be a fine workaround for now.

Thanks.

@lachlan-roberts
Copy link
Collaborator

@stickfigure are you using the appengine.use.EE8 option for the Java17 runtime?

I can see a bug that would cause this in the Jetty12 version, but that is only used in Java17 runtime when you have the appengine.use.EE8 property set, otherwise it will be using the Jetty9 version which does not seem to have the same issue.

@ludoch
Copy link
Collaborator

ludoch commented Dec 12, 2023

The trick is https://github.com/GoogleCloudPlatform/appengine-java-standard/blob/main/TRYLATESTBITSINPROD.md
but I need to update this doc as we recently added more jars in the runtime-deployment
zip distribution.
Basically, you'll need to copy all jars inside this zip (that you build from source once the PR is in)
I can help getting it right if needed.

@ludoch
Copy link
Collaborator

ludoch commented Dec 12, 2023

For java17 runtime target, you should really remove the appengine.use.EE8 property for now. Then you'll be back to jetty9 which is working.
appengine.use.EE8 is (not yet) documented for java17, so you are too advanced!

@stickfigure
Copy link
Contributor Author

@lachlan-roberts Checking and yes, that flag was set. We started going Java8 -> Java21, had this issue, switched to Java17, and that flag was left in place. Oops!

I will remove that flag and try again with the production users.

@ludoch
Copy link
Collaborator

ludoch commented Dec 12, 2023

So pom would be like:
...

<plugin>
    <groupId>com.coderplus.maven.plugins</groupId>
    <artifactId>copy-rename-maven-plugin</artifactId>
    <version>1.0</version>
    <executions>
        <execution>
            <id>rename-file</id>
            <phase>pre-integration-test</phase>
            <goals>
                <goal>rename</goal>
            </goals>
            <configuration>
                <fileSets>
                    <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-impl-jetty9-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-impl-jetty9.jar</destinationFile>
                    </fileSet>
                    <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-shared-jetty9-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-shared-jetty9.jar</destinationFile>
                    </fileSet>
                    <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-impl-jetty12-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-impl-jetty12.jar</destinationFile>
                    </fileSet>
                    <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-shared-jetty12-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-shared-jetty12.jar</destinationFile>
                    </fileSet>
                     <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-shared-jetty12-ee10-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-shared-jetty12-ee10.jar</destinationFile>
                    </fileSet>
                    <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-shared-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-shared.jar</destinationFile>
                    </fileSet>
                                      <fileSet>
                        <sourceFile>${appengine.runtime.location}/WEB-INF/lib/runtime-main-${appengine.runtime.version}.jar</sourceFile>
                        <destinationFile>${appengine.runtime.location}/runtime-main.jar</destinationFile>
                    </fileSet>
                </fileSets>
            </configuration>
        </execution>
    </executions>
</plugin>

@stickfigure
Copy link
Contributor Author

Awesome thanks both of you for the quick feedback!

If I still see an error after having removed the EE8 flag (on Java17), I'll try this out. I'm hoping this is an easy fix though. Template strings in Java21 would be really helpful in our codebase, but we can wait (we've made do this long).

@ludoch
Copy link
Collaborator

ludoch commented Jan 24, 2024

Hi,
Only if you have time, you could retry using java21 with EE8, or Java17 with EE8, or even dare trying java21 with EE10, that would be a good validation we are good now. Thanks!

srinjoyray pushed a commit that referenced this issue Nov 12, 2024
srinjoyray pushed a commit that referenced this issue Nov 12, 2024
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