Make 'nix store gc' use the auto-GC policy by edolstra · Pull Request #7851 · NixOS/nix

edolstra · 2023-02-16T15:42:34Z

Motivation

I.e. nix store gc will observe the min-free and max-free settings, which have been renamed to gc-threshold and gc-limit. To remove inconsistency in handling gc-threshold, auto-GC now must be enabled using the new auto-gc setting.

This PR also allows unit prefix in configuration settings, e.g. min-free = 5G.

Fixes #7822, #7608.

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

agreed on idea
agreed on implementation strategy
tests, as appropriate
- functional tests - tests/**.sh
- unit tests - src/*/tests
- integration tests - tests/nixos/*
documentation in the manual
code and comments are self-explanatory
commit message explains why the change was made
new feature or incompatible change: updated release notes

This makes 'nix store gc' observe the same policy as auto-GC (which of course can be overriden from the command line via --max-free and --min-free).

This is no longer determined by 'min-free', which now defaults to infinity. The reason is to make 'min-free' behave consistently for auto-GC and explicit 'nix store gc' invocations.

Ericson2314 · 2023-02-16T19:37:17Z

@edolstra I am a bit worried about the changes to GcStore because RemoteStore also implements GcStore; GcStore represents the interface of being able to trigger GCs, not the implementation. Presumably these new mechanisms would just used on the server side, not the client side.

Ericson2314 · 2023-02-16T19:42:37Z

Also, I would like to take this opportunity to chip away at #5638. The newly renamed options should be FooStoreConfig options, not global settings, on whatever FooStore subclass we end up putting this stuff on.

If we want to be able to set the settings from the Nix config, @roberth and I discussed making a private global settings singleton (which is registered) whose sole purpose is to initialize those FooStoreConfig settings.

src/nix/store-gc.md

Co-authored-by: Valentin Gagarin <[email protected]>

nixos-discourse · 2023-02-17T11:51:43Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-44/25546/1

src/nix/store-gc.md

fricklerhandwerk · 2023-02-20T12:57:06Z

reviewed together in the Nix team meeting
@Ericson2314: the gc policy should be server-side, but as it is the client would be driving the server's policy
- @edolstra: it's debatable if one really wants this. if you run nix store gc --gc-threshold 1G you wouldn't want this to be ignored on the target store
- @Ericson2314: one may still want to override the server settings, but right now every client would tell the server to GC at random intervals, especially for auto-gc where clients will trigger timers
  - clients should not micro-manage the DB
- @edolstra: currently the implementation mixes regular and auto GC, which is inelegant, and we should refactor this eventually
  - clients do not actually call auto-gc, this is done by workers
  - one could factor out running the timer to have a unique thread doing it, but it's a separate issue
@edolstra: there is still a bug: the only thing autoGC needs is waiting for getAvailableSpace(), and calling that on a remote store isn't implemented yet
@Ericson2314: suggestion: move all additions to a subclass of GcStore
- GcStore and doGC() should be stateless and only worry about the policy parameters
- state should be managed somewhere else (back to LocalStore or a new AutoGcStore)
- @edolstra: agreed
@thufschmitt: does this keep an easy and intuitive way of running GC manually? this would be in order to ensure that users don't get stuck running out of space. the change subtly changes the behavior of nix store gc
- @edolstra: the default for gc-limit is infinity anyaway
- @thufschmitt: the problem is that someone may already have gc-max set and enable auto-gc
- @fricklerhandwerk: nix store gc is experimental, we should be free to change it at will, that's the purpose of experimental features
- @Ericson2314: we could add a --full/--force/--always flag to forcefully override the settings to 0-infinity
@roberth: auto-gc is not experimental today, so this does need a migration story.
- have to detect min is set to something and enable auto-gc implicitly
- issue a warning that this is a deprecated behavior with instructions to change the settings, convert to error a couple of releases later
- @edolstra: this is such an obscure feature that didn't even work until a couple of weeks ago
  - @thufschmitt: I know of people using it (???)
- agreement to add a warning
agreement: needs more work, continue async

nixos-discourse · 2023-02-20T15:01:05Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-02-20-nix-team-meeting-minutes-34/25625/1

roberth · 2023-03-12T18:06:53Z

I think gc-goal would be more accurate than gc-limit. A goal is also something that is allowed to be surpassed, whereas going over a limit is not ok.

Also I think a goal reflects something in absolute space, whereas limit could refer to anything including the gc process; a relative amount, which this is not.

nixos-discourse · 2023-03-22T20:35:15Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-03-20-nix-team-meeting-minutes-42/26615/1

nixos-discourse · 2023-05-31T02:08:56Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-05-26-nix-team-meeting-minutes-58/28572/1

doc/manual/src/release-notes/rl-next.md

src/libstore/globals.hh

nixos-discourse · 2023-08-28T17:54:20Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-08-28-nix-team-meeting-minutes-83/32290/1

nixos-discourse · 2023-09-04T19:46:11Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-09-04-nix-team-meeting-minutes-85/32608/1

roberth · 2023-12-22T16:16:05Z

This has a number of unaddressed comments and merge conflicts. Please undraft when comments and conflicts are resolved.

Co-authored-by: Robert Hensing <[email protected]>

Co-authored-by: John Ericson <[email protected]>

fricklerhandwerk · 2024-01-08T15:33:09Z

doc/manual/rl-next/auto-gc-policy.md

+Automatic garbage collection is now only enabled if you set [`auto-gc
+= true`](@docroot@/command-ref/conf-file.md#conf-auto-gc). You will
+probably also want to set
+[`gc-threshold`](@docroot@/command-ref/conf-file.md#conf-gc-threshold)
+to configure when the garbage collector kicks in, e.g. `1G` to make it
+run when free space drops below 1 gigabyte.


Suggested change

Automatic garbage collection is now only enabled if you set [`auto-gc

= true`](@docroot@/command-ref/conf-file.md#conf-auto-gc). You will

probably also want to set

[`gc-threshold`](@docroot@/command-ref/conf-file.md#conf-gc-threshold)

to configure when the garbage collector kicks in, e.g. `1G` to make it

run when free space drops below 1 gigabyte.

Automatic garbage collection is now only enabled if you set [`auto-gc = true`](@docroot@/command-ref/conf-file.md#conf-auto-gc).

You will probably also want to set [`gc-threshold`](@docroot@/command-ref/conf-file.md#conf-gc-threshold) to configure when the garbage collector kicks in, e.g. `1G` to make it run when free space drops below 1 gigabyte.

Ericson2314 · 2024-01-12T14:47:10Z

src/libstore/gc-store.hh

+
+    /**
+     * Do a garbage collection that observes the policy configured by
+     * `gc-threshold`, `gc-limit`, etc.
+     */
+    void doGC(bool sync = true);
+
+    /**
+     * Perform an automatic garbage collection, if enabled.
+     */
+    void autoGC(bool sync = true);
+
+    /**
+     * Return the amount of available disk space in this store. Used
+     * by `autoGC()`.
+     */
+    virtual uint64_t getAvailableSpace()
+    {
+        return std::numeric_limits<uint64_t>::max();
+    }
+
+private:
+
+    struct State
+    {
+        /**
+         * The last time we checked whether to do an auto-GC, or an
+         * auto-GC finished.
+         */
+        std::chrono::time_point<std::chrono::steady_clock> lastGCCheck;
+
+        /**
+         * Whether auto-GC is running. If so, get gcFuture to wait for
+         * the GC to finish.
+         */
+        bool gcRunning = false;
+        std::shared_future<void> gcFuture;
+
+        /**
+         * How much disk space was available after the previous
+         * auto-GC. If the current available disk space is below
+         * minFree but not much below availAfterGC, then there is no
+         * point in starting a new GC.
+         */
+        uint64_t availAfterGC = std::numeric_limits<uint64_t>::max();
+    };
+
+    Sync<State> _state;


