Skip to content
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 keep-alive for HTTPS (static sslSocketFactory) #287

Merged
merged 1 commit into from
May 24, 2023

Conversation

miditeam
Copy link
Contributor

@miditeam miditeam commented Apr 7, 2022

During a migration from HTTP to HTTPS, we noticed that our JsonRpcHttpClient-based application was no longer using keep-alive connections.

After some research we found Bill Healey's answer on a Stack Overflow post, which explains why: for a connection/socket to be reused, quasi everything about it must constant, including sub-objects attached to the connection.

For SSL in particular the SSLSocketFactory object must be the same across calls to the HttpsUrlConnection. This pull request implements this improvement, which yielded a huge performance boost when using SSL / HTTPS. Instead of using the existing setSslContext() context, one must use the new setSslSocketFactory() method.

The PR does not include a documentation update (could not find an adequate place where to insert) but of course we think that it would be worth it to add a warning about this.

@miditeam
Copy link
Contributor Author

Hello there! Did you have time to take a look at this, any issue ?

@miditeam
Copy link
Contributor Author

miditeam commented Sep 1, 2022

Hi @briandilley, any chance to get this simple extension merged? It is good for the performance of the library and it would allow us to rely on the official distribution again. Thanks!

@miditeam
Copy link
Contributor Author

miditeam commented Dec 5, 2022

Hi @briandilley. Are you still maintaining this project? Or should we contact someone else?

@miditeam
Copy link
Contributor Author

Looks like this repo is no longer supported. We raised issue #304 to try and clarify.

@briandilley briandilley merged commit 28e6910 into briandilley:master May 24, 2023
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