Merge pull request #10496 from ThomasWaldmann/lock-recheck-delay-9899

locking: BORG_LOCK_RECHECK_DELAY sets the race recheck delay, fixes #9899

docs/faq.rst

@@ -40,6 +40,11 @@ clocks of concurrently active clients differ by more than a few minutes.
 The storage's own clock does not need to be correct - it is only used as a
 common reference between the clients.
 
+Concurrent clients rely on the storage listing a newly written lock object
+immediately. If it only does so after a lag (e.g. NFS shared by several clients,
+or some cloud storages used via rclone), set ``BORG_LOCK_RECHECK_DELAY`` on all
+clients, see :ref:`storelocking`.
+
 Can I back up to multiple swapped backup targets?
 --------------------------------------------------
 

docs/internals/data-structures.rst

@@ -1216,6 +1216,22 @@ Where the storage backend provides object timestamps (file, sftp, s3 and
 current rest servers - but not rclone), borg additionally uses the lock
 object's store-side mtime, which is stamped by the storage's clock.
 
+To acquire a lock, borg lists the lock objects, creates its own lock object if
+nothing forbids it, waits for the race recheck delay and lists again, to detect
+other clients that created theirs at the same time (if so, it backs off and
+retries). This needs storage with list-after-write consistency: a listing started
+after a lock object was written must contain it. Then, of two clients racing for
+the lock, at least the one that created its lock object last sees the other's, so
+they can not both get an exclusive lock. Local filesystems, sftp, rest
+(``borg serve --rest``) and S3 as provided by AWS or MinIO give this guarantee and
+the default delay (0.01s) is fine.
+
+If the storage only lists a new object after a lag (e.g. NFS clients caching
+directory listings, or some cloud storages used via rclone), set
+``BORG_LOCK_RECHECK_DELAY`` to at least that lag (in seconds) on all clients
+using the repository: then, the client that created its lock object last still
+sees the other one.
+
 Using that information, borg implements:
 
 - lock auto-removal if the owner process is dead. the primary purpose of this

docs/usage/general/environment.rst.inc

@@ -74,6 +74,16 @@ General:
     BORG_LOCK_WAIT
         You can set the default value for the ``--lock-wait`` option with this, so
         you do not need to give it as a command line option.
