gianm commented on code in PR #19567:
URL: https://github.com/apache/druid/pull/19567#discussion_r3984252029
##########
server/src/main/java/org/apache/druid/guice/http/HttpClientModule.java:
##########
@@ -159,4 +165,35 @@ public <Intermediate, Final> ListenableFuture<Final> go(
}
}
}
+
+ /**
+ * Single shared adaptive allocator, instantiated eagerly. {@link
AdaptiveByteBufAllocator} has no {@code DEFAULT}
+ * singleton of its own (the inherited {@code ByteBufAllocator.DEFAULT}
constant routes through
+ * {@code -Dio.netty.allocator.type}). Instantiate one here so the
"adaptive" config value really is adaptive.
+ */
+ private static final ByteBufAllocator ADAPTIVE_ALLOCATOR_INSTANCE = new
AdaptiveByteBufAllocator();
Review Comment:
Strange place to put a `private static final`. Generally the static fields
should be at the top of the file.
##########
processing/src/main/java/org/apache/druid/java/util/http/client/HttpClientConfig.java:
##########
@@ -76,6 +78,10 @@ public String getEncodingString()
// aggressive default so a slow or dead peer surfaces sooner. Callers can
override via the builder.
private static final Duration DEFAULT_CONNECT_TIMEOUT_DURATION = new
Period("PT10S").toStandardDuration();
+ // Set an explicit allocator so callers know exactly what they're getting
rather than depending on whatever
+ // ByteBufAllocator.DEFAULT happens to resolve to
+ private static final ByteBufAllocator DEFAULT_BYTE_BUF_ALLOCATOR = new
AdaptiveByteBufAllocator();
Review Comment:
Why do we need both this one and the one in `HttpClientModule`?
##########
server/src/main/java/org/apache/druid/discovery/DataServerResponseHandler.java:
##########
@@ -211,10 +211,15 @@ public ClientResponse<InputStream>
done(ClientResponse<InputStream> clientRespon
@Override
public void exceptionCaught(ClientResponse<InputStream> clientResponse,
Throwable e)
{
+ // Fall back to Throwable.toString() when the exception carries no
message, so a timeout
+ // (Netty's ReadTimeoutException is a stackless, messageless singleton)
does not render as
+ // "exception msg [null]" but as "exception msg
[io.netty.handler.timeout.ReadTimeoutException]"
+ // instead. Behaviour for exceptions that DO have a message is unchanged.
Review Comment:
Behavior (spelling). Although it'd be better to cut this last sentence
entirely. Generally comments should avoid referring to prior states of the same
code block.
--
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]
