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

Cache Auth errors for 1 second #1933

Closed
wants to merge 1 commit into from

Conversation

alexcb
Copy link
Collaborator

@alexcb alexcb commented Jan 5, 2021

  • previously auth errors were being cached indefinitely.

fixes #1928

Signed-off-by: Alex Couture-Beil alex@earthly.dev

@alexcb alexcb force-pushed the fix-auth-token-caching branch 2 times, most recently from da4fc4b to 2e4eb65 Compare January 5, 2021 23:33
@alexcb alexcb changed the title auth tokens were not properly cached Cache Auth errors for 1 second Jan 5, 2021
@alexcb alexcb marked this pull request as ready for review January 5, 2021 23:36
- previously auth errors were being cached indefinitely.

Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
@tonistiigi
Copy link
Member

Not ideal to have time-based constants where behavior changes on different machines/load. If the real solution is complicated we could let this in as a temp fix.

@alexcb
Copy link
Collaborator Author

alexcb commented Jan 6, 2021

I agree it's a bit hacky, I'm unsure how to limit the cache for a single build without just disabling all caching of errors completely; would it be better to decrease this time-based retry limiter to something less than a second (e.g. Millisecond, microsecond). The smaller the time-based backoff, the closer we get to completely disabling the caching of errors.

@tonistiigi
Copy link
Member

I'm fine with disabling caching for errors at all as a concept but some analysis needs to be made that this doesn't cause any issues for the parallel builds logic, multiple requests per pull, and for the request retries.

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.

Error: i/o timeout should not be cached
2 participants