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.

Sign up for GitHub

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

Jump to bottom

Lower GCP token min_ttl to 4 minutes and add backoff to token refresh logic #6638

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Lower GCP token min_ttl to 4 minutes and add backoff to token refresh logic #6638

merged 1 commit into from
Oct 30, 2024

Conversation

mwylde
Copy link
Contributor

@mwylde mwylde commented Oct 28, 2024

Which issue does this PR close?

Closes #6625.

Rationale for this change

See #6625

What changes are included in this PR?

This PR makes two changes to the token refresh system:

  • For GCP, the min_ttl is reduced to 4 minutes to avoid the race condition with the metadata server caching as described in the issue
  • A backoff has been added to the token refresh logic, such that if the current token isn't yet expired, but we've recently attempted to refresh it (currently hard-coded to within the past 100ms) we won't attempt to refresh and will just return the cached one. This should help avoid the TokenCache from hammering the underlying token issuing system in the case that it's serving a cached token that's within the min_ttl

Are there any user-facing changes?

No

@github-actions github-actions bot added the object-store Object Store Interface label Oct 28, 2024
@mwylde mwylde mentioned this pull request Oct 28, 2024
Merged
tustvold
tustvold approved these changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you. I will leave this open a little longer in case anyone else wants to review

@tustvold tustvold merged commit 933d348 into apache:master Oct 30, 2024
14 checks passed
@mwylde
Copy link
Contributor Author

mwylde commented Oct 30, 2024

Thanks for the speedy review!

adbmal pushed a commit to adbmal/arrow-rs that referenced this pull request Nov 2, 2024
@mwylde @adbmal
Lower GCP token min_ttl to 4 minutes and add backoff to token refresh…
02f425d 
… logic (apache#6638)
mwylde added a commit to ArroyoSystems/arrow-rs that referenced this pull request Nov 7, 2024
@mwylde
Lower GCP token min_ttl to 4 minutes and add backoff to token refresh…
4cfe480 
… logic (apache#6638)
@alamb alamb mentioned this pull request Nov 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tustvold tustvold tustvold approved these changes

Assignees
No one assigned
Labels
object-store Object Store Interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Object_store] min_ttl is too high for GKE tokens
2 participants
@mwylde @tustvold