Skip to content

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

Open
plusplusjiajia wants to merge 1 commit into
apache:mainfrom
plusplusjiajia:feat-vended-credential-refresh
Open

feat(io): refresh vended storage credentials before they expire#892
plusplusjiajia wants to merge 1 commit into
apache:mainfrom
plusplusjiajia:feat-vended-credential-refresh

Conversation

@plusplusjiajia

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
plusplusjiajia force-pushed the feat-vended-credential-refresh branch 3 times, most recently from 408c40f to d92ef4d Compare August 19, 2026 07:37
@plusplusjiajia
plusplusjiajia force-pushed the feat-vended-credential-refresh branch from d92ef4d to 7088aed Compare August 21, 2026 15:56
@wgtmac

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

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 for the suggestion — splitting it definitely makes this easier to review. Smallest first:

  1. test(io): stop probing the EC2 metadata service in S3 tests #897 — the S3 test suites 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 the treatment refactor(io): make FileIO resolution registry-driven #889 gave ResolvingFileIO: shared_ptr delegates, a shared_mutex, delegate construction off the lock. No behavior change.
  3. Next: the LoadCredentials plumbing — response parsing, a refresher callback built from the endpoint, forwarded through ResolvingFileIO but not yet invoked. Opens once refactor(io): hand out S3 delegates by shared_ptr and build them off the lock #898 merges.
  4. This PR then shrinks to just the policy: read s3.session-token-expires-at-ms, refresh five minutes early, back off on failure, discard superseded result

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