Subject: [bug#51427] [PATCH] nix: libstore: Do not remove unused links when deleting specific items.

From: Ludovic Courtès <[email-masked]>
Date: Sun, 31 Oct 2021 15:07:02 +0100

Hi,

Liliana Marie Prikler <[email-masked]> skribis:

> Am Donnerstag, den 28.10.2021, 16:16 +0200 schrieb Ludovic Courtès:
>> Hi,
>> 
>> Maxim Cournoyer <[email-masked]> skribis:
>> 
>> > Deleting unused links can be a very costly operation, especially on
>> > rotative hard drives.  As removing single store items is often used
>> > for experimentation rather than for cleaning purposes, this change
>> > allows it to run without the links cleanup.
>> > 
>> > * nix/libstore/gc.cc (LocalStore::collectGarbage): Do not clean up
>> > links when
>> > the specified action is GCOptions::gcDeleteSpecific.
>> > ---
>> >  nix/libstore/gc.cc | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc
>> > index e1d0765154..7d872d8cc1 100644
>> > --- a/nix/libstore/gc.cc
>> > +++ b/nix/libstore/gc.cc
>> > @@ -771,7 +771,7 @@ void LocalStore::collectGarbage(const GCOptions
>> > & options, GCResults & results)
>> >      deleteGarbage(state, state.trashDir);
>> >  
>> >      /* Clean up the links directory. */
>> > -    if (options.action == GCOptions::gcDeleteDead ||
>> > options.action == GCOptions::gcDeleteSpecific) {
>> > +    if (options.action == GCOptions::gcDeleteDead) {
>> 
>> I believe the effect is that ‘guix gc -D /gnu/store/…-disk-image’
>> would remove nothing: /gnu/store/.links would still contain a copy of
>> that big disk image, so as a result, you’ve freed zero bytes.
>> 
>> Am I right?
> I think that might be the point.  As Maxim said, single items are
> (likely) not removed for cleaning purposes, so freeing the disk image
> has little effect.

What do you mean?  When doing VM testing, I regularly do ‘guix gc -D
/gnu/store/…-disk-image’ precisely to save space.  Fortunately it does
have the intended effect of freeing a bunch of GiBs.

> Plus, you could invoke it like
>
>   guix gc -D dead-item dead-item live-item dead-item
>
> It would fail at live-item and then not continue to free the links of
> the two dead items prior.

Yes, and that’s annoying, but it’s unrelated.  :-)

> So there's a few things we could do here:
>
> 1. simply fail and have the user deal with it (including the option of
> doing a normal `guix gc' or `guix gc -C 1')
> 2. remember which paths were live and dead and always clean up the
> links, only reporting errors afterwards
> 3. add an option to explicitly check the .links directory (which
> defaults to true for the current things, but could also be used to
> clean links after a liveness check or after a do-nothing `guix gc -F').
> 4. ...

You seem to be proposing to remove ‘-D’ altogether.  I agree it has the
shortcomings you write, but I think it’s occasionally useful
nonetheless.

My proposal would be either the status quo, or removing just the one
link that matters from /gnu/store/.links upon ‘-D’.

Thoughts?

Ludo’.