rhodo commented on code in PR #15625:
URL: https://github.com/apache/pinot/pull/15625#discussion_r2056736182
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/grpc/GrpcQueryServer.java:
##########
@@ -107,17 +113,36 @@ public GrpcQueryServer(int port, GrpcConfig config,
TlsConfig tlsConfig, QueryEx
);
_queryExecutor = queryExecutor;
_serverMetrics = serverMetrics;
- if (tlsConfig != null) {
- try {
- _server =
NettyServerBuilder.forPort(port).sslContext(buildGrpcSslContext(tlsConfig))
-
.maxInboundMessageSize(config.getMaxInboundMessageSizeBytes()).addService(this)
- .addTransportFilter(new GrpcQueryTransportFilter()).build();
- } catch (Exception e) {
- throw new RuntimeException("Failed to start secure grpcQueryServer",
e);
+
+ try {
+ NettyServerBuilder builder = NettyServerBuilder.forPort(port);
+ if (tlsConfig != null) {
+ builder.sslContext(buildGrpcSslContext(tlsConfig));
}
- } else {
- _server =
ServerBuilder.forPort(port).addService(this).addTransportFilter(new
GrpcQueryTransportFilter()).build();
Review Comment:
I refactor this a bit along the way: ServerBuilder.forPort(port) will load
the first implementation in the classpath which is almost always to be
NettyServerBuilder but it is good to make it deterministic here IMO. Also we
don't really enforce maxInboundMessageSize when using ServerBuilder so fix this
small bug along the way.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
