FrankChen021 commented on code in PR #20273:
URL: https://github.com/apache/druid/pull/20273#discussion_r3968498537
##########
processing/src/main/java/org/apache/druid/java/util/http/client/HttpClientInit.java:
##########
@@ -48,7 +48,7 @@
*/
public class HttpClientInit
{
- public static HttpClient createClient(HttpClientConfig config, Lifecycle
lifecycle)
+ public static NettyHttpClient createClient(HttpClientConfig config,
Lifecycle lifecycle)
Review Comment:
[P2] Changing the return type breaks binary callers
This public static method previously returned `HttpClient`, but the new
return type changes the JVM method descriptor to `(...):NettyHttpClient`.
Existing compiled extensions or downstream tests that call
`HttpClientInit.createClient(...):HttpClient` will therefore fail with
`NoSuchMethodError` after upgrading, even though the same source still
compiles. Preserve the old signature and add a differently named
Netty-returning factory (or expose the pool another way) for the registration
code.
##########
docs/configuration/index.md:
##########
@@ -2329,6 +2334,9 @@ Supported query contexts:
|`druid.router.http.numConnections`|Size of connection pool for the Router to
connect to Broker processes. If there are more queries than this number that
all need to speak to the same process, then they will queue up.|`20`|
|`druid.router.http.eagerInitialization`|Indicates that http connections from
Router to Broker should be eagerly initialized. If set to true,
`numConnections` connections are created upon initialization|`true`|
|`druid.router.http.readTimeout`|The timeout for data reads from Broker
processes.|`PT15M`|
+|`druid.router.http.unusedConnectionTimeout`|The timeout for idle connections
in connection pool. The connection in the pool will be closed after this
timeout and a new one will be established. This timeout should be less than
`druid.router.http.readTimeout`. Set this timeout = ~90% of
`druid.router.http.readTimeout`|`PT4M`|
+|`druid.router.http.poolImplementation`|How the connection pool follows
demand. With `adaptive`, a query discards every stale or broken connection it
walks past, so the pool falls back to the size the traffic needs. With
`retaining`, the pool keeps every connection it has opened, replacing a stale
or broken one by a fresh one, one for one.|`adaptive`|
Review Comment:
[P2] Router pool settings are not applied
The Router process binds `druid.router.http` through
`JettyHttpClientModule`, whose provider does not read `poolImplementation` or
`strictConnectionValidation`; only the Netty `HttpClientModule` copies these
fields into `ResourcePoolConfig`. Consequently these documented Router settings
are silently ignored, so Router does not get the advertised adaptive shrinking
or strict validation. Remove these rows or wire the settings into the Router
client before documenting them.
--
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]
