Add cache garbage collection #12634

ehuss · 2023-09-07T05:07:06Z

What does this PR try to resolve?

This introduces a new garbage collection system which can track the last time files were used in cargo's global cache, and delete old, unused files either automatically or manually.

How should we test and review this PR?

This is broken up into a large number of commits, and each commit should have a short overview of what it does. I am breaking some of these out into separate PRs as well (unfortunately GitHub doesn't really support stacked pull requests). I expect to reduce the size of this PR if those other PRs are accepted.

I would first review unstable.md to give you an idea of what the user side of this looks like. I would then skim over each commit message to give an overview of all the changes. The core change is the introduction of the GlobalCacheTracker which is an interface to a sqlite database which is used for tracking the timestamps.

Additional information

I think the interface for this will almost certainly change over time. This is just a stab to create a starting point where we can start testing and discussing what actual user flags should be exposed. This is also intended to start the process of getting experience using sqlite, and getting some testing in real-world environments to see how things might fail.

I'd like to ask for the review to not focus too much on bikeshedding flag names and options. I expect them to change, so this is by no means a concrete proposal for where it will end up. For example, the options are very granular, and I would like to have fewer options. However, it isn't clear how that might best work. The size-tracking options almost certainly need to change, but I do not know exactly what the use cases for size-tracking are, so that will need some discussion with people who are interested in that.

I decided to place the gc commands in cargo's cargo clean command because I would like to have a single place for users to go for deleting cache artifacts. It may be possible that they get moved to another command, however introducing new subcommands is quite difficult (due to shadowing existing third-party commands). Other options might be cargo gc, cargo maintenance, cargo cache, etc. But there are existing extensions that would interfere with.

There are also more directions to go in the future. For example, we could add a cargo clean info subcommand which could be used for querying cache information (like the sizes and such). There is also the rest of the steps in the original proposal at https://hackmd.io/U_k79wk7SkCQ8_dJgIXwJg for rolling out sqlite support.

See #12633 for the tracking issue

rustbot · 2023-09-07T05:07:11Z

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

Add with_stdout_unordered. This adds the `with_stdout_unordered` method to cargo's test system so that tests can use it to check stdout but ignoring the order of lines. Nothing in this PR actually uses this method, but it is added to support #12634. I also expect it could potentially be useful in other cases in the future.

Add wrappers around std::fs::metadata This adds wrappers around `std::fs::metadata` and `std::fs::symlink_metadata` which provide better error messages indicating the path that caused the error. This just helps clean up some duplicated code, and is also going to be used to assist with some code changes in #12634.

This was requested to separate the interaction of `cargo clean` and the cleaning of global cache data, and to minimize the impact of this initial implementation.

epage · 2023-11-11T20:41:01Z

src/bin/cargo/commands/clean.rs

+                // FIXME: arg_quiet doesn't work because `config_configure`
+                // doesn't know about subcommands.


Side note: what do we do about cargo report? I assume this would also affect #11879?

So far, cargo report doesn't have a need for the -q flag because its only purpose is to display something on the console, so there isn't any output that needs suppressing. But it is something we should consider moving forward.

I think it will be a problem for #11879. I don't really know the best way to fix this. I don't suppose clap has a way to say "if this arg appears anywhere"? Perhaps another option is to have each subcommand manually handle setting quiet/verbose settings? Do you have any ideas?

clap has global(true) which propagates a flag definition down through the commands and then propagates the parsed value back up and is used by all of the flags in cli.rs's cli().

I opened #12957 to continue this conversation. We currently don't use global(true) because we want to be able to customize the help text per-command.

epage · 2023-11-11T20:43:55Z

Thanks!

@bors r+

bors · 2023-11-11T20:43:57Z

📌 Commit 0cd970b has been approved by epage

It is now in the queue for this repository.

bors · 2023-11-11T20:45:05Z

⌛ Testing commit 0cd970b with merge 9a1b092...

bors · 2023-11-11T21:17:58Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing 9a1b092 to master...

weihanglo · 2023-11-11T22:49:46Z

Thank you @ehuss and @epage for your efforts for this!

This fixes an issue where some last-use tests would fail sporadically because the code that populates missing database entries was using the current time as it was iterating over the files. If the clock happened to roll over while it was iterating, then different files would get different timestamps non-deterministically. The fix is to snapshot the current time when it starts, and reuse that time while repopulating. I didn't do this originally just because I was reluctant to pass yet another argument around. However, it seems like this is necessary. In rust-lang#12634, we discussed other options such as having some kind of process-global "now" snapshot (like in `Config`), but I'm reluctant to do that because I am uneasy dealing with long-lived programs, or handling before/after relationships (like different parts of the code not considering that all timestamps might be equal). It might be something that we could consider in the future, but I'm not sure I want to try right now.

Fix non-deterministic behavior in last-use repopulation This fixes an issue where some last-use tests would fail sporadically because the code that populates missing database entries was using the current time as it was iterating over the files. If the clock happened to roll over while it was iterating, then different files would get different timestamps non-deterministically. The fix is to snapshot the current time when it starts, and reuse that time while repopulating. I didn't do this originally just because I was reluctant to pass yet another argument around. However, it seems like this is necessary. In #12634, we discussed other options such as having some kind of process-global "now" snapshot (like in `Config`), but I'm reluctant to do that because I am uneasy dealing with long-lived programs, or handling before/after relationships (like different parts of the code not considering that all timestamps might be equal). It might be something that we could consider in the future, but I'm not sure I want to try right now.

hi-rustin · 2023-11-13T05:42:17Z

Thanks for working on it! ❤️

Update cargo 15 commits in 6790a5127895debec95c24aefaeb18e059270df3..87ee3e96285e0142b71d8c11c02b18647e43974d 2023-11-10 17:09:35 +0000 to 2023-11-14 18:00:46 +0000 - fix error message for duplicate links (rust-lang/cargo#12973) - Only filter out target if its in the package root (rust-lang/cargo#12944) - Ignore changing_spec_relearns_crate_types on windows-gnu (rust-lang/cargo#12972) - fix: do not panic when failed to parse rustc commit-hash (rust-lang/cargo#12965) - query{_vec} use IndexSummary (rust-lang/cargo#12970) - Bump to 0.77.0; update changelog (rust-lang/cargo#12966) - Improve about information of `cargo search` (rust-lang/cargo#12962) - Fix --quiet being used with nested subcommands. (rust-lang/cargo#12959) - make some debug assertion failures more informative (rust-lang/cargo#12963) - refactor(toml): Consistently lead with 'Toml' prefix (rust-lang/cargo#12960) - refactor(toml): Remove unused method (rust-lang/cargo#12961) - Fix non-deterministic behavior in last-use repopulation (rust-lang/cargo#12958) - Add cache garbage collection (rust-lang/cargo#12634) - refactor(toml): Improve consistency (rust-lang/cargo#12954) - Fix typo (rust-lang/cargo#12956) r? ghost

Update cargo 19 commits in 6790a5127895debec95c24aefaeb18e059270df3..2c03e0e2dcd05dd064fcf10cc1050d342eaf67e3 2023-11-10 17:09:35 +0000 to 2023-11-16 04:21:44 +0000 - docs(ref): Find a place to comment on --cap-lints (rust-lang/cargo#12976) - Switch from AtomicU64 to Mutex. (rust-lang/cargo#12981) - If the only path is a loop then counted as the shortest path. (rust-lang/cargo#12977) - fix(resolver): Prefer MSRV, rather than ignore incompatible (rust-lang/cargo#12950) - fix error message for duplicate links (rust-lang/cargo#12973) - Only filter out target if its in the package root (rust-lang/cargo#12944) - Ignore changing_spec_relearns_crate_types on windows-gnu (rust-lang/cargo#12972) - fix: do not panic when failed to parse rustc commit-hash (rust-lang/cargo#12965) - query{_vec} use IndexSummary (rust-lang/cargo#12970) - Bump to 0.77.0; update changelog (rust-lang/cargo#12966) - Improve about information of `cargo search` (rust-lang/cargo#12962) - Fix --quiet being used with nested subcommands. (rust-lang/cargo#12959) - make some debug assertion failures more informative (rust-lang/cargo#12963) - refactor(toml): Consistently lead with 'Toml' prefix (rust-lang/cargo#12960) - refactor(toml): Remove unused method (rust-lang/cargo#12961) - Fix non-deterministic behavior in last-use repopulation (rust-lang/cargo#12958) - Add cache garbage collection (rust-lang/cargo#12634) - refactor(toml): Improve consistency (rust-lang/cargo#12954) - Fix typo (rust-lang/cargo#12956)

I missed adding these in rust-lang#12634.

Add more doc comments for gc changes. I missed adding these in #12634.

rustbot assigned epage Sep 7, 2023

ehuss force-pushed the last-use branch 2 times, most recently from fd1b671 to 2577bdb Compare September 7, 2023 05:23

rustbot added the A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. label Sep 7, 2023

Move gc options under a cargo clean gc subcommand.

0cd970b

This was requested to separate the interaction of `cargo clean` and the cleaning of global cache data, and to minimize the impact of this initial implementation.

ehuss force-pushed the last-use branch from a11f362 to 0cd970b Compare November 11, 2023 20:36

epage reviewed Nov 11, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 11, 2023

epage mentioned this pull request Nov 11, 2023

Support cargo owner add #11879

Open

bors merged commit 9a1b092 into rust-lang:master Nov 11, 2023
21 of 22 checks passed

ehuss mentioned this pull request Nov 12, 2023

Fix non-deterministic behavior in last-use repopulation #12958

Merged

weihanglo mentioned this pull request Nov 14, 2023

Update cargo rust-lang/rust#117916

Merged

ehuss added a commit to ehuss/cargo that referenced this pull request Nov 28, 2023

Add more doc comments for gc changes.

6950059

I missed adding these in rust-lang#12634.

ehuss mentioned this pull request Nov 28, 2023

Add more doc comments for gc changes. #13055

Merged

bors added a commit that referenced this pull request Nov 28, 2023

Auto merge of #13055 - ehuss:document-more-gc-fields, r=epage

f101fa0

Add more doc comments for gc changes. I missed adding these in #12634.

This was referenced Nov 28, 2023

gc: Verify performance numbers #13058

Open

gc: Determine CLI design for manual cleaning #13060

Open

gc: Determine which config options should be exposed and their defaults #13061

Open

bors added a commit that referenced this pull request Nov 28, 2023

Auto merge of #13055 - ehuss:document-more-gc-fields, r=epage

15a35e5

Add more doc comments for gc changes. I missed adding these in #12634.

ehuss mentioned this pull request Nov 28, 2023

gc: Determine max-size use cases, and consider design changes #13062

Open

ehuss added this to the 1.76.0 milestone Dec 6, 2023

epage mentioned this pull request Dec 8, 2023

Pin cache entries still in use #13137

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cache garbage collection #12634

Add cache garbage collection #12634

ehuss commented Sep 7, 2023 •

edited by epage

Loading

rustbot commented Sep 7, 2023

epage Nov 11, 2023

ehuss Nov 11, 2023

epage Nov 11, 2023

ehuss Nov 11, 2023

epage commented Nov 11, 2023

bors commented Nov 11, 2023

bors commented Nov 11, 2023

bors commented Nov 11, 2023

weihanglo commented Nov 11, 2023

hi-rustin commented Nov 13, 2023

		// FIXME: arg_quiet doesn't work because `config_configure`
		// doesn't know about subcommands.

Add cache garbage collection #12634

Add cache garbage collection #12634

Conversation

ehuss commented Sep 7, 2023 • edited by epage Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Sep 7, 2023

epage Nov 11, 2023

Choose a reason for hiding this comment

ehuss Nov 11, 2023

Choose a reason for hiding this comment

epage Nov 11, 2023

Choose a reason for hiding this comment

ehuss Nov 11, 2023

Choose a reason for hiding this comment

epage commented Nov 11, 2023

bors commented Nov 11, 2023

bors commented Nov 11, 2023

bors commented Nov 11, 2023

weihanglo commented Nov 11, 2023

hi-rustin commented Nov 13, 2023

ehuss commented Sep 7, 2023 •

edited by epage

Loading