Reconsider gc implementation

[pared]

As pointed out in discussion in #1691, we should reconsider gc implementation.
Currently, if called without any options, dvc will collect current branch dependencies and outputs checksums, and remove everything besides it. We can easily clear history of changes with this command. gc should be safer with default options. Straightforward implementation could get all outputs for all revisions in git repo and remove everything that is not on list.

As pointed out by @Suor, this approach might be slow for repository with long history.

[Suor]

This is more about UI defaults than implementation, which will surely be affected too.

[hhoeflin]

yes - very much agree. I am new to dvc, but the gc implementation is already making me nervous.

One additional note on gc. Should "metrics" files be treated differently than the rest? I agree that gc should easily collect old model files etc, but keeping the development of performance over time around would be nice.

@Suor Why do you think this is more about UI than implementation? I am not familiar in detail with the implementation, but the existence of the "--projects" parameter points to an interesting problem. What should happen to files that aren't referenced anywhere in the current git repository?

The conservative answer to this would be to not remove them. However this sounds like a major change in strategy, where only those items are considered for removal that
a) are known where they are referenced
b) are cleared for deletion (e.g. by being in a certain branch or by having never versions for the same file in the a branch)

Or am I misunderstanding this?

Thanks

[efiop]

Hi @hhoeflin ! Sorry for the delay.

Should "metrics" files be treated differently than the rest? I agree that gc should easily collect old model files etc, but keeping the development of performance over time around would be nice.

We usually recommend keeping metrics files in the git (e.g. dvc run -M) for that precise reason plus if you'd lose access to your cache at some point, you would still be able to use git to see what the metric was like.

I am not familiar in detail with the implementation, but the existence of the "--projects" parameter points to an interesting problem. What should happen to files that aren't referenced anywhere in the current git repository?

In the current state of things, those files are going to be removed, that is why gc needs --projects if you are using the same cache or remote for a number of projects. The perk of doing that is that you get inter-project deduplication of cache files, which might be handy 🙂 In other cases, we usually recommend keeping separate caches/remotes for your projects.

a) are known where they are referenced

Currently dvc cache/remote doesn't have any notion of which project it came from, it would require some additional complications to make it aware of that.

b) are cleared for deletion (e.g. by being in a certain branch or by having never versions for the same file in the a branch)

It is clear that we need different strategies for gc. Here are a few related tickets about other strategies: #678 #377 #155 #855
And we clearly need to adopt some reasonable default approach by gathering a number of strategies 🙂

Improving gc is around the top of our TODO list, so we will get to dedicating it more time soon. Thank you for the feedback, we really appreciate it! 🙂

[shcheklein]

An excellent overview and answer @efiop, thank you :)

A few interesting questions have come to mind reading this. For example, should it be analyzing all commits in all branches in the repo by default (vs only all commits in the current branch)? How do we ensure that we have all the branches we need? How do we ensure (and should we?) that when we run it, the branch is correct?

Should it be for example, a global command (in a dvc get sense) - (e.g. dvc remote gc, dvc cache gc - just throwing some ideas) that takes a Git repo link (or multiple links) and analyzes it (them) holistically? By default saving everything - all commits/tags/branches/repos. Then a few options that will be cutting the scope down - branch names, tag names, dates of commits, etc.

[Suor]

One of the sources of issues is we are treating local gc and remote gc the same now, which is wrong for the very usual scenario of using both - in that case local cache is temporary, so we want to say what we want to keep. However for last/permanent storage such as remote cache we usually want to keep everything referenced by default. This is complicated by the existence of other scenarios like shared machine with shared local cache. We should somehow separate this cases. BTW, when we think like that #2037 becomes obvious.

Another thing is, given distributed nature of git, it's impossible to implement dvc gc against shared cache safely - one might just have pushed something only he has ref to and we will collect that unknowingly. So we shouldn't try to be perfect, we should try to be safe though. We may not collect recent files in shared cache for example.

[hhoeflin]

@Suor
Thank you for that explanation. From my point of view, the main issue of gc is a bit of a different one than you describe. It is its basic philosophy of "deleting everything that I don't know I need" rather than "deleting everything that I know I don't need".

[Suor]

Extracted from #2147:

Since git has a distributed nature and we are collecting everything not referenced we may remove something recently pushed and only referenced in git commits not available locally yet - not pulled or not even pushed yet by an author.

The sane method to circumvent this is providing a grace period: do not gc anything newer than N days. In the case someone has just pushed something one shouldn't and wants to remove that it should be still possible to do that:

dvc gc -c --grace-period=0

This is "I know what I am doing even thoufgh I just messed up flag" :)

[efiop]

@Suor grace-period won't work, because a user might use some models that he has generated earlier and by removing those we would break the project for him. Next logical step is LRU, but, once again it is not possible to properly implement it in a distributed nature. I think gc should only be run with the upstream project, to include all important references, and for forks users should use separate storage until their PR is merged.

[Suor]

@efiop LRU won't work either, if that model is in local cache it won't be accessed anyway. But grace period will work for most cases, the example with using some old model, which is still in use, but never referenced by any tag/branch is artificial.

[efiop]

@Suor I'm not saying that is not referenced by any tag/commit/branch, I'm saying that you might've added a dataset 2 years ago and still using it on your master. How would a grace period work in that case?

[Suor]

@efiop if it's referenced then it won't be collected, the grace period is needed to protect something only recently added and not referenced in a particular repo dvc gc is run in, but maybe referenced in other clone.

[Suor]

@hhoeflin "deleting everything that I don't know I need" is the right thing for local repo backed by remote. So the issue is the same approach in both situations.

[dmpetrov]

@shcheklein and @skshetry you are going to the rabbit hole again. GC/not-GC is not the most important part here and it is not very important how it corresponds to programming languages. What is the important part - how to clean space?

If we just make GC safe - users won't be able to clean the space. So, we cannot take this step without introducing more aggressive strategies or at least without keeping the current one (probably as an optional). Also, the specific remove is needed dvc cache remove <datafile-or-dir>.

Actions that I'd suggest (the commands' names TBD) sorted by priorities:

1. In this ticket. Replace the current default scenario to the safe one and keep the current one as an option (probably in the same command).
   1. (optional) Consider renaming dvc gc to dvc cache gc.
2. Create a separate ticket for the specific remove dvc cache remove <datafile-or-dir>.
3. Start introducing -n for others commands (see @shcheklein comment).
4. Think about other gc/delete strategies.

[efiop]

Ok, had another round of discussion with @dmpetrov and @shcheklein . Here are the proposed first steps:

1. Forbid dvc gc(no arguments) and create -w|--workspace flag to work as current behaviour; Remove confirmation prompt that we already have;
2. For --all-tags and --all-commits assume -w automatically to match previous behaviour;
3. Introduce -n(not sure about the best long option name) that will tell how much history commits to keep. So dvc gc -n 10 will keep 10 commits back in your workspace. dvc gc -n 10 -a will keep 10 commits back in all tags and the workspace and so on.

And the rest we can leave for the future discussion. What do you think guys, can we start with these steps?

[dmpetrov]

@efiop thank you for the brief summary. Let's start with this.

PS: you can keep only (1) and (2) in this issue and make (3) as a separate issue if you think the scope is too big. Ups to you...

[skshetry]

Ok, had another round of discussion with @dmpetrov and @shcheklein . Here are the proposed first steps:

1. Forbid dvc gc(no arguments) and create -w|--workspace flag to work as current behaviour; Remove confirmation prompt that we already have;
2. For --all-tags and --all-commits assume -w automatically to match previous behaviour;
3. Introduce -n(not sure about the best long option name) that will tell how much history commits to keep. So dvc gc -n 10 will keep 10 commits back in your workspace. dvc gc -n 10 -a will keep 10 commits back in all tags and the workspace and so on.

And the rest we can leave for the future discussion. What do you think guys, can we start with these steps?

