From 72510dafd60c0261b760af767741241760ad1286 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 11 Dec 2024 21:11:39 -0600 Subject: [PATCH] fix: reapply RST_STREAM(cancel) workaround, and disable grpc-timeout (#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 --- .../jakarta/AsyncServletOutputStreamWriter.java | 10 ++++++++-- .../java/io/grpc/servlet/jakarta/ServletAdapter.java | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java index 9a819c2426e..e05194e132c 100644 --- a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java +++ b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java @@ -94,9 +94,15 @@ final class AsyncServletOutputStreamWriter { transportState.runOnTransportThread( () -> { transportState.complete(); - asyncContext.complete(); - logger.log(FINE, "[{0}] call completed", logId); + // asyncContext.complete(); + logger.fine("call completed"); }); + // Jetty specific fix: When AsyncContext.complete() is called, Jetty sends a RST_STREAM with + // "cancel" error to the client, while other containers send "no error" in this case. Calling + // close() instead on the output stream still sends the RST_STREAM, but with "no error". Note + // that this does the opposite in at least Tomcat, so we're not going to upstream this change. + // See https://github.com/deephaven/deephaven-core/issues/6400 + outputStream.close(); }; } diff --git a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java index 3fb2655b43b..2673d10aea7 100644 --- a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java +++ b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java @@ -144,7 +144,10 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOEx logger.log(FINEST, "[{0}] headers: {1}", new Object[] {logId, headers}); } - Long timeoutNanos = headers.get(TIMEOUT_KEY); + // Always ignore grpc-timeout at this time, as the servlet timeout isn't being reset + // when the output stream is closed. See https://github.com/deephaven/deephaven-core/issues/6400 + // for more information. + Long timeoutNanos = null; // headers.get(TIMEOUT_KEY); if (timeoutNanos == null) { timeoutNanos = 0L; }