+    BORG_LOCK_RECHECK_DELAY
+        When acquiring the repository lock, borg creates its lock object, waits for this
+        many seconds (default: 0.01) and then lists the lock objects again to detect other
+        clients that created theirs at the same time. The default is fine for storage that
+        lists a new object immediately (local filesystems, sftp, ssh / rest, AWS S3, MinIO).
+        If your storage lists new objects only after a lag (e.g. NFS shared by several
+        clients, which caches directory listings, or some cloud storages used via rclone),
+        set this to at least that lag (e.g. 2 for a lag of up to 2 seconds) on all clients
+        using the repository, so that concurrent clients do not both get an exclusive lock.
+        See :ref:`storelocking`.
     BORG_LOGGING_CONF
         When set, use the given filename as INI-style logging configuration (see
         https://docs.python.org/3/library/logging.config.html#configuration-file-format).

src/borg/archiver/_common.py

@@ -315,6 +315,7 @@ rst_plain_text_references = {
     "home_config_borg": 'FAQ -> "How important is the borg config directory?"',
     "home_data_borg": 'FAQ -> "How important is the borg data directory?"',
     "json_output": "Internals -> All about JSON: How to develop frontends",
+    "storelocking": "Internals -> Data structures and file formats -> Locks (storelocking)",
 }
 
 

src/borg/archiver/help_cmd.py

@@ -671,6 +671,16 @@ class HelpMixIn:
             BORG_LOCK_WAIT
                 You can set the default value for the ``--lock-wait`` option with this, so
                 you do not need to give it as a command line option.
+            BORG_LOCK_RECHECK_DELAY
+                When acquiring the repository lock, borg creates its lock object, waits for this
+                many seconds (default: 0.01) and then lists the lock objects again to detect other
+                clients that created theirs at the same time. The default is fine for storage that
+                lists a new object immediately (local filesystems, sftp, ssh / rest, AWS S3, MinIO).
+                If your storage lists new objects only after a lag (e.g. NFS shared by several
+                clients, which caches directory listings, or some cloud storages used via rclone),
+                set this to at least that lag (e.g. 2 for a lag of up to 2 seconds) on all clients
+                using the repository, so that concurrent clients do not both get an exclusive lock.
+                See :ref:`storelocking`.
             BORG_LOGGING_CONF
                 When set, use the given filename as INI-style logging configuration (see
                 https://docs.python.org/3/library/logging.config.html#configuration-file-format).

src/borg/storelocking.py

@@ -26,6 +26,18 @@ creating theirs at the same time: an exclusive acquirer backs off if another exc
 up (and otherwise waits for remaining shared locks to go away), a shared acquirer backs off if an
 exclusive lock showed up. This is tried at least once, then retried until the timeout.
 
+This needs a store with list-after-write consistency: a listing started after a lock object was
+written must contain it. Then, of two clients racing for the lock, at least the one that created its
+lock object last sees the other's in its second listing, so they can not both get an exclusive lock
+(they might both back off, then they retry). Local filesystems, sftp, rest (``borg serve --rest``)
+and S3 as provided by AWS or MinIO give this guarantee.
+
+Between creating the lock object and the second listing, acquire() waits for the "race recheck
+delay" (default: 0.01s, BORG_LOCK_RECHECK_DELAY overrides it). With list-after-write consistency, it
+is not needed for correctness. Some stores only show a new object in listings after a lag, e.g. NFS
+clients caching directory listings or some cloud storages behind rclone: there, a delay of at least
+that lag is needed so that the client that created its lock object last still sees the other one.
+
 Staleness
 ---------
 A lock whose owner died (crash, power loss, suspended laptop, ...) must not block others forever, so
@@ -83,6 +95,8 @@ might have acquired its own lock meanwhile), so there is no safe way to continue
 
 import datetime
 import json
+import math
+import os
 import random
 import threading
 import time
@@ -104,6 +118,23 @@ logger = create_logger(__name__)
 # whole, so concurrent readers (e.g. a LockRefresher thread) never see a torn mix of its fields.
 LockAnchor = namedtuple("LockAnchor", "key dt mtime monotonic")
 
+DEFAULT_RACE_RECHECK_DELAY = 0.01  # [s], enough for stores with list-after-write consistency
+
+
+def get_race_recheck_delay():
+    """Return the race recheck delay [s]: BORG_LOCK_RECHECK_DELAY if set, else the default, see "Acquiring"."""
+    value = os.environ.get("BORG_LOCK_RECHECK_DELAY")
+    if not value:
+        return DEFAULT_RACE_RECHECK_DELAY
+    try:
+        delay = float(value)
+    except ValueError:
+        raise Error(f"BORG_LOCK_RECHECK_DELAY must be a number of seconds, but is: {value!r}") from None
+    if not math.isfinite(delay) or delay < 0:
+        raise Error(f"BORG_LOCK_RECHECK_DELAY must be a finite, non-negative number of seconds, but is: {value!r}")
+    return delay
+
+
 # why a refresh gives up: our own lock object is gone, see refresh().
 LOCK_KILLED_MSG = (
     "Our lock was killed by another borg (it considered our lock stale, e.g. because this machine "
@@ -199,7 +230,7 @@ class Lock:
         self.is_exclusive = exclusive
         self.sleep = sleep
         self.timeout = timeout
-        self.race_recheck_delay = 0.01  # local: 0.01, network/slow remote: >= 1.0
+        self.race_recheck_delay = get_race_recheck_delay()
         self.other_locks_go_away_delay = 0.1  # local: 0.1, network/slow remote: >= 1.0
         self.retry_delay_min = 1.0
         self.retry_delay_max = 5.0

src/borg/testsuite/storelocking_test.py

@@ -13,6 +13,7 @@ from .fslocking_test import free_pid  # NOQA
 from ..crypto.key import AESOCBKey, store_hash
 from ..platform import get_process_id
 from .. import storelocking
+from ..helpers import Error
 from ..storelocking import NotLocked, LockTimeout
 
 LOCK_KEY = AESOCBKey(None)
@@ -475,6 +476,33 @@ def test_lock_object_is_sealed(lockstore):
     lock.release()
 
 
+@pytest.mark.parametrize("value, expected", [(None, 0.01), ("", 0.01), ("0", 0.0), ("2", 2.0), ("0.5", 0.5)])
+def test_race_recheck_delay_from_env(monkeypatch, value, expected):
+    if value is None:
+        monkeypatch.delenv("BORG_LOCK_RECHECK_DELAY", raising=False)
+    else:
+        monkeypatch.setenv("BORG_LOCK_RECHECK_DELAY", value)
+    assert storelocking.get_race_recheck_delay() == expected
+
+
+@pytest.mark.parametrize("value", ["abc", "-1", "nan", "inf"])
+def test_race_recheck_delay_from_env_invalid(monkeypatch, lockstore, value):
+    monkeypatch.setenv("BORG_LOCK_RECHECK_DELAY", value)
+    with pytest.raises(Error, match="BORG_LOCK_RECHECK_DELAY"):
+        Lock(lockstore, exclusive=True, id=ID1)
+
+
+@pytest.mark.parametrize("exclusive", [True, False])
+def test_acquire_waits_race_recheck_delay(monkeypatch, lockstore, exclusive):
+    # acquire() waits for the configured delay between creating its lock object and listing again.
+    monkeypatch.setenv("BORG_LOCK_RECHECK_DELAY", "1.5")
+    sleeps = []
+    monkeypatch.setattr(storelocking.time, "sleep", sleeps.append)
+    lock = Lock(lockstore, exclusive=exclusive, id=ID1).acquire()
+    assert sleeps == [1.5]
+    lock.release()
+
+
 def write_unreadable_lock(lockstore, content, *, mtime=None):
     key = store_hash(content).hexdigest()
     lockstore.store(f"locks/{key}", content)