xiangfu0 commented on code in PR #19828:
URL: https://github.com/apache/pinot/pull/19828#discussion_r4265158942
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/tls/TlsUtils.java:
##########
@@ -342,8 +342,10 @@ public static SslContext buildClientContext(TlsConfig
tlsConfig) {
SSLFactory sslFactory =
RenewableTlsUtils.createSSLFactoryAndEnableAutoRenewalWhenUsingFileStores(
tlsConfig, PinotInsecureMode::isPinotInInsecureMode);
- SslContextBuilder sslContextBuilder =
-
SslContextBuilder.forClient().sslProvider(SslProvider.valueOf(tlsConfig.getSslProvider()));
+ // Keep Pinot's existing TLS behavior: verify certificate trust without
hostname verification.
+ // Netty 4.2 enables hostname verification by default, which requires a
peer hostname.
+ SslContextBuilder sslContextBuilder = SslContextBuilder.forClient()
+
.sslProvider(SslProvider.valueOf(tlsConfig.getSslProvider())).endpointIdentificationAlgorithm(null);
Review Comment:
Formatted the fluent calls onto separate lines in 46ec9e7. Pinot transport
TLS behavior is unchanged. Root and affected-module Spotless, Checkstyle and
license checks pass.
##########
pom.xml:
##########
@@ -218,7 +218,7 @@
<libthrift.version>0.25.0</libthrift.version>
<log4j.version>2.26.1</log4j.version>
<slf4j.version>2.0.20</slf4j.version>
- <netty.version>4.1.139.Final</netty.version>
+ <netty.version>4.2.18.Final</netty.version>
Review Comment:
Updated the managed Netty version to 4.2.19.Final in 46ec9e7. Fresh Maven
wrapper/JDK 25 transport, TLS and Arrow tests passed: 44 passed, two existing
skips, zero failures/errors. The test classpaths select Netty 4.2.19.Final;
root and affected-module quality checks pass.
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/QueryServer.java:
##########
@@ -137,6 +138,10 @@ public void start() {
protected void initChannel(SocketChannel ch) {
_allChannels.put(ch, true);
ch.closeFuture().addListener(f -> _allChannels.remove(ch));
+ if (_shuttingDown) {
Review Comment:
Registering first closes the shutdown race: a pre-check could pass, then
shutdown could drain the map before this channel is added. With registration
before the flag check, shutdown sees the channel or the initializer closes the
late registration. I added that explanation in 46ec9e7 and kept the order. The
existing QueryRoutingTest and ServerChannelsTest pass with Netty 4.2.19.Final
(21 tests).
--
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]
