New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable client side session cache when using native SSL by default #13562
Conversation
Motivation: When we introduced support for client side session caching for our native SSL implementation we disabled it by default as some java versions had a bug that could result in a NPE. This was fixed for all java versions >= 8 for a while. Let's enable it by default so people will receive the performance gains with the need to adjust properties. Related JDK ticket: https://bugs.openjdk.org/browse/JDK-8241248 Modifications: Set io.netty.handler.ssl.openssl.sessionCacheClient to true by default Result: Enable client session caching by default.
/cc @trustin |
Do we have an end-to-end caching unit test? |
@hyperxpro yes tests are already there |
@@ -115,10 +115,8 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen | |||
|
|||
static final boolean SERVER_ENABLE_SESSION_CACHE = | |||
SystemPropertyUtil.getBoolean("io.netty.handler.ssl.openssl.sessionCacheServer", true); | |||
// session caching is disabled by default on the client side due a JDK bug: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update this comment then stating it is now enabled from being disabled previously. Also, let's add a logger for this in the static
block so people can explicitly know that it's turned on now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly I think there is no need for a comment or static logger
…3562) Motivation: When we introduced support for client side session caching for our native SSL implementation we disabled it by default as some java versions had a bug that could result in a NPE. This was fixed for all java versions >= 8 for a while. Let's enable it by default so people will receive the performance gains with the need to adjust properties. Related JDK ticket: https://bugs.openjdk.org/browse/JDK-8241248 Modifications: Set io.netty.handler.ssl.openssl.sessionCacheClient to true by default Result: Enable client session caching by default.
netty/netty#13562 This change enables client side session cache when using native SSL by default The current CI build with Netty SNAPSHOT fails https://github.com/reactor/reactor-netty/actions/runs/6025947526
netty/netty#13562 This change enables client side session cache when using native SSL by default The current CI build with Netty SNAPSHOT fails https://github.com/reactor/reactor-netty/actions/runs/6025947526
Motivation:
When we introduced support for client side session caching for our native SSL implementation we disabled it by default as some java versions had a bug that could result in a NPE. This was fixed for all java versions >= 8 for a while. Let's enable it by default so people will receive the performance gains with the need to adjust properties.
Related JDK ticket:
https://bugs.openjdk.org/browse/JDK-8241248
Modifications:
Set io.netty.handler.ssl.openssl.sessionCacheClient to true by default
Result:
Enable client session caching by default.