[pve-devel] [PATCH cluster 1/1] cfs lock: attempt to acquire lock more frequently

[Fiona Ebner <f.ebner@proxmox.com>]

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/20251103162330.112603-5-f.ebner@proxmox.com/
[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 <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 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
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel