Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

32k truncation issue on master branch #316

Closed
ludoch opened this issue Aug 8, 2016 · 7 comments
Closed

32k truncation issue on master branch #316

ludoch opened this issue Aug 8, 2016 · 7 comments
Milestone

Comments

@ludoch
Copy link
Contributor

ludoch commented Aug 8, 2016

See https://code.google.com/p/googleappengine/issues/detail?id=12790&q=32k&colspec=ID%20Type%20Component%20Status%20Stars%20Summary%20Language%20Priority%20Owner%20Log
Issue is sovled in async branch, but need resolution on master as more customers are using it now, and it's getting escalated.
@gregw : can you take an urgent look? It seems it might be possible to address according the bug above?

@joakime
Copy link

joakime commented Aug 8, 2016

Looking into this.

joakime added a commit to jetty-project/appengine-java-vm-runtime that referenced this issue Aug 8, 2016
@joakime
Copy link

joakime commented Aug 8, 2016

The reproduction examples provided in that code.google.com link to reproduce are not reproducible directly (outside of Spring / Groovy / Google)

Test case added to branch bugs/316 uses the CommitDelayResponse object in an effort to trigger this bug.

It does not trigger with sizes:

size / notes
32768 / 32k on the nose
32668 / 32k - 100
32767 / 32k - 1
32868 / 32k + 100
32769 / 32k + 1

Do we know what the CommitDelayResponse configuration is when the issue occurs?
Does this occur with standard HTTP or HTTPS? or both?

Will endeavor to find the cause.

Incidentally, the CommitDelayResponse implementation (in master) has a spec breaking implementation of sendError() and sendRedirect() (unrelated to this specific bug)

@ludoch
Copy link
Contributor Author

ludoch commented Aug 8, 2016

Thanks for looking into this one... Do you have permission to post directly
on the original bug? If not, I'll see if I can add you

On Mon, Aug 8, 2016 at 12:28 PM, Joakim Erdfelt [email protected]
wrote:

The reproduction examples provided in that code.google.com link to
reproduce are not reproducible directly (outside of Spring / Groovy /
Google)

Test case added to branch bugs/316 uses the CommitDelayResponse object in
an effort to trigger this bug.

It does not trigger with sizes:

size / notes
32768 / 32k on the nose
32668 / 32k - 100
32767 / 32k - 1
32868 / 32k + 100
32769 / 32k + 1

Do we know what the CommitDelayResponse configuration is when the issue
occurs?
Does this occur with standard HTTP or HTTPS? or both?

Will endeavor to find the cause.

Incidentally, the CommitDelayResponse implementation (in master) has a
spec breaking implementation of sendError() and sendRedirect() (unrelated
to this specific bug)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#316 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAE4zUtD12uPtmfJ_lL9GvfIFptgUZoxks5qd4NCgaJpZM4JfOzP
.

@gregw
Copy link
Contributor

gregw commented Aug 9, 2016

Ludo,

Do you want us to look into removing the 32KB limit from master? This
would be doable, but would basically require redoing most of what is in the
async-support branch.

Remind me again what is the main issue why we cannot make async-support the
master branch?

cheers

On 9 August 2016 at 09:37, Ludovic Champenois [email protected]
wrote:

Thanks for looking into this one... Do you have permission to post directly
on the original bug? If not, I'll see if I can add you

On Mon, Aug 8, 2016 at 12:28 PM, Joakim Erdfelt [email protected]
wrote:

The reproduction examples provided in that code.google.com link to
reproduce are not reproducible directly (outside of Spring / Groovy /
Google)

Test case added to branch bugs/316 uses the CommitDelayResponse object in
an effort to trigger this bug.

It does not trigger with sizes:

size / notes
32768 / 32k on the nose
32668 / 32k - 100
32767 / 32k - 1
32868 / 32k + 100
32769 / 32k + 1

Do we know what the CommitDelayResponse configuration is when the issue
occurs?
Does this occur with standard HTTP or HTTPS? or both?

Will endeavor to find the cause.

Incidentally, the CommitDelayResponse implementation (in master) has a
spec breaking implementation of sendError() and sendRedirect() (unrelated
to this specific bug)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/GoogleCloudPlatform/appengine-
java-vm-runtime/issues/316#issuecomment-238348922>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAE4zUtD12uPtmfJ_
lL9GvfIFptgUZoxks5qd4NCgaJpZM4JfOzP>
.


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#316 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEUrabBzKMhAYbkBoO7wdAFwkaRG41mks5qd73VgaJpZM4JfOzP
.

Greg Wilkins [email protected] CTO http://webtide.com

@ludoch
Copy link
Contributor Author

ludoch commented Aug 9, 2016

Main issues on async branch:

1/ request logs: The problem is that the runtime logs are not available in request_log like they normally do. Data is still available but as far as I checked those reside in app logs.
https://code.google.com/p/googleappengine/issues/detail?id=13170

2/ Some new API exceptions https://code.google.com/p/googleappengine/issues/detail?id=13169 (might or might not be Jetty, but need further investigation)

3/ Crashes https://code.google.com/p/googleappengine/issues/detail?id=13168 Again, might not be related to async work, but blocking some customers.

Main pain point is number one. But I guess it is inherent to the async work? Can it be improved?

@gregw
Copy link
Contributor

gregw commented Aug 10, 2016

For 1/ I have created #318 , note also that we have #231 outstanding on the logging as well, which is waiting for a bit of testing. Will look into both shortly.

@gregw gregw added this to the Beta milestone Sep 8, 2016
@gregw gregw removed their assignment Sep 8, 2016
@gregw
Copy link
Contributor

gregw commented Sep 8, 2016

closing as this issue became rather meta and issues are discussed elsewhere.
@ludoch please reopen if there are changes you want for logging in master branch of this repo.

@gregw gregw closed this as completed Sep 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants