systemd: add weekly and monthly scrub timers #12193
Conversation
to use with systemd Pr: openzfs/zfs#12193 Signed-off-by: Georgy Yakovlev <[email protected]>
behlendorf
added
Component: Systemd
There was a problem hiding this comment.
Neat! My only concern is that we should document how to enable these timers. One option would be to add a few lines to the zpool-scrub.8 man page. Maybe something like, "On machines using systemd, scheduled scrub can be enabled by...".
I believe you also need to add the new units to the 50-zfs.present.in to set the default behavior.
https://www.freedesktop.org/software/systemd/man/systemd.preset.html
|
sure, will update manpage. thanks for suggestion. as for preset, we need to know pool names, hence we can't enable in via it's not "scrub everything", those timers are opt-in on per-pool basis. so to enable a weekly timer for
|
|
I see, well I think we'll want to make sure this functionality is disabled by default. Which since it requires a pool name seems like it will be the case. |
|
yes it's disabled by default and completely opt-in added short description how to use timers: |
There was a problem hiding this comment.
On Debian, we have this script:
#!/bin/sh -eu
# Scrub all healthy pools that are not already scrubbing.
zpool list -H -o health,name 2>&1 | \
awk '$1 ~ /^ONLINE/ { print $2; }' | \
while read pool
do
if ! zpool status "$pool" | grep -q "scrub in progress"
then
# Ignore errors (i.e. HDD pools),
# and continue with scrubing other pools.
zpool scrub "$pool" || true
fi
done
(I'm not sure what that "HDD pools" comment is about. That's new and makes no sense to me.)
which runs from cron.d like this:
# Scrub the second Sunday of every month.
24 0 8-14 * * root if [ $(date +\%w) -eq 0 ] && [ -x /usr/lib/zfs-linux/scrub ]; then /usr/lib/zfs-linux/scrub; fi
Can you comment on why you chose a per-pool approach rather than a "scrub all the pools" approach? (I'm not saying it's wrong, and I can likely argue a case either way. I'd just like to hear your thinking, if you don't mind.)
Specific advantages of "scrub all pools" would be that it can be enabled by default and it would provide a relatively clean upgrade path for Debian, which is already taking that approach (and does for mdadm too, at least from cron, FWIW).
I'm not sure if we need to offer both weekly and monthly scrubs. A weekly scrub seems overly frequent to me. Do you use weekly scrubs yourself?
|
Hi all, how does this interact with default scrub scheduling? I was watching a YT video tonight and it started to intermittently freeze. I knew it was ZFS related because the always-idle cold-storage zpool HDDs in front of me were making noise and flashing lights, so I checked and both of my two pools were scrubbing. I don't mind an auto scrub, but before 2am is unacceptable to me. The "enable" commands here don't appear to specify a time. What's the default behaviour, timing wise, and how can it be controlled to be acceptable to the end user's particular scenario? |
there's no default scheduling to interact with. some downstream addition probably does it for you, similar to one mentioned in #12193 (review). in that case you don't need those timers, probably. timer scheduling deferred to systemd. check |
I, personally, dislike some of debian/ubuntu automagic.
this can be different PR. this one provides per pool units. and timings can be edited separately.
I do, for root/boot pool, which changes VERY often, and is kinda small, no more than 256G usually. But again, this PR is about providing reasonably flexible timers, not a solution that fits all and enabled by default. |
|
I'm not sure why GitHub won't let me reply directly to your comment... Given that you tested the .in thing, it seems fine to keep it simple (i.e. as you already have it). |
|
Thanks @gyakovlev - you were spot on, from another ticket I found an older version of this: Which I'll edit to suit myself, your approach sounds good, nice and flexible but simple/easy if desired. |
|
@rlaager anything else to do here to get approval? I still see the red icon requesting changes =) |
|
I'd love to see this merged, and it would obsolete my version (spoiler: I call |
|
finally found time to implement things suggested in comments. thank you very much for feedback and ideas. unit now properly waits for scrub to exit ( via and exits and becomes inactive once scrub is finished I haven't tested shutting down with I left multiple commits for easier review, but ofc I can squash if you wish. |
|
forgot to mention. |
gyakovlev
force-pushed
the
scrub-timers
branch
from
c007fa9 to
ab60eee
Compare
|
@rlaager I was very busy but finally found time to finish it. I haven't tested the shutdown/stop while scrub is running though, will give it a test. |
Do we want an explicit stop of the scrub service to stop or pause the scrub? If stop, I suggest testing like this:
If pause, I suggest testing like this:
My inclination is that stopping the service should pause the scrub. This would allow someone to stop the service because they need to stop the disk I/O without losing the progress of their scrub. |
|
stumbled on this with this is status after scrub finished. so looks like in needs bash wrapper similar to |
|
ExecStop=/bin/sh -c '\
if /sbin/zpool status %i | grep "scrub in progress"; then\
exec /sbin/zpool scrub -p %i; fi'should do the trick I think. |
|
now it looks like this: I'll test the reboot scenario soon-ish too. |
|
I'm not sure if we need to actually check that the scrub is running before trying to pause. We could just pause and ignore errors: Checking has a race condition: if the scrub stops in between the check and the scrub, the pause will still fail. Just eating errors avoids that, but has the downside of eating real errors if pausing the scrub fails for some reason. Given that pausing the scrub is extremely unlikely to fail in any other way, I think the right trade-off is to just ignore the errors. But I don't feel strongly about this. |
gyakovlev
force-pushed
the
scrub-timers
branch
from
ab60eee to
372fc09
Compare
|
no strong opinion on this too, so I implemented suggestion, race cond argument is true. |
|
Are there any remaining concerns with this PR? Or is mainly waiting on some testing? |
|
I tested, reboots look fine. zpool state after reboot: starting service after reboot resumes scrub as intended. so I think it's ready to merge, huge thanks to @rlaager for help and suggestions. also this patch was shipped in gentoo for quite some time and no bugs has been reported so far. users are happy. the only thing that may need attention is, when service is stopped after scrub is finished, systemd logs this:
it's cosmetic, but somewhat misleading. |
Currently, you have this: You could address that error with: Alternatively, if you're not going to suppress the error, you should be able to just prefix the command with a dash to ignore the exit status, eliminating the sh and true: |
|
gives:
so I opted in for this: this gives clean output if scrub completes: and and as you said I agree that catching errors for pause can be omitted, because it's already safe to reboot with scrub running anyway. |
gyakovlev
force-pushed
the
scrub-timers
branch
from
372fc09 to
4bac4ea
Compare
|
rebased |
timers can be enabled as follows: systemctl enable [email protected] --now systemctl enable [email protected] --now Each timer will pull in zfs-scrub@${poolname}.service, which is not schedule-specific. Signed-off-by: Georgy Yakovlev <[email protected]>
Signed-off-by: Georgy Yakovlev <[email protected]>
gyakovlev
force-pushed
the
scrub-timers
branch
from
4bac4ea to
edb85cf
Compare
|
done, removed exec |
openzfs/zfs#12193 Signed-off-by: Georgy Yakovlev <[email protected]>
behlendorf
added
Status: Accepted
|
@gyakovlev @rlaager thanks for working to get this one over the finish line. Merged. |
Timers can be enabled as follows: systemctl enable [email protected] --now systemctl enable [email protected] --now Each timer will pull in zfs-scrub@${poolname}.service, which is not schedule-specific. Added PERIODIC SCRUB section to zpool-scrub.8. Reviewed-by: Richard Laager <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Georgy Yakovlev <[email protected]> Closes openzfs#12193
Timers can be enabled as follows: systemctl enable [email protected] --now systemctl enable [email protected] --now Each timer will pull in zfs-scrub@${poolname}.service, which is not schedule-specific. Added PERIODIC SCRUB section to zpool-scrub.8. Reviewed-by: Richard Laager <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Georgy Yakovlev <[email protected]> Closes openzfs#12193
Timers can be enabled as follows: systemctl enable [email protected] --now systemctl enable [email protected] --now Each timer will pull in zfs-scrub@${poolname}.service, which is not schedule-specific. Added PERIODIC SCRUB section to zpool-scrub.8. Reviewed-by: Richard Laager <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Georgy Yakovlev <[email protected]> Closes openzfs#12193
infra/ops#506 1st Sat schedule can overlap with zfs-dump, which can cause excessive disk I/O remove [email protected] and changed to use the unit file introduced in OpenZFS 2.1.3 openzfs/zfs#12193
to use with systemd Pr: openzfs/zfs#12193 Signed-off-by: Georgy Yakovlev <[email protected]>
openzfs/zfs#12193 Signed-off-by: Georgy Yakovlev <[email protected]>
Motivation and Context
Provide 2 basic timers that leverage systemd scheduling tech.
Description
provide 3 extra files for use with systemd:
timers can be enabled as follows:
Each timer will pull in
zfs-scrub@${poolname}.service, which is notschedule-specific, but the
zfs-scrubwill receive pool argument from timer unit.Configuration provided is generic and simple.
Users can tweak parameters by using
systemctl --edit <unitname>How Has This Been Tested?
in example above scrub for zroot pool will be triggered weekly.
Types of changes
Checklist:
Signed-off-by.