Skip to content

Navigation Menu

Sign in
Sign up

feat(io): refresh vended storage credentials before they expire - #892

Draft
plusplusjiajia wants to merge 3 commits into
apache:main from
plusplusjiajia:feat-vended-credential-refresh
Draft

feat(io): refresh vended storage credentials before they expire #892
plusplusjiajia wants to merge 3 commits into
apache:main from
plusplusjiajia:feat-vended-credential-refresh

Conversation

@plusplusjiajia

@plusplusjiajia plusplusjiajia commented Aug 19, 2026

Copy link
Copy Markdown
Member

What

Vended storage credentials are short-lived, so a long scan or a reused table eventually fails on expired ones. This fills the TODO(gangwu) left in arrow_s3_file_io.cc by #719.

How

ArrowS3FileIO reads s3.session-token-expires-at-ms and, five minutes before the earliest applied credential expires, calls a StorageCredentialRefresher and rebuilds its per-prefix delegates. RestCatalog supplies that callback from the LoadCredentials endpoint when the server advertises it; ResolvingFileIO
passes it down. Aligned with Java's VendedCredentialsProvider: same lead time, same lazy refresh rather than a background thread, same refusal of an empty credential list.

  • A refresh never makes things worse. On failure, or a result with nothing this FileIO can serve, the current credentials stay and the next attempt backs off 30s — installing an empty result would drop working credentials for whatever ambient identity the AWS chain finds.
  • Rebuilding happens outside the credential lock, since building an S3 client can reach out to discover a bucket region. An install counter lets a refresh notice it has been superseded meanwhile.
  • An operation whose credentials already expired waits for an in-flight refresh instead of proceeding to a guaranteed auth error, bounded at 10s.

SupportsStorageCredentials::credentials() now returns by value: a refresh can replace the vector concurrently. All in-tree callers are tests.

plusplusjiajia force-pushed the feat-vended-credential-refresh branch 4 times, most recently from d92ef4d to 7088aed Compare August 21, 2026 15:56

wgtmac commented Aug 23, 2026

Copy link
Copy Markdown
Member

Thanks for adding this feature! I think it is too large to review which may incur a long delay. Perhaps let's split it into smaller ones so we can review it one by one?

plusplusjiajia commented Aug 24, 2026
edited
Loading

Copy link
Copy Markdown
Member Author

Thanks for adding this feature! I think it is too large to review which may incur a long delay. Perhaps let's split it into smaller ones so we can review it one by one?

@wgtmac Thanks — split into a stack, smallest first:

  1. test(io): stop probing the EC2 metadata service in S3 tests #897 — S3 tests stop probing the EC2 metadata service (54s → 0.3s).
  2. refactor(io): hand out S3 delegates by shared_ptr and build them off the lock #898 — ArrowS3FileIO gets refactor(io): make FileIO resolution registry-driven #889 's concurrency treatment. No behavior change.
  3. feat(rest): load table credentials from the LoadCredentials endpoint #899 (draft until refactor(io): hand out S3 delegates by shared_ptr and build them off the lock #898 merges) — LoadCredentials plumbing, not yet invoked.
  4. This PR shrinks to just the refresh policy.

Each builds and passes the full suite standalone; the diff here narrows as each one merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /