Skip to content

Share and close the S3 Express cache in S3Presigner - #7498

Open
dagnir wants to merge 2 commits into
masterfrom
dongie/presigner-s3express-cache
Open

dagnir wants to merge 2 commits into
masterfrom
dongie/presigner-s3express-cache

Conversation

@dagnir

@dagnir dagnir commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

S3Presigner built a new S3 Express auth scheme, with its own empty session cache, on every presign call. Each presign for a directory bucket made a blocking CreateSession call and started a background refresh that nothing ever stopped. A long-running application that presigns S3 Express URLs leaked one refresh loop per presign call and sent growing CreateSession traffic.

Changes

  1. Add UncachedS3ExpressAuthScheme, an internal S3 Express auth scheme whose identity provider calls CreateSession on every identity resolution, with no cache and no background refresh.
  2. S3Presigner uses this scheme for directory buckets, so a presign call leaves nothing running after it returns.
  3. Extract the CreateSession call in S3ExpressIdentityCache into a static createSession(key) so both paths share it.
  4. Add a bugfix changelog entry for Amazon S3.

Compatibility

  1. Each presigned URL still gets a session from its own CreateSession call, the same as before, so URLs keep the full session lifetime (about 5 minutes).
  2. Each presign for a directory bucket still makes one blocking CreateSession call. This matches the current behavior.
  3. S3Client and its session cache are unchanged.

Testing

A new unit test class uses a mocked S3Client whose CreateSession returns a distinct token each call, with credentials that expire about 62 seconds out. A cached session would refresh in the background within a few seconds. Two tests cover the change:

  • Three presigns of one directory bucket call CreateSession three times, and each URL carries its own session token.
  • After one presign, no further CreateSession call happens within 6 seconds.

The existing presigner and S3 Express unit tests pass.

Alternatives Considered

  1. Share one session cache per presigner and close it in S3Presigner.close(). This fixes the leak, but presigns reuse a cached session until it refreshes about 60 seconds before it expires. A URL could then be valid for only about a minute, or about 15 seconds if the background refresh is skipped, even when signatureDuration asks for more.
  2. Close the per-call cache at the end of each presign. This has the same CreateSession cost as this change, but it still builds a cache and schedules a refresh only to cancel it.
  3. Reuse the session cache of the presigner's S3Client. The client does not expose its auth schemes, and the presigner would use a cache it does not own.

## Motivation

`S3Presigner` built a new S3 Express auth scheme, with its own empty session cache, on every presign call. Each presign for a directory bucket therefore made its own blocking `CreateSession` call and started a background refresh that nothing ever stopped, because `S3Presigner.close()` only closed the credentials provider. A long-running application that presigns S3 Express URLs leaked one refresh loop per presign call and sent growing `CreateSession` traffic.

## Changes

1. Each presigner now creates its auth schemes once, so all of its presign calls share one S3 Express session cache.
2. Repeated presigns for the same bucket and credentials reuse one session, with one `CreateSession` call and one refresh loop.
3. `S3Presigner.close()` now closes that cache, which cancels every background refresh it started, and it still closes the credentials provider if closing the cache fails.
4. The cache keeps its limit of 25 entries, and evicted entries are closed.
5. Add a bugfix changelog entry for Amazon S3.

## Compatibility

1. Presigned URLs for one bucket now carry the same session token until the cached session refreshes, where before each URL carried a token from a fresh `CreateSession` call.
2. A presign call made after `close()`, or concurrently with it, can still add a cache entry whose refresh is never closed, because there is no closed-state check.
3. `S3Client` keeps its current behavior on `close()`, with a cache bounded at 25 entries per client.

## Testing

A new unit test class uses a mocked `S3Client` whose `CreateSession` returns credentials that expire about 62 seconds out, so a background refresh fires within a few seconds. Four tests cover the change:

- Four presigns of one directory bucket call `CreateSession` once, and the URL contains a session token.
- Two buckets presigned twice each call `CreateSession` twice.
- Without `close()`, the refresh calls `CreateSession` at least twice within 6 seconds.
- After `close()`, `CreateSession` is still called only once after 6 seconds.

Against the old code, three of the four fail. The existing presigner and S3 Express tests pass.

## Alternatives Considered

1. Close the per-call cache at the end of each presign. Every presign would still make a blocking `CreateSession` call and discard the session.
2. Reuse the session cache of the presigner's `S3Client`. The client does not expose its auth schemes, and the presigner would use a cache it does not own.
3. Use one cache shared by all presigners in the process. Nothing would own it to close it, and its key would mix presigners that differ only by client.
4. Switch presigner caches to a strategy without background refresh. That changes refresh behavior shared with the client and still makes one `CreateSession` call per presign.
@dagnir
dagnir requested a review from a team as a code owner October 29, 2026 22:19
@dagnir
dagnir requested a review from davidh44 October 29, 2026 22:21
- Call CreateSession directly on every presign, with no cache
- Add UncachedS3ExpressAuthScheme for the presigner
- Extract static createSession in S3ExpressIdentityCache
- Revert close support in S3 Express cache and auth scheme
- Rewrite presigner S3 Express test for uncached behavior
- Update changelog entry

This branch has not been deployed

No deployments
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