From a5a6fc0f58f2ee491f426391dfbf9ae5c910d161 Mon Sep 17 00:00:00 2001 From: pajkastare Date: Mon, 23 Oct 2023 21:43:46 +0200 Subject: [PATCH 1/3] Fixes jimsalterjrs/sanoid#851 --- sanoid | 59 ++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/sanoid b/sanoid index 8848e3b..6373c35 100755 --- a/sanoid +++ b/sanoid @@ -4,7 +4,7 @@ # from http://www.gnu.org/licenses/gpl-3.0.html on 2014-11-17. A copy should also be available in this # project's Git repository at https://github.com/jimsalterjrs/sanoid/blob/master/LICENSE. -$::VERSION = '2.2.0'; +$::VERSION = '2.2.1'; my $MINIMUM_DEFAULTS_VERSION = 2; use strict; @@ -34,6 +34,14 @@ if (keys %args < 4) { $args{'cron'} = 1; $args{'verbose'} = 1; } +my $no_need_for_cache_update = 0; +# Do not update the snapshot cache file if _only_ "--monitor-*" action commands are given (ignore "--verbose", "--configdir" etc) +if (($args{'monitor-snapshots'} || $args{'monitor-health'} || $args{'monitor-capacity'}) && ! ($args{'cron'} || $args{'force-update'} || $args{'take-snapshots'} || $args{'prune-snapshots'} || $args{'force-prune'})) { + # The command combination above must not assert true for any command that takes or prunes snapshots + # As long as no snapshots are taken, no conflict with the $forcecacheupdate variable below should occur + $no_need_for_cache_update = 1; + if ($args{'debug'}) { print "DEBUG: command combo means that the cache file (provided it exists) will not be updated regardless of age.\n"; } +} # for compatibility reasons, older versions used hardcoded command paths $ENV{'PATH'} = $ENV{'PATH'} . ":/bin:/sbin"; @@ -813,31 +821,42 @@ sub getsnaps { my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size, $atime, $mtime, $ctime, $blksize, $blocks) = stat($cache); if ( $forcecacheupdate || ! -f $cache || (time() - $mtime) > $cacheTTL ) { - if (checklock('sanoid_cacheupdate')) { - writelock('sanoid_cacheupdate'); - if ($args{'verbose'}) { - if ($args{'force-update'}) { - print "INFO: cache forcibly expired - updating from zfs list.\n"; - } else { - print "INFO: cache expired - updating from zfs list.\n"; - } - } - open FH, "$zfs get -Hrpt snapshot creation |"; - @rawsnaps = ; - close FH; - - open FH, "> $cache" or die 'Could not write to $cache!\n'; - print FH @rawsnaps; - close FH; - removelock('sanoid_cacheupdate'); - } else { - if ($args{'verbose'}) { print "INFO: deferring cache update - valid cache update lock held by another sanoid process.\n"; } + if ( -f $cache && ! $forcecacheupdate && $no_need_for_cache_update ) { + # Even though $forcecacheupdate and $no_need_for_cache_update should never be true at the same time, let $forcecacheupdate take precedence + if ($args{'debug'}) { print "DEBUG: no need to update cache even though it's expired, so don't.\n"; } open FH, "< $cache"; @rawsnaps = ; close FH; + } else { + if (checklock('sanoid_cacheupdate')) { + writelock('sanoid_cacheupdate'); + if ($args{'verbose'}) { + if ($args{'force-update'}) { + print "INFO: cache forcibly expired - updating from zfs list.\n"; + } else { + print "INFO: cache expired - updating from zfs list.\n"; + } + } + open FH, "$zfs get -Hrpt snapshot creation |"; + @rawsnaps = ; + close FH; + + open FH, "> $cache" or die 'Could not write to $cache!\n'; + print FH @rawsnaps; + close FH; + removelock('sanoid_cacheupdate'); + } else { + if ($args{'verbose'}) { print "INFO: deferring cache update - valid cache update lock held by another sanoid process.\n"; } + open FH, "< $cache"; + @rawsnaps = ; + close FH; + } } } else { # if ($args{'debug'}) { print "DEBUG: cache not expired (" . (time() - $mtime) . " seconds old with TTL of $cacheTTL): pulling snapshot list from cache.\n"; } + if ( $no_need_for_cache_update ) { + if ($args{'debug'}) { print "DEBUG: cache has not expired, so will not update it, but wouldn't have even if it had.\n"; } + } open FH, "< $cache"; @rawsnaps = ; close FH; From a8c15c977a96476d74b34b7c3e1766aad3988b3f Mon Sep 17 00:00:00 2001 From: pajkastare Date: Wed, 24 Jan 2024 13:32:22 +0100 Subject: [PATCH 2/3] Fixes jimsalterjrs/sanoid#851, updated based on review in discussion thread --- sanoid | 64 +++++++++++++++++++++++++--------------------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/sanoid b/sanoid index 6373c35..a22bc87 100755 --- a/sanoid +++ b/sanoid @@ -4,7 +4,7 @@ # from http://www.gnu.org/licenses/gpl-3.0.html on 2014-11-17. A copy should also be available in this # project's Git repository at https://github.com/jimsalterjrs/sanoid/blob/master/LICENSE. -$::VERSION = '2.2.1'; +$::VERSION = '2.2.0'; my $MINIMUM_DEFAULTS_VERSION = 2; use strict; @@ -34,13 +34,17 @@ if (keys %args < 4) { $args{'cron'} = 1; $args{'verbose'} = 1; } -my $no_need_for_cache_update = 0; -# Do not update the snapshot cache file if _only_ "--monitor-*" action commands are given (ignore "--verbose", "--configdir" etc) + + +my $cacheTTL = 900; # 15 minutes + +# Allow a much older snapshot cache file than default if _only_ "--monitor-*" action commands are given +# (ignore "--verbose", "--configdir" etc) if (($args{'monitor-snapshots'} || $args{'monitor-health'} || $args{'monitor-capacity'}) && ! ($args{'cron'} || $args{'force-update'} || $args{'take-snapshots'} || $args{'prune-snapshots'} || $args{'force-prune'})) { # The command combination above must not assert true for any command that takes or prunes snapshots # As long as no snapshots are taken, no conflict with the $forcecacheupdate variable below should occur - $no_need_for_cache_update = 1; - if ($args{'debug'}) { print "DEBUG: command combo means that the cache file (provided it exists) will not be updated regardless of age.\n"; } + $cacheTTL = 18000; # 5 hours + if ($args{'debug'}) { print "DEBUG: command combo means that the cache file (provided it exists) will be allowed to be older than default.\n"; } } # for compatibility reasons, older versions used hardcoded command paths @@ -66,7 +70,6 @@ make_path($run_dir); # if we call getsnaps(%config,1) it will forcibly update the cache, TTL or no TTL my $forcecacheupdate = 0; my $cache = "$cache_dir/snapshots.txt"; -my $cacheTTL = 900; # 15 minutes my %snaps = getsnaps( \%config, $cacheTTL, $forcecacheupdate ); my %pruned; my %capacitycache; @@ -821,42 +824,31 @@ sub getsnaps { my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size, $atime, $mtime, $ctime, $blksize, $blocks) = stat($cache); if ( $forcecacheupdate || ! -f $cache || (time() - $mtime) > $cacheTTL ) { - if ( -f $cache && ! $forcecacheupdate && $no_need_for_cache_update ) { - # Even though $forcecacheupdate and $no_need_for_cache_update should never be true at the same time, let $forcecacheupdate take precedence - if ($args{'debug'}) { print "DEBUG: no need to update cache even though it's expired, so don't.\n"; } + if (checklock('sanoid_cacheupdate')) { + writelock('sanoid_cacheupdate'); + if ($args{'verbose'}) { + if ($args{'force-update'}) { + print "INFO: cache forcibly expired - updating from zfs list.\n"; + } else { + print "INFO: cache expired - updating from zfs list.\n"; + } + } + open FH, "$zfs get -Hrpt snapshot creation |"; + @rawsnaps = ; + close FH; + + open FH, "> $cache" or die 'Could not write to $cache!\n'; + print FH @rawsnaps; + close FH; + removelock('sanoid_cacheupdate'); + } else { + if ($args{'verbose'}) { print "INFO: deferring cache update - valid cache update lock held by another sanoid process.\n"; } open FH, "< $cache"; @rawsnaps = ; close FH; - } else { - if (checklock('sanoid_cacheupdate')) { - writelock('sanoid_cacheupdate'); - if ($args{'verbose'}) { - if ($args{'force-update'}) { - print "INFO: cache forcibly expired - updating from zfs list.\n"; - } else { - print "INFO: cache expired - updating from zfs list.\n"; - } - } - open FH, "$zfs get -Hrpt snapshot creation |"; - @rawsnaps = ; - close FH; - - open FH, "> $cache" or die 'Could not write to $cache!\n'; - print FH @rawsnaps; - close FH; - removelock('sanoid_cacheupdate'); - } else { - if ($args{'verbose'}) { print "INFO: deferring cache update - valid cache update lock held by another sanoid process.\n"; } - open FH, "< $cache"; - @rawsnaps = ; - close FH; - } } } else { # if ($args{'debug'}) { print "DEBUG: cache not expired (" . (time() - $mtime) . " seconds old with TTL of $cacheTTL): pulling snapshot list from cache.\n"; } - if ( $no_need_for_cache_update ) { - if ($args{'debug'}) { print "DEBUG: cache has not expired, so will not update it, but wouldn't have even if it had.\n"; } - } open FH, "< $cache"; @rawsnaps = ; close FH; From 01053e6cceccf2d5194b5bc6e61277685146a079 Mon Sep 17 00:00:00 2001 From: pajkastare Date: Wed, 24 Jan 2024 13:51:24 +0100 Subject: [PATCH 3/3] Removed unnecessary comment, no code change --- sanoid | 1 - 1 file changed, 1 deletion(-) diff --git a/sanoid b/sanoid index a22bc87..295957b 100755 --- a/sanoid +++ b/sanoid @@ -42,7 +42,6 @@ my $cacheTTL = 900; # 15 minutes # (ignore "--verbose", "--configdir" etc) if (($args{'monitor-snapshots'} || $args{'monitor-health'} || $args{'monitor-capacity'}) && ! ($args{'cron'} || $args{'force-update'} || $args{'take-snapshots'} || $args{'prune-snapshots'} || $args{'force-prune'})) { # The command combination above must not assert true for any command that takes or prunes snapshots - # As long as no snapshots are taken, no conflict with the $forcecacheupdate variable below should occur $cacheTTL = 18000; # 5 hours if ($args{'debug'}) { print "DEBUG: command combo means that the cache file (provided it exists) will be allowed to be older than default.\n"; } }