kdave / btrfsmaintenance

Scripts for btrfs maintenance tasks like periodic scrub, balance, trim or defrag on selected mountpoints or directories.
GNU General Public License v2.0
897 stars 79 forks source link

Multiple runs of scheduled jobs when period differs from default #91

Closed davidmwilkinson closed 3 years ago

davidmwilkinson commented 3 years ago

I noticed this past month that the scrub job happened twice:

Jun 01 00:00:03 dave-lab systemd[1]: Started Scrub btrfs filesystem, verify block checksums.
...
Jun 01 15:14:17 dave-lab systemd[1]: btrfs-scrub.service: Succeeded.
Jun 01 15:14:17 dave-lab systemd[1]: btrfs-scrub.service: Consumed 2h 46min 28.412s CPU time.
Jun 01 15:14:17 dave-lab systemd[1]: Started Scrub btrfs filesystem, verify block checksums.

I had changed my scrub period to start at 4AM every 1st of the month prior to this. However, when I check via systemctl what the next trigger time is, it still reports that it's scheduled to start at midnight on July 1:

[~]$ cat /etc/sysconfig/btrfsmaintenance | grep "BTRFS_SCRUB_PERIOD"
BTRFS_SCRUB_PERIOD="*-*-01 04:00:00"
[~]$ systemctl status btrfs-scrub.timer                             
● btrfs-scrub.timer - Scrub btrfs filesystem, verify block checksums
     Loaded: loaded (/usr/lib/systemd/system/btrfs-scrub.timer; enabled; vendor preset: enabled)
    Drop-In: /etc/systemd/system/btrfs-scrub.timer.d
             └─schedule.conf
     Active: active (waiting) since Thu 2021-06-10 12:40:12 EDT; 58min ago
    Trigger: Thu 2021-07-01 00:00:00 EDT; 2 weeks 6 days left
   Triggers: ● btrfs-scrub.service
       Docs: man:btrfs-scrub

Jun 10 12:40:12 dave-lab systemd[1]: Started Scrub btrfs filesystem, verify block checksums.

systemctl shows 2 entries for TimersCalendar when investigating:

[~]$ systemctl show btrfs-scrub.timer | head -n 4
Unit=btrfs-scrub.service
TimersCalendar={ OnCalendar=*-*-01 04:00:00 ; next_elapse=Thu 2021-07-01 04:00:00 EDT }
TimersCalendar={ OnCalendar=*-*-01 00:00:00 ; next_elapse=Thu 2021-07-01 00:00:00 EDT }
OnClockChange=no

Upon inspection, this same thing occurs with the other periods I'd overridden:

[~]$ systemctl show btrfs-balance.timer | head -n 4
Unit=btrfs-balance.service
TimersCalendar={ OnCalendar=*-*-* 03:30:00 ; next_elapse=Fri 2021-06-11 03:30:00 EDT }
TimersCalendar={ OnCalendar=*-*-01 00:00:00 ; next_elapse=Thu 2021-07-01 00:00:00 EDT }
OnClockChange=no
[~]$ systemctl show btrfs-defrag.timer | head -n 4
Unit=btrfs-defrag.service
TimersCalendar={ OnCalendar=*-*-* 03:15:00 ; next_elapse=Fri 2021-06-11 03:15:00 EDT }
TimersCalendar={ OnCalendar=*-*-01 00:00:00 ; next_elapse=Thu 2021-07-01 00:00:00 EDT }
OnClockChange=no

But not with trim, which I had left disabled with the 'none' period:

[~]$ systemctl show btrfs-trim.timer | head -n 4
Unit=btrfs-trim.service
TimersCalendar={ OnCalendar=*-*-01 00:00:00 ; next_elapse=n/a }
OnClockChange=no
OnTimezoneChange=no

After some digging, I found this:

OnCalendar= Defines realtime (i.e. wallclock) timers with calendar event expressions. See systemd.time(7) for more information on the syntax of calendar event expressions. Otherwise, the semantics are similar to OnActiveSec= and related settings.

Note that timers do not necessarily expire at the precise time configured with this setting, as it is subject to the AccuracySec= setting below.

May be specified more than once, in which case the timer unit will trigger whenever any of the specified expressions elapse. Moreover calendar timers and monotonic timers (see above) may be combined within the same timer unit.

If the empty string is assigned to any of these options, the list of timers is reset (both OnCalendar= timers and monotonic timers, see above), and all prior assignments will have no effect.

The last paragraph of the section describes the change I'm proposing. Changing the drop-in files for the timers to this:

[Timer]
OnCalendar=
OnCalendar=$PERIOD

will reset the list of timers and respect only the period specified in the /etc/sysconfig/btrfsmaintenance file. Without this change, the default period is always used in addition to the values explicitly specified.

I've forked and made a commit for what I believe the change should be here: https://github.com/davidmwilkinson/btrfsmaintenance/commit/aa7cbc3c3054b8c3c8f353f04c0b588b946a8e70

I can open a PR if desired, but since it's a one line change it's up to you

Seb35 commented 3 years ago

This is a duplicate of #71.

I cannot speak for @kdave but I guess a PR would be welcome. I have modified according to your commit on systems where I use btrfs, and it works.

kdave commented 3 years ago

Thanks for the analysis and fix #94.