This is what you proposed last week as well, right @efiop? I stalled (eh? 😄) on it a bit as I wanted to discuss more on the usecases (ref: #2325 (comment)) that I felt like were important from reading this issue.

Though, we can start with this than getting stuck. Let's roll with these for now. 🙂

[pared]

Introduce -n(not sure about the best long option name)

How about --tail? though that would not match -n, on the other hand -t could be confused with -T

[kenahoo]

Coming late to this thread.

I expected to have a --dry-run option in dvc gc that would just report what would happen, without actually removing anything. That's similar in spirit to the "require --force to do anything" suggestion, but I think it's a little more straightforward and explicit.

I'd probably suggest requiring either --dry-run or --force to be explicitly specified, just like git clean does.

I also find the wording of the --cloud and --remote options confusing:

• --cloud: "Collect garbage in remote repository"
• --remote: "Remote storage to collect garbage in."

To me, that actually sounds like we will put the garbage into the remote repository, i.e. find garbage locally and upload it to the remote. I think if you just changed "in" to "from", that would help; otherwise I think the following would be more clear:

• --cloud: "Remove garbage from remote repository"
• --remote: "Remote repository to remove garbage from"

[casperdcl]

git clean is a good point. I use it all the time but never without arguments - to the extent that I forgot that it raises an error without args. Though you could argue that makes it a little pointless (surely the default should be dry run).

[skshetry]

@casperdcl @kenahoo, it's not really possible to provide much information in --dry-run because we only delete what we don't know (though we can probably dig into the older commits and try to find out more information).

I also find the wording of the --cloud and --remote options confusing:

Thanks for pointing that out. It seems to be a help message. I'll try making a PR this week for that.

[jorgeorpinel]

that the only way to make the cleanup process both safe and flexible is to make it interactive
Interactivity would be one of the next stages

I just want to also vote for considering interactivity when running dvc gc without flags. Or with an -i option not to interfere with shell scripting.

Related discussion: iterative/dvc.org#1023 (comment)

[kenahoo]

@skshetry I don't think --dry-run has to be hugely informative, it just mainly has to disable the actual deletion.

[Suor]

@kenahoo If you don't mind --dry-run to be almost as slow as a non-dry one then that is not hard to implement. The thing is we don't really know what we will delete until we list all the cache/all the remote cache, which is the slow part.

[dmpetrov]

I just want to also vote for considering interactivity when running dvc gc without flags

Interactivity is a very good idea but not as a default behavior. It is a bit related to prompt\not-prompt problem (#2498) because it complicates scripting.

[efiop]

From #2451

Maybe a report of what was collected (and from where) would be more informative. This way the user could know something actually happened, without having to understand and check DVC internals before and after running the command.

[iesahin]

I don't know if this is still an issue, but:

It's possible to limit dvc gc to work on MD5 references only to those found in Git history. In this case, if I run dvc gc (or something), DVC can check all the references in history, and delete them only. This solves "if a remote is shared between multiple repos/branches/people" problem. If there is no reference in Git history, DVC won't touch it.

Other than this, there can be something like a dvc remote blowup command to remove everything in a remote. This can ask whether the user is sure three times and write a sentence like "I know what I'm doing, everything in the remote will be deleted" before going ahead.

The command names may be modified. dvc gc may be renamed to dvc clean and dvc remote blowup could be dvc remote clean or something, but these two use cases are better to be different commands.

[karajan1001]

It's possible to limit dvc gc to work on MD5 references only to those found in Git history. In this case, if I run dvc gc (or something), DVC can check all the references in history, and delete them only. This solves "if a remote is shared between multiple repos/branches/people" problem. If there is no reference in Git history, DVC won't touch it.

I think so.

Other than this, there can be something like a dvc remote blowup command to remove everything in a remote. This can ask whether the user is sure three times and write a sentence like "I know what I'm doing, everything in the remote will be deleted" before going ahead.

Blowup whole directory can be done on the cloud server side.

[iesahin]

Why is it unrelated?

I can (or someone else) can open a ticket when it can be considered feasible. This looks like an open ended discussion where we share ideas.

[jorgeorpinel]

Remote clearing seems unrelated from reconsidering the implementation of gc.
Or are you suggesting that it becomes part of it, like dvc gc --wipe -r myremote?

[karajan1001]

As the bucket creation was been done in the cloud client outside of DVC, I think it is better to blow up it in the same place.

[iesahin]

Yep, there doesn't need to be a command to "delete all elements" in a remote. This can be left as an exercise to the user.

[jorgeorpinel]

I thought we were talking about wiping the contents without destroying the actual container. But OK fine, this side idea is not the point of the discussion anyway.