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

feat(turbo-updater): only request latest for now #3017

Closed
wants to merge 1 commit into from

Conversation

tknickman
Copy link
Member

Right now, update_informer does not support caching the correct version per tag. There's only one file right now that is storing the latest known version, so if a canary version makes it way in there (if you ran with a canary first, or if you were running a canary when the interval expired and it refetched latest) then that's going to be the latest for all other versions until the interval expires and we reach out to the registry again.

We need to update this on the update-informer side, but for now I think we should yank updates per tag, stick with latest only, and drop our interval to 6 hours (implemented in this PR)

This will guarantee that the only users who will see this update notification incorrectly are users who have both 1.7.0-canary.x and 1.7.0. AND they run 1.7.0-canary.x first, and then go back to running 1.7.0. Even if this does occur, it would auto fix in 6 hours when the first check to the registry goes out and we return and store the real latest version.

@vercel
Copy link

vercel bot commented Dec 14, 2022

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

Name Status Preview Updated
turbo-site ✅ Ready (Inspect) Visit Preview Dec 14, 2022 at 8:53PM (UTC)
7 Ignored Deployments
Name Status Preview Updated
examples-basic-web ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Dec 14, 2022 at 8:53PM (UTC)

@github-actions
Copy link
Contributor

Benchmark for 1f96f39

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 7791.28µs ± 45.56µs 7803.34µs ± 48.15µs +0.15%
bench_hmr_to_commit/Turbopack RCC/1000 modules 8091.98µs ± 54.41µs 8056.53µs ± 48.47µs -0.44%
bench_hmr_to_commit/Turbopack RSC/1000 modules 853.88ms ± 21.86ms 834.69ms ± 17.32ms -2.25%
bench_hmr_to_commit/Turbopack SSR/1000 modules 7863.65µs ± 61.07µs 7950.88µs ± 50.81µs +1.11%
bench_hmr_to_eval/Turbopack CSR/1000 modules 6776.67µs ± 42.42µs 6863.76µs ± 56.84µs +1.29%
bench_hmr_to_eval/Turbopack RCC/1000 modules 7036.33µs ± 66.27µs 7125.42µs ± 45.85µs +1.27%
bench_hmr_to_eval/Turbopack SSR/1000 modules 7001.93µs ± 51.61µs 6931.05µs ± 47.88µs -1.01%
bench_hydration/Turbopack RCC/1000 modules 3415.54ms ± 16.33ms 3377.74ms ± 22.24ms -1.11%
bench_hydration/Turbopack RSC/1000 modules 2632.15ms ± 55.61ms 2527.48ms ± 27.76ms -3.98%
bench_hydration/Turbopack SSR/1000 modules 2897.54ms ± 11.73ms 2898.56ms ± 5.48ms +0.04%
bench_startup/Turbopack CSR/1000 modules 1407.57ms ± 6.32ms 1397.22ms ± 3.52ms -0.74%
bench_startup/Turbopack RCC/1000 modules 2606.41ms ± 23.50ms 2657.34ms ± 14.04ms +1.95%
bench_startup/Turbopack RSC/1000 modules 2280.91ms ± 32.83ms 2197.19ms ± 31.87ms -3.67%
bench_startup/Turbopack SSR/1000 modules 2277.32ms ± 10.42ms 2278.53ms ± 5.38ms +0.05%

@github-actions
Copy link
Contributor

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Rust benchmark tests

See workflow summary for details

Copy link
Member Author

@tknickman tknickman left a comment

Choose a reason for hiding this comment

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

Decided to go a different, more correct direction here.

@tknickman tknickman closed this Dec 15, 2022
@tknickman tknickman deleted the updater_improvements branch December 15, 2022 03:45
tknickman added a commit that referenced this pull request Dec 15, 2022
We can overload name because update-informer allows name in an
owner/name format and then splits the name but includes the owner in the
cached file. So we can do something like canary/turbo, latest/turbo etc.

We should still send a fix upstream to support this more formally. But
this should work for now.

Updated version of #3017
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.

None yet

1 participant