Skip to content

Commit 98f6bf3

Browse files
committed
Auto merge of #13492 - ehuss:stabilize-gc-collect, r=weihanglo
Stabilize global cache data tracking. This stabilizes the global cache last-use data tracking. This does not stabilize automatic or manual gc. Tracking issue: #12633 ## Motivation The intent is to start getting cargo to collect data so that when we do stabilize automatic gc, there will be a wider range of cargo versions that will be updating the data so the user is less likely to see cache misses due to an over-aggressive gc. Additionally, this should give us more exposure and time to respond to any problems, such as filesystem issues. ## What is stabilized? Cargo will now automatically create and update an SQLite database, located at `$CARGO_HOME/.global-cache`. This database tracks timestamps of the last time cargo touched an index, `.crate` file, extracted crate `src` directory, git clone, or git checkout. The schema for this database is [here](https://github.com/rust-lang/cargo/blob/a7e93479261432593cb70aea5099ed02dfd08cf5/src/cargo/core/global_cache_tracker.rs#L233-L307). Cargo updates this file on any command that needs to touch any of those on-disk caches. The testsuite for this feature is located in [`global_cache_tracker.rs`](https://github.com/rust-lang/cargo/blob/a7e93479261432593cb70aea5099ed02dfd08cf5/tests/testsuite/global_cache_tracker.rs). ## Stabilization risks There are some risks to stabilizing, since it commits us to staying compatible with the current design. The concerns I can think of with stabilizing: This commits us to using the database schema in the current design. The code is designed to support both backwards and forwards compatible extensions, so I think it should be fairly flexible. Worst case, if we need to make changes that are fundamentally incompatible, then we can switch to a different database filename or tracking approach. There are certain kinds of errors that are ignored if cargo fails to save the tracking data (see [`is_silent_error`](https://github.com/rust-lang/cargo/blob/64ccff290fe20e2aa7c04b9c71460a7fd962ea61/src/cargo/core/global_cache_tracker.rs#L1796-L1813)). The silent errors are only shown with --verbose. This should help deal with read-only filesystem mounts and other issues. Non-silent errors always show just a warning. I don't know if that will be sufficient to avoid problems. I did a fair bit of testing of performance, and there is a bench suite for this code, but we don't know if there will be pathological problems in the real world. It also incurs an overhead that all builds will have to pay for. I've done my best to ensure that this should be reliable when used on network or unusual filesystems, but I think those are still a high-risk category. SQLite should be configured to accommodate these cases, as well as the extensive locking code (which has already been enabled). A call for public testing was announced in December at https://blog.rust-lang.org/2023/12/11/cargo-cache-cleaning.html. At this time, I don't see any issues in https://github.com/rust-lang/cargo/labels/Z-gc that should block this step.
2 parents bf5acf8 + 39863e7 commit 98f6bf3

File tree

2 files changed

+15
-24
lines changed

2 files changed

+15
-24
lines changed

