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

[Maxim Cournoyer]

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:
>
>> Am Donnerstag, den 28.10.2021, 16:16 +0200 schrieb Ludovic Courtès:
>>> Hi,
>>> 
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> 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’.

The second proposal makes sense.  I didn't care about freeing space, as
my use case was getting around corrupting an item in my store due to
https://issues.guix.gnu.org/51400, which the patch proposed here allowed
me to do without wasting hours of cleaning up links (nearly 1 GiB of
store on spinning drives).

Thanks,

Maxim