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

add support for upload speed / remaining in the cache upload step #8081

Merged
merged 1 commit into from May 22, 2024

Conversation

arlyon
Copy link
Contributor

@arlyon arlyon commented May 3, 2024

Description

When turbo is waiting for the cache, don't really expose any data to the user. This PR modifies the cache put operation to take a stream and introduces a new stream adapter that captures a running history of writes using a ring buffer. These requests are stored in a hashmap which is read by the spinner while waiting for shutdown, and a live number is posted with the remaining bytes and current upload speed.

Also gracefully shuts down the upload phase when pressing ctrl-c

Before:

>>  ...Finishing writing to cache...                                                                                                                                                                                                                     WARNING  failed to contact remote cache: skipping HTTP Request, too many failures have occurred.
Last error: error sending request for url (https://vercel.com/api/v8/artifacts/1e1db6d9c17b138d): operation timed out

After:

>   ...Finishing writing to cache... (1.50GB remaining, 8.86MB/s)                                                                                                                                                                                        WARNING  the cache artifact for d6b10341668ff294 was too large to upload within the timeout  

https://asciinema.org/a/s9AsFiVHQRISc22Ve0YY6IlTQ

Testing Instructions

TBD

Closes TURBO-2977

Copy link

vercel bot commented May 3, 2024

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

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:37am
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:37am
8 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:37am

Copy link
Contributor

github-actions bot commented May 3, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

@arlyon arlyon marked this pull request as ready for review May 3, 2024 14:54
@arlyon arlyon requested a review from a team as a code owner May 3, 2024 14:54
@arlyon arlyon requested review from gsoltis and paulogdm and removed request for a team May 3, 2024 14:54
Copy link
Contributor

github-actions bot commented May 3, 2024

🟢 CI successful 🟢

Thanks

@arlyon arlyon force-pushed the arlyon/cache-retry-strategy branch from f0f9ea3 to 6130f00 Compare May 21, 2024 10:23
@arlyon arlyon force-pushed the arlyon/cache-upload-status branch from 23c3965 to cfb9bd9 Compare May 21, 2024 10:23
@arlyon arlyon force-pushed the arlyon/cache-retry-strategy branch from 6130f00 to 2f5f461 Compare May 21, 2024 11:00
@arlyon arlyon force-pushed the arlyon/cache-upload-status branch from cfb9bd9 to ba71868 Compare May 21, 2024 11:00
@arlyon arlyon force-pushed the arlyon/cache-upload-status branch from ba71868 to bc29e74 Compare May 21, 2024 11:18
@arlyon arlyon force-pushed the arlyon/cache-upload-status branch from bc29e74 to 017454a Compare May 21, 2024 11:35
Base automatically changed from arlyon/cache-retry-strategy to main May 21, 2024 11:57
@arlyon arlyon requested review from chris-olszewski and NicholasLYang and removed request for gsoltis and paulogdm May 21, 2024 12:07
@arlyon arlyon force-pushed the arlyon/cache-upload-status branch from 017454a to 61d6e8d Compare May 21, 2024 12:40
Copy link
Contributor

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

LGTM. Only thing that needs changing before merging is unifying our signal handlers.

crates/turborepo-lib/src/run/mod.rs Outdated Show resolved Hide resolved
@arlyon arlyon force-pushed the arlyon/cache-upload-status branch from 61d6e8d to 6213948 Compare May 22, 2024 07:35
@arlyon arlyon merged commit 5f8636b into main May 22, 2024
47 checks passed
@arlyon arlyon deleted the arlyon/cache-upload-status branch May 22, 2024 07:36
Copy link
Contributor Author

arlyon commented May 22, 2024

Merge activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants