-
Notifications
You must be signed in to change notification settings - Fork 80
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
Tracking issue for Jetty sending RST_STREAM(cancel) when AsyncContext.complete() is called #6400
Comments
niloc132
added a commit
that referenced
this issue
Nov 21, 2024
niloc132
added a commit
that referenced
this issue
Nov 22, 2024
niloc132
added a commit
that referenced
this issue
Nov 26, 2024
This is a Jetty-specific workaround to avoid irritating the Python gRPC client into failing calls that had already half-closed successfully. Since we're now using ServletOutputStream.close() in place of AsyncContext.complete(), we need to apply the same wrapping for trailers to close() - that is, when the stream is clsoed, we can't rely on the servlet container sending our trailers because grpc-web trailers are actually a DATA frame which must be explicitly written. See #6400 Fixes #5996 Reapplies #6401
nbauernfeind
pushed a commit
to nbauernfeind/deephaven-core
that referenced
this issue
Nov 29, 2024
…ven#6401) This is a Jetty-specific workaround to avoid irritating the Python gRPC client into failing calls that had already half-closed successfully. See deephaven#6400 Fixes deephaven#5996
niloc132
added a commit
that referenced
this issue
Dec 12, 2024
…6478) Changes from #6420 were supposed to be reverted in #6424, but apparently git interpreted the branch's relationship with main differently than I did. This patch reapplies #6401, with an additional change to avoid noisy stack traces from grpc clients that set `grpc-timeout`. At this time, Deephaven doesn't handle timeouts nor do any default clients set timeouts, so the impact here should be minimal. Fixes #5996 See #6400
niloc132
added a commit
to niloc132/deephaven-core
that referenced
this issue
Dec 12, 2024
…eephaven#6478) Changes from deephaven#6420 were supposed to be reverted in deephaven#6424, but apparently git interpreted the branch's relationship with main differently than I did. This patch reapplies deephaven#6401, with an additional change to avoid noisy stack traces from grpc clients that set `grpc-timeout`. At this time, Deephaven doesn't handle timeouts nor do any default clients set timeouts, so the impact here should be minimal. Fixes deephaven#5996 See deephaven#6400
devinrsmith
pushed a commit
that referenced
this issue
Dec 12, 2024
This is a Jetty-specific workaround to avoid irritating the Python gRPC client into failing calls that had already half-closed successfully. Since we're now using ServletOutputStream.close() in place of AsyncContext.complete(), we need to apply the same wrapping for trailers to close() - that is, when the stream is clsoed, we can't rely on the servlet container sending our trailers because grpc-web trailers are actually a DATA frame which must be explicitly written. See #6400 Fixes #5996 Reapplies #6401 Backport #6424 Backport #6478
nbauernfeind
pushed a commit
to nbauernfeind/deephaven-core
that referenced
this issue
Dec 16, 2024
… gRPC client (deephaven#6420) This reverts commit 6ada0cb. See deephaven#6401 See deephaven#6400 See deephaven#5996
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Tracking issue to let us close #5996 with its workaround, to resolve whatever spec issue exists between Jetty/Tomcat/etc, and to see about adding tests to grpc-java to validate the expected behavior.
Unexpected sideeffect of this is that
grpc-timeout
can't be managed by the servlet container implementation this way - apparently while closing the output stream does end the h2 stream, it does not terminate the timeout.The text was updated successfully, but these errors were encountered: