search

apache / libcloud

Apache Libcloud is a Python library which hides differences between different cloud provider APIs and allows you to manage different cloud resources through a unified and easy to use API.
https://libcloud.apache.org
Apache License 2.0
2.04k stars 925 forks source link

[OpenStack] Reuse connections to same host/port #1886

Closed llamasoft closed 1 year ago

llamasoft commented 1 year ago

Fixes #1885 1885. If the connection host/port/secure hasn't changed then the current connection can be reused.
This greatly reduces overhead when doing bulk actions such as storage object downloads.

In testing with bulk downloading storage objects, this resulted in an overall 2x throughput increase.

Enables OpenStack connection reuse

Description

The underlying common.base.Connection object reuses connections (via LibcloudConnection -> Requests Session -> urllib3 connection pool) but the OpenStack's driver discards this every time it makes a new request. This results in a large amount of time spent opening connections and verifying SSL certificates. The proposed change only creates a new connection if any of the host/port/secure settings have changed.

In theory, for HTTP-based providers there is never a need to explicitly create a new connection because the underlying Requests/urllib3 libraries will create new connections as needed. However, the underlying LibcloudConnection class treats host an immutable, so the session it creates will only ever be used for a single host/port. For this reason, we only need to re-connect() when the host/port changes.

Status

Done, ready for review.

Checklist (tick everything that applies)

Kami commented 1 year ago

Thanks for the contribution - that's a very good catch.

I would imagine that this "regression" is likely related to the time when we switched from home grown library for handling HTTP(s) requests to a wrapper around the requests library.

Kami commented 1 year ago

@llamasoft Can you please sign an ICLA - http://libcloud.readthedocs.org/en/latest/development.html#contributing-bigger-changes? Thanks.

Kami commented 1 year ago

I've added a basic unit test in ebecb9af9593ab639298b3f17373efa3056fb765.

This test is not the most ideal since it only exercises the logic of _set_up_connection_info() method, but it doesn't actually verify it end to end to make sure the underlying HTTP connection is actually re-used. That would require an integration test / similar. It's still better to have this unit test than having no test though.

Change has now been merged into trunk. Thanks