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

fix(core): exponential retry on cache put fail #29017

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mike-fam
Copy link
Contributor

@mike-fam mike-fam commented Nov 21, 2024

Fix exponential retries so that it behaves according to the comments.

The current comments and implementation of this exponential backoff are not behaving correctly, causing issues when many (~100) parallel tasks access the same local cache on a Network Drive. I've updated it to have the correct functionality while maintaining the maximum allowable time.

Current Behavior

The current exponential backoff retry logic only allows up to 1364 ms (4 + 4² + 4³ + 4⁴ + 4⁵), while the comment allows up to 20480 ms, albeit with confusing maths.

Expected Behavior

Exponential backoff retry is run correctly with the maximum allowable time of 20480 ms

Related Issue(s)

#21926

Fixes #21926

Fix exponential retries so that it behaves according to the comments.

The current comments and implementation of this exponential backoff are not behaving correctly, causing issues when many (~100) parallel tasks access the same local cache. I've updated it to have the correct functionality while still maintaining the max allowable time.
@mike-fam mike-fam requested a review from a team as a code owner November 21, 2024 05:29
@mike-fam mike-fam requested a review from Cammisuli November 21, 2024 05:29
Copy link

vercel bot commented Nov 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jan 13, 2025 5:14am

Copy link

nx-cloud bot commented Nov 21, 2024

View your CI Pipeline Execution ↗ for commit 05a436b.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 36m 25s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 1m 1s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 1s View ↗
nx-cloud record -- nx format:check --base=dc2c9... ✅ Succeeded 24s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 23s View ↗
nx documentation --no-dte ✅ Succeeded 1m 8s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-13 06:04:57 UTC

@mike-fam
Copy link
Contributor Author

mike-fam commented Jan 8, 2025

@AgentEnder @Cammisuli Please take a look and give feedback if need be. Thank you 🙏 🙏

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