remember this stuff cannot go here, because this mixin is also used by the remote stores. It should go in local store or a new mixin class

nixos-discourse · 2024-02-02T11:32:46Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-01-30-nix-team-meeting-minutes-119/39185/1

roberth · 2024-04-02T16:04:35Z

Please mark as ready for review when the following are addressed:

nixos-discourse · 2024-04-17T21:04:35Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-04-10-nix-team-meeting-138/43585/1

edolstra added 7 commits February 15, 2023 16:08

Move autoGC() into GcStore

96acb0b

nix store gc: Use auto-GC

8dcbcc0

This makes 'nix store gc' observe the same policy as auto-GC (which of course can be overriden from the command line via --max-free and --min-free).

Support unit prefixes in configuration settings

e276375

Worker::run(): Use GcStore

6972728

Add option 'auto-gc' to enable automatic garbage collection

6442fbd

This is no longer determined by 'min-free', which now defaults to infinity. The reason is to make 'min-free' behave consistently for auto-GC and explicit 'nix store gc' invocations.

Rename min-free/max-free to gc-threshold/gc-limit

a2bb5ed

nix store gc: Update manpage

ffa8dc7

edolstra requested review from fricklerhandwerk and thufschmitt as code owners February 16, 2023 15:42

edolstra added 2 commits February 16, 2023 17:21

Fix macOS build

3a72847

Fix warning

7f65c55

fricklerhandwerk reviewed Feb 16, 2023

View reviewed changes

src/nix/store-gc.md Outdated Show resolved Hide resolved

src/nix/store-gc.md Outdated Show resolved Hide resolved

src/nix/store-gc.md Outdated Show resolved Hide resolved

Apply suggestions from code review

3d118a7

Co-authored-by: Valentin Gagarin <[email protected]>

roberth reviewed Feb 20, 2023

View reviewed changes

src/nix/store-gc.md Outdated Show resolved Hide resolved

fricklerhandwerk mentioned this pull request Feb 20, 2023

nix store gc: Stabilize #7608

Draft

4 tasks

fricklerhandwerk added the new-cli Relating to the "nix" command label Feb 24, 2023

thufschmitt assigned roberth Mar 20, 2023

Ericson2314 self-assigned this May 26, 2023

github-actions bot added documentation with-tests Issues related to testing. PRs with tests have some priority store Issues and pull requests concerning the Nix store labels Aug 28, 2023

fricklerhandwerk reviewed Aug 28, 2023

View reviewed changes

doc/manual/src/release-notes/rl-next.md Outdated Show resolved Hide resolved

src/libstore/globals.hh Outdated Show resolved Hide resolved

src/libstore/globals.hh Outdated Show resolved Hide resolved

tomberek mentioned this pull request Sep 1, 2023

CLI stabilization effort #7701

Open

86 tasks

thufschmitt added this to the CLI Stabilisation milestone Sep 4, 2023

roberth marked this pull request as draft December 22, 2023 16:15

edolstra added 2 commits January 8, 2024 14:33

Merge remote-tracking branch 'origin/master' into nix-store-gc

a8c6da2

Merge branch 'nix-store-gc' of github.com:edolstra/nix into nix-store-gc

3f516e2

edolstra marked this pull request as ready for review January 8, 2024 13:42

edolstra and others added 3 commits January 8, 2024 14:46

Improve example

4ecb05b

Co-authored-by: Robert Hensing <[email protected]>

Tweak wording

0761b61

Co-authored-by: John Ericson <[email protected]>

Add links

66ed726

fricklerhandwerk reviewed Jan 8, 2024

View reviewed changes

Ericson2314 reviewed Jan 12, 2024

View reviewed changes

roberth marked this pull request as draft April 2, 2024 16:04

edolstra mentioned this pull request May 8, 2024

Support unit prefixes in configuration settings #10668

Merged

Uh oh!

Conversation

edolstra commented Feb 16, 2023 • edited by tomberek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Context

Checklist for maintainers

Uh oh!

Ericson2314 commented Feb 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ericson2314 commented Feb 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nixos-discourse commented Feb 17, 2023

Uh oh!

Uh oh!

fricklerhandwerk commented Feb 20, 2023

Uh oh!

nixos-discourse commented Feb 20, 2023

Uh oh!

roberth commented Mar 12, 2023

Uh oh!

nixos-discourse commented Mar 22, 2023

Uh oh!

nixos-discourse commented May 31, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nixos-discourse commented Aug 28, 2023

Uh oh!

nixos-discourse commented Sep 4, 2023

Uh oh!

roberth commented Dec 22, 2023

Uh oh!

fricklerhandwerk Jan 8, 2024

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Jan 12, 2024

Choose a reason for hiding this comment

Uh oh!

nixos-discourse commented Feb 2, 2024

Uh oh!

roberth commented Apr 2, 2024

Uh oh!

nixos-discourse commented Apr 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

edolstra commented Feb 16, 2023 •

edited by tomberek

Loading

Ericson2314 commented Feb 16, 2023 •

edited

Loading