gianm commented on code in PR #12032:
URL: https://github.com/apache/druid/pull/12032#discussion_r940608109
##########
core/src/main/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandler.java:
##########
@@ -56,18 +56,15 @@ public class SequenceInputStreamResponseHandler implements
HttpResponseHandler<I
@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
+ queue.put(new ByteBufInputStream(Unpooled.EMPTY_BUFFER)); // lgtm
[java/input-resource-leak]
}
catch (InterruptedException e) {
- log.error(e, "Queue appending interrupted");
+ log.warn(e, "Thread interrupted while taking from queue");
Review Comment:
Do we need the stack trace here for debugging? If not:
`log.noStackTrace().warn` will suppress it.
##########
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'm in favor of leaving this as-is and extending the comment to be more
explicit. Something like:
> SequenceInputStream constructor blocks if the queue is empty, but we won't
have content available until the first chunk comes in. So, add an empty first
element.
--
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]
