xvrl commented on a change in pull request #12032:
URL: https://github.com/apache/druid/pull/12032#discussion_r779173654
##########
File path:
core/src/main/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandler.java
##########
@@ -56,18 +56,15 @@
@Override
public ClientResponse<InputStream> handleResponse(HttpResponse response,
TrafficCop trafficCop)
{
- try (ChannelBufferInputStream channelStream = new
ChannelBufferInputStream(response.getContent())) {
- queue.put(channelStream);
- }
- catch (IOException e) {
- throw new RuntimeException(e);
+ try {
+ // add empty initial buffer since SequenceInputStream will peek the
first element right away
Review comment:
I can make the comment more explicit, the SequenceInputStream
constructor peeking will block on the empty queue, causing the entire method to
block. Previously we were guaranteed at least one chunk in this method, but
that's no longer the case now.
To fix this, here are some options:
1. write our own SequenceInputStream.
2. change `HttpResponseHandler` to have an `InitialType` returned by
handleReponse(), so that we can create the SequenceInputStream in
handleResponse once we get the first chunk.
I would probably opt for the first one, but either one seems a bit complex
and my inclinations is to defer those improvements to avoid further increasing
the scope of this PR
--
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]
