The cfs lock helper would only try N times to acquire a lock when a timeout of N seconds was specified (default 10). That is not very much and was noticed while testing patches for properly locking shared LVM storages during snapshot operations [0]. There, with only two contenders doing a loop of operations that require taking a cfs lock, one could already starve. With three instances of a synthetic reproducer [1] running on three nodes, not even 10 iterations each could be reached in testing.
Thomas suggested having the frequency of attempts to acquire the lock depend on how much time is left until hitting the timeout, rather than just increasing the frequency in general. Like this, contenders already waiting longer have better chances. Until reaching 10 seconds remaining, the sleep time for one iteration stays the same, namely 1 second. With less than 10 seconds remaining, the sleep time is the integer amount of seconds remaining divided by ten. With this approach, three instances of [1] can reliably reach 100 iterations each. [0]: https://lore.proxmox.com/pve-devel/[email protected]/ [1]: use v5.36; use Time::HiRes qw(usleep); use PVE::Cluster; my $count = shift or die "specify number of lock acquisitions\n"; for (my $i = 0; $i < $count; $i++) { PVE::Cluster::cfs_lock_storage( "foo", 10, sub { print "got lock $i\n"; usleep(500_000); }, ); die $@ if $@; usleep(100_000); } Suggested-by: Thomas Lamprecht <[email protected]> Signed-off-by: Fiona Ebner <[email protected]> --- src/PVE/Cluster.pm | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm index e829687..bdb465f 100644 --- a/src/PVE/Cluster.pm +++ b/src/PVE/Cluster.pm @@ -7,10 +7,12 @@ use Encode; use File::stat qw(); use File::Path qw(make_path); use JSON; +use List::Util; use Net::SSLeay; use POSIX qw(ENOENT); use Socket; use Storable qw(dclone); +use Time::HiRes qw(usleep); use PVE::Certificate; use PVE::INotify; @@ -622,19 +624,29 @@ my $cfs_lock = sub { my $timeout_err = sub { die "got lock request timeout\n"; }; local $SIG{ALRM} = $timeout_err; + my $slept_usec = 0; while (1) { - alarm($timeout); + my $slept_sec = int($slept_usec / 1_000_000); + # Below increases by the actual amount of time slept, so in principle, the value + # $timeout - $slept_sec could end up being zero. + my $remaining_timeout_sec = List::Util::max($timeout - $slept_sec, 1); + + alarm($remaining_timeout_sec); $got_lock = mkdir($filename); - $timeout = alarm(0) - 1; # we'll sleep for 1s, see down below last if $got_lock; - $timeout_err->() if $timeout <= 0; + my $sleep_usec = List::Util::min($remaining_timeout_sec, 10) * 100_000; + my $next_slept_sec = int(($slept_usec + $sleep_usec) / 1_000_000); - print STDERR "trying to acquire cfs lock '$lockid' ...\n"; + $timeout_err->() if $next_slept_sec >= $timeout; + + if ($next_slept_sec > $slept_sec) { # don't log more often than once per second + print STDERR "trying to acquire cfs lock '$lockid' ...\n"; + } utime(0, 0, $filename); # cfs unlock request - sleep(1); + $slept_usec += usleep($sleep_usec); } # fixed command timeout: cfs locks have a timeout of 120 -- 2.47.3 _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
