clintropolis commented on code in PR #19567:
URL: https://github.com/apache/druid/pull/19567#discussion_r3964766108
##########
processing/src/main/java/org/apache/druid/java/util/http/client/NettyHttpClient.java:
##########
@@ -254,37 +254,46 @@ public void abort()
assert currentChunkNum == 0;
possiblySuspendReads(response);
+ // Servers (notably Druid's QueryResource on the historical)
often flush the response
+ // status line + headers in one TCP write and the chunked body
in subsequent writes.
+ // Netty 4's AUTO_READ chains reads automatically after a
complete read cycle, but in
+ // practice the body chunks for these split writes are not
picked up without an explicit
+ // ctx.read(), leaving the caller blocked on an InputStream
that never gets bytes.
+ // Force a read here to drain the body chunks for these
multi-write responses.
+ ctx.read();
Review Comment:
oops, forgot to update here, but to update in this PR's comments I dug into
it a bit, and it turned out that this extra `ctx.read()` wasn't actually
needed, and indeed wack if it should be suspended. Rather claude likely stuck
it in there while fixing one of the other bugs which was causing a hang in the
tests and it didn't hurt anything tests covered and so got counted as an
important part of fixing things. I probably should have checked a bit closer
before trusting things 😅
Removed this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
