xvrl commented on a change in pull request #12032:
URL: https://github.com/apache/druid/pull/12032#discussion_r779179397



##########
File path: 
core/src/main/java/org/apache/druid/java/util/http/client/NettyHttpClient.java
##########
@@ -227,30 +221,27 @@ public void messageReceived(ChannelHandlerContext ctx, 
MessageEvent e)
                 }
 
                 assert currentChunkNum == 0;
-                possiblySuspendReads(response);
-
-                if (!httpResponse.isChunked()) {
-                  finishRequest();
-                }
-              } else if (msg instanceof HttpChunk) {
-                HttpChunk httpChunk = (HttpChunk) msg;
+                possiblyRead(response);
+              } else if (msg instanceof HttpContent) {
+                HttpContent httpChunk = (HttpContent) msg;
                 if (log.isDebugEnabled()) {
                   log.debug(
                       "[%s] Got chunk: %sB, last=%s",
                       requestDesc,
-                      httpChunk.getContent().readableBytes(),
-                      httpChunk.isLast()
+                      httpChunk.content().readableBytes(),
+                      httpChunk instanceof LastHttpContent
                   );
                 }
 
-                if (httpChunk.isLast()) {
+                response = handler.handleChunk(response, httpChunk, 
++currentChunkNum);
+                if (response.isFinished() && !retVal.isDone()) {
+                  retVal.set((Final) response.getObj());
+                }
+
+                if (httpChunk instanceof LastHttpContent) {

Review comment:
       in the past we would only call handleChunk() on chunks that were not the 
last chunk. Not sure if that was a mistake or if netty 3 happened to always 
finish with an empty chunk. With netty 4 the last chunk can contain data, so we 
do need to call handleChunk of all HttpContent messages, irrespective of 
whether they implemenent LastHttpContent or not.
   isFinished() may also return true before we handle all the chunks (e.g. in 
the case of a streaming response) so we wouldn't want to call finishRequest 
until we handled all the chunks.




-- 
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]

Reply via email to