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

[FEATURE] Expiration buffer for credentials #677

Closed
ozonep opened this issue Dec 20, 2023 · 1 comment
Closed

[FEATURE] Expiration buffer for credentials #677

ozonep opened this issue Dec 20, 2023 · 1 comment
Labels
🧗 enhancement New feature or request

Comments

@ozonep
Copy link
Contributor

ozonep commented Dec 20, 2023

Is your feature request related to a problem?

If job takes long time to complete (more than credentials expiry time), OpenSearch client may have issues renewing credentials upon their expiry, so requests start to fail in case AWS SDK v3 is used.
We found this to be an issue once we migrated to AWS SDK v3. We have jobs that may take 40 hours to complete, and requests almost always start to fail exactly on 1 hour mark (that is our credentials expiry time, max value allowed by AWS). Sometimes this happens after 2 hours, sometimes after 3 hours, but always does.

What solution would you like?

Add expiration buffer value that is at least a value of requestTimeout, so in awsSigner module expiration check would be:

const EXPIRATION_BUFFER_MS = 60_000 // or pass value of `requestTimeout`
if (currentCredentials.expiration?.getTime() - Date.now() < EXPIRATION_BUFFER_MS) {
   expired = true;
}

Setting expiration buffer to 5 minutes fixed our issue (we have requestTimeout of 1 minute, but we used 5 minutes as buffer just to be on the safe side).
But IMO value should be at least the value of requestTimeout so we don't start request with credentials that will expire sooner than request may time out.

I can create PR with this feature.

What alternatives have you considered?

Currently I see 3 ways of setting this buffer value:

  • Have it as a static value (1 minute? 30 seconds?)
  • Allow passing this value to the client as an option
  • Have it always be a value of requestTimeout, or maybe even 2xrequestTimeout
@ozonep ozonep changed the title [FEATURE] [FEATURE] Expiration buffer for credentials Dec 20, 2023
@nhtruong
Copy link
Collaborator

@ozonep Feel free to create a PR and thank you for the detailed explanation! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧗 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants