Skip to content

Adds a new config to parallelize loading of ssl_multicert.config#7877

Closed
zwoop wants to merge 2 commits intoapache:masterfrom
zwoop:ParallelCertsLoad
Closed

Adds a new config to parallelize loading of ssl_multicert.config#7877
zwoop wants to merge 2 commits intoapache:masterfrom
zwoop:ParallelCertsLoad

Conversation

@zwoop
Copy link
Contributor

@zwoop zwoop commented May 25, 2021

The new setting is (default: 0):

CONFIG proxy.config.ssl.server.multicert.concurrency INT 24

A setting of -1 will create one loader thread per system CPU. Still testing performance implications here, but this likely is only a noticeable win with many, many thousands of certificates, or, a very slow "file system".

I intentionally left the line parser as-is, to avoid too much breakage and change. I think we should redo this parser, and use standard STL streams etc. to read the file line by line rather than loading the entire file into memory.

@zwoop zwoop added the SSL label May 25, 2021
@zwoop zwoop added this to the 10.0.0 milestone May 25, 2021
@zwoop zwoop self-assigned this May 25, 2021
@zwoop zwoop added the WIP label May 25, 2021
@zwoop zwoop force-pushed the ParallelCertsLoad branch from cd49bab to 170be82 Compare May 25, 2021 16:24
@maskit
Copy link
Member

maskit commented May 26, 2021

This is just FYI, but certs are loaded twice if you enable QUIC because QUICMultiCertConfigLoader is created and it does basically the same thing as SSLMultiCertConfigLoader with a couple of different options for SSL_CTX for QUIC.

@zwoop zwoop force-pushed the ParallelCertsLoad branch from 170be82 to c7df60d Compare May 26, 2021 15:24
The new setting is (default: 0):

    CONFIG proxy.config.ssl.server.multicert.concurrency INT 24

A setting of -1 will create one loader thread per system CPU. The
CPU used during load will be pretty heavy, loading certs is costly.

If enabbled, we will also allow the first load to consume all CPU.
@zwoop zwoop force-pushed the ParallelCertsLoad branch from b37dfbe to 4d75e02 Compare May 26, 2021 18:47
@zwoop
Copy link
Contributor Author

zwoop commented May 26, 2021

if anyone wants to test / play with this (I'm still baffled that loading certs is so CPU expensive!), I have a tar-ball with 20,000 certificates and the accompanying ssl_multicert.config file.

@zwoop zwoop force-pushed the ParallelCertsLoad branch from 4d75e02 to 7bdb854 Compare May 26, 2021 18:58
@bryancall bryancall requested a review from shinrich June 14, 2021 23:09
@bryancall bryancall marked this pull request as ready for review June 14, 2021 23:09
@bryancall bryancall self-requested a review as a code owner June 14, 2021 23:09
@bryancall
Copy link
Contributor

@shinrich is going to review
@zwoop please fix the conflicts

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Sep 24, 2021
@github-actions github-actions bot closed this Oct 1, 2021
@zwoop zwoop removed this from the 10.0.0 milestone Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants