Skip to content

Commit

Permalink
fix: reapply RST_STREAM(cancel) workaround, and disable grpc-timeout (d…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
niloc132 committed Dec 12, 2024
1 parent f3d353a commit 16d4854
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,15 @@ public boolean isFinestEnabled() {
transportState.runOnTransportThread(
() -> {
transportState.complete();
asyncContext.complete();
// asyncContext.complete();
log.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();
};
this.isReady = () -> outputStream.isReady();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,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;
}
Expand Down

0 comments on commit 16d4854

Please sign in to comment.