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

SSLContext get created unconditionally which can cause serious performance issues #2298

Closed
kissgyorgy opened this issue Jul 8, 2022 Discussed in #2245 · 1 comment
Closed

Comments

@kissgyorgy
Copy link

Discussed in #2245

Originally posted by kissgyorgy May 25, 2022
Our use case is that we built an API Gateway, in which we use httpx as a client to connect to the backend service.

We have a multi-tenant system, so we make a new httpx client every time to isolate customer requests completely.
We have a serious performance issue this way, we measured it, and every request takes 100ms longer just because a ssl.SSLContext is created unconditionally, so we spend a lot of time building SSLContexts, which we don't need anyway, because we don't need an HTTPS connection to the backend, as the two services are on the same machines.

I tried to solve it by passing in a global httpx.AsyncHTTPTransport, but httpx closes the transport after finishing the request, and I was very confused by that so I didn't thought it could be solved this way.
The next thing I tried to implement a global connection pool, so the SSLContext would be created at application startup only once, but because of a serious bug in httpcore about corrupted connections, we couldn't do that either (we would have hit the same bug with the global transport implementation too).

A trivial implementation would be to only create ssl.SSLContext in case of HTTPS connections, which httpcore is already doing correctly, but for some reason, httpx does this unconditionally.

Another implementation idea by @vlaci that there could be an ssl_context_factory, which could be implemented by users.

Here is a py-spy snapshot analyzed with speedscope, which shows we spend half the time creating unused SSLContexts:
image

@tomchristie
Copy link
Member

I think you'd find the same behaviour here with requests or urllib3.

I'd suggest either reusing the same transport as @florimondmanca suggested in #2245. You could also just share a single ssl context by creating a global SSL context instance that you pass in to each client with verify = ssl_context.

For now I'd suggest we close this off, as this isn't an issue when you:

  • Share a single client.
  • Share a single transport across multiple clients.
  • Share a single ssl context across multiple clients.

I do think there's some possible scope for improvement here, both around the API for SSL configuration, and potentially also for reusing rather than recreating SSL contexts, but at this point in time I don't think we'd want to consider either of those as being in scope.

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

No branches or pull requests

2 participants