src/cargo/core/global_cache_tracker.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,7 @@ impl GlobalCacheTracker {
354354
// provide user feedback) rather than blocking inside sqlite
355355
// (which by default has a short timeout).
356356
let db_path = gctx.assert_package_cache_locked(CacheLockMode::DownloadExclusive, &db_path);
357-
let mut conn = if gctx.cli_unstable().gc {
358-
Connection::open(db_path)?
359-
} else {
360-
// To simplify things (so there aren't checks everywhere for being
361-
// enabled), just process everything in memory.
362-
Connection::open_in_memory()?
363-
};
357+
let mut conn = Connection::open(db_path)?;
364358
conn.pragma_update(None, "foreign_keys", true)?;
365359
sqlite::migrate(&mut conn, &migrations())?;
366360
Ok(GlobalCacheTracker {

tests/testsuite/global_cache_tracker.rs

+14-17
Original file line numberDiff line numberDiff line change
@@ -164,23 +164,20 @@ fn rustup_cargo() -> Execs {
164164

165165
#[cargo_test]
166166
fn auto_gc_gated() {
167-
// Requires -Zgc to both track last-use data and to run auto-gc.
167+
// Requires -Zgc to run auto-gc.
168168
let p = basic_foo_bar_project();
169169
p.cargo("check")
170170
.env("__CARGO_TEST_LAST_USE_NOW", months_ago_unix(4))
171171
.run();
172-
// Check that it did not create a database or delete anything.
172+
// Check that it created a database.
173173
let gctx = GlobalContextBuilder::new().build();
174-
assert!(!GlobalCacheTracker::db_path(&gctx)
174+
assert!(GlobalCacheTracker::db_path(&gctx)
175175
.into_path_unlocked()
176176
.exists());
177177
assert_eq!(get_index_names().len(), 1);
178178

179179
// Again in the future, shouldn't auto-gc.
180180
p.cargo("check").run();
181-
assert!(!GlobalCacheTracker::db_path(&gctx)
182-
.into_path_unlocked()
183-
.exists());
184181
assert_eq!(get_index_names().len(), 1);
185182
}
186183

@@ -203,7 +200,7 @@ See [..]
203200
fn implies_source() {
204201
// Checks that when a src, crate, or checkout is marked as used, the
205202
// corresponding index or git db also gets marked as used.
206-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
203+
let gctx = GlobalContextBuilder::new().build();
207204
let _lock = gctx
208205
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
209206
.unwrap();
@@ -563,7 +560,7 @@ fn auto_gc_various_commands() {
563560
.masquerade_as_nightly_cargo(&["gc"])
564561
.env("__CARGO_TEST_LAST_USE_NOW", months_ago_unix(4))
565562
.run();
566-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
563+
let gctx = GlobalContextBuilder::new().build();
567564
let lock = gctx
568565
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
569566
.unwrap();
@@ -647,7 +644,7 @@ fn updates_last_use_various_commands() {
647644
.arg("-Zgc")
648645
.masquerade_as_nightly_cargo(&["gc"])
649646
.run();
650-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
647+
let gctx = GlobalContextBuilder::new().build();
651648
let lock = gctx
652649
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
653650
.unwrap();
@@ -696,7 +693,7 @@ fn both_git_and_http_index_cleans() {
696693
.masquerade_as_nightly_cargo(&["gc"])
697694
.env("__CARGO_TEST_LAST_USE_NOW", months_ago_unix(4))
698695
.run();
699-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
696+
let gctx = GlobalContextBuilder::new().build();
700697
let lock = gctx
701698
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
702699
.unwrap();
@@ -821,7 +818,7 @@ fn tracks_sizes() {
821818
.run();
822819

823820
// Check that the crate sizes are the same as on disk.
824-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
821+
let gctx = GlobalContextBuilder::new().build();
825822
let _lock = gctx
826823
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
827824
.unwrap();
@@ -863,7 +860,7 @@ fn tracks_sizes() {
863860
#[cargo_test]
864861
fn max_size() {
865862
// Checks --max-crate-size and --max-src-size with various cleaning thresholds.
866-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
863+
let gctx = GlobalContextBuilder::new().build();
867864

868865
let test_crates = [
869866
// name, age, crate_size, src_size
@@ -962,7 +959,7 @@ fn max_size_untracked_crate() {
962959
// When a .crate file exists from an older version of cargo that did not
963960
// track sizes, `clean --max-crate-size` should populate the db with the
964961
// sizes.
965-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
962+
let gctx = GlobalContextBuilder::new().build();
966963
let cache = paths::home().join(".cargo/registry/cache/example.com-a6c4a5adcb232b9a");
967964
cache.mkdir_p();
968965
paths::home()
@@ -1003,7 +1000,7 @@ fn max_size_untracked_prepare() -> (GlobalContext, Project) {
10031000
let p = basic_foo_bar_project();
10041001
p.cargo("fetch").run();
10051002
// Pretend it was an older version that did not track last-use.
1006-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
1003+
let gctx = GlobalContextBuilder::new().build();
10071004
GlobalCacheTracker::db_path(&gctx)
10081005
.into_path_unlocked()
10091006
.rm_rf();
@@ -1084,7 +1081,7 @@ fn max_download_size() {
10841081
// This creates some sample crates of specific sizes, and then tries
10851082
// deleting at various specific size thresholds that exercise different
10861083
// edge conditions.
1087-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
1084+
let gctx = GlobalContextBuilder::new().build();
10881085

10891086
let test_crates = [
10901087
// name, age, crate_size, src_size
@@ -1339,7 +1336,7 @@ fn clean_syncs_missing_files() {
13391336
.run();
13401337

13411338
// Verify things are tracked.
1342-
let gctx = GlobalContextBuilder::new().unstable_flag("gc").build();
1339+
let gctx = GlobalContextBuilder::new().build();
13431340
let lock = gctx
13441341
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
13451342
.unwrap();
@@ -1992,7 +1989,7 @@ fn forward_compatible() {
19921989
.masquerade_as_nightly_cargo(&["gc"])
19931990
.run();
19941991

1995-
let config = GlobalContextBuilder::new().unstable_flag("gc").build();
1992+
let config = GlobalContextBuilder::new().build();
19961993
let lock = config
19971994
.acquire_package_cache_lock(CacheLockMode::MutateExclusive)
19981995
.unwrap();

0 commit comments

Comments
 (0)