GoogleCloudPlatform / appengine-java-vm-runtime

Apache License 2.0
67 stars 34 forks source link

Remove overwrite of response buffer size and just warn on committed response #352

Closed meltsufin closed 7 years ago

meltsufin commented 7 years ago

This PR rolls back the change in #349 which apparently causes very high memory use in the JVM. Instead, this PR enables handling of large responses by just warning about the possibility of log messages being lost.

You might see messages like these in the logs.

com.google.apphosting.vmruntime.jetty9.VmRuntimeWebAppContext doScope: Response for request to '/big.jsp' was already committed (code=200). This might result in lost log messages.' 
com.google.apphosting.vmruntime.VmAppLogsWriter waitForCurrentFlush: End of request or previous flush has not yet completed, blocking.

However, the responses should not be truncated.

meltsufin commented 7 years ago

Interestingly, now the VmRuntimeJettyKitchenSinkTest.testHugeTxt() test is failing. However, previously it was passing. I'm not sure why removing the call to setBuffer() causes the test to fail.

[Edited - the test was passing previously for the right reason.]