cache ssl contexts and reuse them #12404
Conversation
| public static final String SSL = "ssl"; | ||
| private static final Map<TlsConfig, SslContext> CLIENT_SSL_CONTEXTS_CACHE = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
what about just use hashcode as the key to avoid the object copy?
There was a problem hiding this comment.
That is a great idea!
| return TlsUtils.buildClientContext(tlsConfig).newHandler(ch.alloc()); | ||
| // Make a copy of the TlsConfig because the TlsConfig is mutable, when the TlsConfig is used as the key of the | ||
| // CLIENT_SSL_CONTEXTS_CACHE, the TlsConfig should not be changed. | ||
| TlsConfig tlsConfigCopy = new TlsConfig(tlsConfig); |
There was a problem hiding this comment.
what's the frequency of this call and object copy?
There was a problem hiding this comment.
The frequency should be low, only at the time when the channel is created. It is the same as how many times the channel should be created.
Anyway, changed the map to use hashCode as the key, so the object copy can be avoided.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #12404 +/- ##
=============================================
- Coverage 61.74% 0.00% -61.75%
=============================================
Files 2436 2360 -76
Lines 133108 129354 -3754
Branches 20617 20059 -558
=============================================
- Hits 82191 0 -82191
- Misses 44876 129354 +84478
+ Partials 6041 0 -6041
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
zhtaoxiang
force-pushed
the
reuse-ssl-context
branch
from
4cf5418
to
4cd757b
Compare
performanceInstead of creating a new SslContext for the same TlsConfig multiple times, this PR reuses the same SslContext if it's already created.
a. reducing the number of objects created.
b. only one thread (https://github.com/apache/pinot/blob/master/pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java#L385) will be created for reloading the same tls store and trust store when tlsconfigs are the same.
Reusing the same SslContext does not change the validation logic and does not introduce security flaws because the same key and ca are used in the old and new implementation.