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

Get S3 credentials lazily #31

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Get S3 credentials lazily #31

wants to merge 2 commits into from

Conversation

pkhuong
Copy link
Collaborator

@pkhuong pkhuong commented Oct 6, 2024

We sometimes create a loader and end up not using it to fetch anything from S3 (e.g., because all the chunks were cached in memory). This happens a lot when the read side wants to decode a manifest, which may refer to a base chunk: that base chunk is usually cached in memory.

Wait until we need to fetch something from S3 to get the credentials and create the bucket objects.

The write VFS (that produces replication data) sometimes needs a
loader, but usually doesn't.  Before this commit, creating a loader
could perform (local) network I/O to get credentials.  We now
wait until we actually have to make an S3 API call (to fetch a
chunk) before getting the credentials.

Sample debug printer output:

```
 cargo test loader -- --nocapture
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.07s
     Running unittests src/lib.rs (target/debug/deps/verneuil-106bf643236b6261)

running 1 test
Resolved Loader: Loader { cache: Cache { write_side: None, auto_sync: true, read_side: ReadOnlyCache { stack: [] } }, remote_sources: [RedactedBucket { name: "test-bucket", region: UsEast1 }], known_chunks: {} }
test loader::tests::test_loader_no_credential ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 46 filtered out; finished in 0.00s
```
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.

1 participant