Skip to content

Respect SSL Provider for GrpcQueryServer in support of RenewableTlsUtils#17760

Open
AlexanderKM wants to merge 1 commit intoapache:masterfrom
AlexanderKM:grpc-respect-ssl-provider
Open

Respect SSL Provider for GrpcQueryServer in support of RenewableTlsUtils#17760
AlexanderKM wants to merge 1 commit intoapache:masterfrom
AlexanderKM:grpc-respect-ssl-provider

Conversation

@AlexanderKM
Copy link
Contributor

Context

We have observed that MSE queries start failing after some time (30 days+ when our internal certs expire)

snippet of example error:

failed: Connection refused
io.grpc.netty.shaded.io.netty.channel.unix.Errors.newConnectException0(Errors.java:166)
io.grpc.netty.shaded.io.netty.channel.unix.Errors.handleConnectErrno(Errors.java:131)
io.grpc.netty.shaded.io.netty.channel.unix.Socket.finishConnect(Socket.java:359)
io.grpc.netty.shaded.io.netty.channel.epoll.AbstractEpollChannel$AbstractEpollUnsafe.doFinishConnect(AbstractEpollChannel.java:715)
finishConnect(..) failed: Connection refused
io.grpc.netty.shaded.io.netty.channel.unix.Errors.newConnectException0(Errors.java:166)
io.grpc.netty.shaded.io.netty.channel.unix.Errors.handleConnectErrno(Errors.java:131)
io.grpc.netty.shaded.io.netty.channel.unix.Socket.finishConnect(Socket.java:359)
io.grpc.netty.shaded.io.netty.channel.epoll.AbstractEpollChannel$AbstractEpollUnsafe.doFinishConnect(AbstractEpollChannel.java:715)

After some digging, it looks like there is a mismatch in the way Pinot is setting the SSL Provider for GRPC connections (i.e. connections for MSE queries).

The Fix

We can take inspiration from the BaseGrpcQueryClient

    if (tlsConfig.getSslProvider() != null) {
      sslContextBuilder =
          GrpcSslContexts.configure(sslContextBuilder, SslProvider.valueOf(tlsConfig.getSslProvider()));
    } else {
      sslContextBuilder = GrpcSslContexts.configure(sslContextBuilder);
    }

Note, that when the sslProvider is not null, we want to explicitly call GrpcSslContexts.configure(sslContextBuilder, SslProvider.valueOf(tlsConfig.getSslProvider()));.

When the call GrpcSslContexts.configure(sslContextBuilder); is made without the explicit sslProvider,
this silently uses the default ssl provider, OpenSSL, with this code:

    @CanIgnoreReturnValue
    public static SslContextBuilder configure(SslContextBuilder builder) {
        return configure(builder, defaultSslProvider());
    }

[source here]

So even when we were building and setting the ssl provider, it is essentially skipped later.

Thus, the fix here is to mimic the grpc client code, and always call GrpcSslContexts.configure(sslContextBuilder, sslProvider).

The big root cause is that the default OpenSSL provider only loads the raw key bytes and cert bytes ONCE into native memory, and does not reload them later. The JDK provider stores references to the KeyManager and TrustManager objects, which can be refreshed in the background to support the reload use case from RenewableTlsUtils

Testing

Confirmed the MSE queries work with our deployments 👍

Tagging @xiangfu0 since I've seen some recent PRs related to this type of code
#17358
#17559

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.18%. Comparing base (6dabd1d) to head (0899e8b).

Files with missing lines Patch % Lines
...che/pinot/core/transport/grpc/GrpcQueryServer.java 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17760      +/-   ##
============================================
- Coverage     63.21%   63.18%   -0.03%     
  Complexity     1454     1454              
============================================
  Files          3179     3179              
  Lines        191279   191282       +3     
  Branches      29251    29252       +1     
============================================
- Hits         120909   120870      -39     
- Misses        60930    60973      +43     
+ Partials       9440     9439       -1     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.16% <0.00%> (-0.03%) ⬇️
java-21 63.16% <0.00%> (-0.01%) ⬇️
temurin 63.18% <0.00%> (-0.03%) ⬇️
unittests 63.18% <0.00%> (-0.03%) ⬇️
unittests1 55.57% <0.00%> (-0.04%) ⬇️
unittests2 34.11% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Alex Maniates <maniatesa@gmail.com>
@AlexanderKM AlexanderKM force-pushed the grpc-respect-ssl-provider branch from 9d72220 to 0899e8b Compare February 25, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants