Bug 275965 - periodic: add a daily zfs trim
Summary: periodic: add a daily zfs trim
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 14.0-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: Warner Losh
URL: https://github.com/freebsd/freebsd-sr...
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-27 13:47 UTC by Lexi Winter
Modified: 2024-08-05 14:37 UTC (History)
2 users (show)

See Also:
linimon: mfc-stable13?


Attachments
patch (3.11 KB, patch)
2023-12-27 13:48 UTC, Lexi Winter
no flags Details | Diff
trim-zfs.diff (4.92 KB, patch)
2024-04-20 20:02 UTC, Dmitry Wagin
no flags Details | Diff
v0 by AS (4.79 KB, patch)
2024-08-05 14:37 UTC, Anton Saietskii
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lexi Winter freebsd_triage 2023-12-27 13:47:46 UTC
when using ZFS on certain SSDs, it might be preferable to run a periodic 'zpool trim' rather than using autotrim.  this patch adds a new daily script "801.trim-zfs", controlled by "daily_trim_zfs_enable" (default NO) which does a daily trim of all pools or the pools listed in "daily_trim_zfs_pools".
Comment 1 Lexi Winter freebsd_triage 2023-12-27 13:48:10 UTC
Created attachment 247294 [details]
patch
Comment 2 commit-hook freebsd_committer freebsd_triage 2024-04-09 21:57:22 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=493908c4b45c7b75e4a6b4aea3b5b63ea5c268d5

commit 493908c4b45c7b75e4a6b4aea3b5b63ea5c268d5
Author:     Lexi Winter <lexi@le-fay.org>
AuthorDate: 2024-04-09 21:49:56 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2024-04-09 21:55:11 +0000

    periodic/daily/801.trim-zfs: Add a daily zfs trim script

    As mentioned in zpoolprops(7), on some SSDs, it may not be desirable to
    use ZFS autotrim because a large number of trim requests can degrade
    disk performance; instead, the pool should be manually trimmed at
    regular intervals.

    Add a new daily periodic script for this purpose, 801.trim-zfs.  If
    enabled (daily_trim_zfs_enable=YES; the default is NO), it will run a
    'zpool trim' operation on all online pools, or on the pools listed in
    'daily_trim_zfs_pools'.

    The trim is not started if the pool is degraded (which matches the
    behaviour of the existing 800.scrub-zfs script) or if a trim is already
    running on that pool.  Having autotrim enabled does not inhibit the
    periodic trim; it's sometimes desirable to run periodic trims even with
    autotrim enabled, because autotrim can elide trims for very small
    regions.

    PR:             275965
    MFC after:      1 week
    Reviewed by:    imp
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/956

 share/man/man5/periodic.conf.5                    | 11 ++++-
 usr.sbin/periodic/etc/daily/801.trim-zfs (new +x) | 59 +++++++++++++++++++++++
 usr.sbin/periodic/etc/daily/Makefile              |  3 +-
 usr.sbin/periodic/periodic.conf                   |  5 ++
 4 files changed, 76 insertions(+), 2 deletions(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-04-16 20:13:52 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3e69ab88a032ac43d433f787b715466a87e72817

commit 3e69ab88a032ac43d433f787b715466a87e72817
Author:     Lexi Winter <lexi@le-fay.org>
AuthorDate: 2024-04-09 21:49:56 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2024-04-16 19:54:20 +0000

    periodic/daily/801.trim-zfs: Add a daily zfs trim script

    As mentioned in zpoolprops(7), on some SSDs, it may not be desirable to
    use ZFS autotrim because a large number of trim requests can degrade
    disk performance; instead, the pool should be manually trimmed at
    regular intervals.

    Add a new daily periodic script for this purpose, 801.trim-zfs.  If
    enabled (daily_trim_zfs_enable=YES; the default is NO), it will run a
    'zpool trim' operation on all online pools, or on the pools listed in
    'daily_trim_zfs_pools'.

    The trim is not started if the pool is degraded (which matches the
    behaviour of the existing 800.scrub-zfs script) or if a trim is already
    running on that pool.  Having autotrim enabled does not inhibit the
    periodic trim; it's sometimes desirable to run periodic trims even with
    autotrim enabled, because autotrim can elide trims for very small
    regions.

    PR:             275965
    MFC after:      1 week
    Reviewed by:    imp
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/956

    (cherry picked from commit 493908c4b45c7b75e4a6b4aea3b5b63ea5c268d5)

 share/man/man5/periodic.conf.5                    | 11 ++++-
 usr.sbin/periodic/etc/daily/801.trim-zfs (new +x) | 59 +++++++++++++++++++++++
 usr.sbin/periodic/etc/daily/Makefile              |  3 +-
 usr.sbin/periodic/periodic.conf                   |  5 ++
 4 files changed, 76 insertions(+), 2 deletions(-)
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2024-04-17 20:10:15 UTC
^Triage: assign to committer.
Comment 5 Warner Losh freebsd_committer freebsd_triage 2024-04-17 22:49:05 UTC
Not sure what it makes sense to stable 13...
Comment 6 Dmitry Wagin 2024-04-20 20:02:30 UTC
Created attachment 250113 [details]
trim-zfs.diff

- add threshold option
- pool specific threshold
- add rate option
- pool specific rate
- skip trimming when already trimming
- skip trimming when scrubbing in progress
- skip trimming when resilvering in progress
- update config and man
Comment 7 Lexi Winter freebsd_triage 2024-04-20 20:12:44 UTC
> - skip trimming when already trimming

i deliberately didn't do this, as i mentioned in the commit message, because autotrim may not trim all blocks immediately (it prefers to aggregate smaller blocks into larger ranges), so the admin may want to run trim periodically to free these small ranges.

so it might make sense for this to be a configurable option, although i have no preference on whether it's enabled by default.
Comment 8 Dmitry Wagin 2024-04-20 20:22:05 UTC
(In reply to Lexi Winter from comment #7)

zpool status ${pool} | grep -q "trimming"

It's about this, I changed it a little
Comment 9 Anton Saietskii 2024-07-24 11:26:41 UTC
(In reply to Dmitry Wagin from comment #6)

> - skip trimming when already trimming
> - skip trimming when scrubbing in progress
> - skip trimming when resilvering in progress
IMO these are most important things that could ever be in trim script -- I'd really like to see this reviewed and commited.
While it's not -- I'm going to apply patch and test it locally.
Comment 10 Anton Saietskii 2024-08-05 14:37:37 UTC
Created attachment 252519 [details]
v0 by AS

(In reply to Dmitry Wagin from comment #6)

Most obvoius thing is that we shouldn't run expensive and slow 'zpool history' more than once, so I changed relevant lines to pass two expressions to sed.

Few other notes:
1. Result of sed processing history can't be zero length unless we're trying to catch some really weird stuff like SEGFAULT, so likely `if [-z "$last_trim ...` can be removed.
2. Regexp can be simplified further by replacing it by single `s/^([0-9.:-]{19}) zpool (create|trim) .*$/\1/p'`, however I didn't test that yet.
3. I'm not sure if mixing tabs and 4 spaces is correct way to indent.