Re-use connections when remote S3 signing #2543
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The existing S3 remote signing hook function (`s3v4_rest_signer`) uses `requests.post` to submit `POST` requests to the REST signing endpoint. This internally creates a new `requests.Session` for every request, preventing any reuse of connections. In my profiling I saw this add overhead from repeated loading of CA certs and reestablishing of TLS connections. This change makes the signing function a callable object that wraps a `request.Session`, using this for `POST`ing, therefore achieving connection reuse. Signer callables are stored on the hook internals of the `aiobotocore` client inside the `s3fs.S3FileSystem` instance, so use and lifetime will match that of those instances. They are thread-local since: apache#2495.
lwfitzgerald
commented
Sep 29, 2025
Contributor
Author
There was a problem hiding this comment.
Diff best viewed with whitespace changes turned off
jonbannister
approved these changes
Sep 30, 2025
Fokko
approved these changes
Sep 30, 2025
Contributor
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Looks reasonable to me, thanks @lwfitzgerald for working on this 👍
Contributor
|
Thanks @lwfitzgerald for working on this, and thanks @jonbannister for the review 🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The existing S3 remote signing hook function (
s3v4_rest_signer) usesrequests.postto submitPOSTrequests to the REST signing endpoint. This internally creates a newrequests.Sessionfor every request, preventing any reuse of connections.In my profiling I saw this add overhead from repeated loading of CA certs and reestablishing of TLS connections.
This change makes the signing function a callable object that wraps a
request.Session, using this forPOSTing, therefore achieving connection reuse.Signer callables are stored on the hook internals of the
aiobotocoreclient inside thes3fs.S3FileSysteminstance, so use and lifetime will match that of those instances. They are thread-local since: #2495.Are these changes tested?
Tested locally. Existing unit tests updated for changes.
Are there any user-facing changes?
Yes - S3 signing requests now use connection pools (scoped to the
s3fs.S3FileSystemobject).