Implement Locking System Across Multiple Pavilion Instances

lib/pavilion/build_tracker.py

@@ -7,6 +7,7 @@
 from contextlib import contextmanager
 
 from pavilion.status_file import STATES
+from .lockfile import NFSLock
 
 
 class MultiBuildTracker:
@@ -25,14 +26,13 @@ def __init__(self):
         self.lock = threading.Lock()
         self._build_locks = {} # type: Dict[str, threading.Lock]
 
-    def register(self, test) -> "BuildTracker":
+    def register(self, test: 'TestRun') -> 'BuildTracker':
         """Register a builder, and get your own build tracker.
 
         :param test: The TestRun object to track.
         :return: A build tracker instance that can be used by builds directly."""
 
         tracker = BuildTracker(test, self)
-        hash = test.builder.build_hash
 
         with self.lock:
             # Test may actually be a TestRun object rather than a TestBuilder object,
@@ -42,28 +42,8 @@ def register(self, test) -> "BuildTracker":
             self.messages[test.builder] = []
             self.trackers[test.builder] = tracker
 
-            if hash not in self._build_locks:
-                self._build_locks[hash] = threading.Lock()
-
         return tracker
 
-    @contextmanager
-    def make_lock_context(self, hash: str, timeout: float = -1) -> ContextManager[bool]:
-        """Return a context manager to manage the build-specific lock.
-
-        :param str hash: The hash identifying the specific build.
-        :return: A context manager to manage the (optionally) timed lock
-        associated with the build."""
-
-        lock = self._build_locks[hash]
-
-        try:
-            result = lock.acquire(timeout=timeout)
-            yield result
-        finally:
-            if result:
-                lock.release()
-
     def update(self, builder, note, state=None):
         """Add a message for the given builder without changes the status.
 

lib/pavilion/builder.py

@@ -21,6 +21,7 @@
 import pavilion.errors
 from pavilion import extract, lockfile, utils, wget, create_files
 from pavilion.build_tracker import BuildTracker
+from pavilion.lockfile import NFSLock
 from pavilion.errors import TestBuilderError, TestConfigError
 from pavilion.status_file import TestStatusFile, STATES
 from pavilion.test_config import parse_timeout
@@ -408,7 +409,9 @@ def build(self, test_id: str, tracker: BuildTracker,
                 state=STATES.BUILD_WAIT,
                 note="Waiting on lock for build {}.".format(self.name))
 
-            timed_lock = mb_tracker.make_lock_context(self.build_hash)
+            # Pass a timeout here to prevent starvation
+            timed_lock = NFSLock(self.path.parent, self.name, timeout=5)
+
             with timed_lock as acquired:
                 # Make sure the build wasn't created while we waited for
                 # the lock.

lib/pavilion/lockfile.py

@@ -10,6 +10,8 @@
 from pathlib import Path
 from typing import Union, TextIO
 import threading
+from uuid import uuid4
+from time import sleep
 
 from pavilion import output
 from pavilion import utils
@@ -19,7 +21,7 @@
 
 
 class LockFile:
-    """An NFS friendly way to create a lock file. Locks contain information
+    """A custom lock object that is not NFS-friendly. Locks contain information
 on what host and user created the lock, and have a built in expiration
 date. To be used in a 'with' context.
 
@@ -295,7 +297,6 @@ def _warn(self, msg):
         output.fprint(self._errfile, msg, color=output.YELLOW)
 
 
-
 class LockFilePoker:
     """This context creates a thread that regularly 'pokes' a lockfile to make sure it
     doesn't expire."""
@@ -328,3 +329,73 @@ def __exit__(self, exc_type, exc_val, exc_tb):
 
         self._done_event.set()
         self._thread.join()
+
+
+class NFSLock:
+    """A custom lock object designed for use on NFS systems. The lock behaves like a queue:
+    each process declares its intent to take the lock, and the lock goes to whichever process
+    declared first. The object lessens, but does not fully resolve, some of the concurrency issues
+    related to NFS, particularly in the case where multiple Pavilion instances are working with
+    the same build. Intended to be invoked as a context manager, using the 'with' keyword.
+    """
+    def __init__(self, lock_dir: Path, build_name: str,
+        wait_time: float = 0.5, timeout: float = -1):
+        """
+        :param lock_dir: directory in which lockfiles will be created
+        :param build_name: full name of the build to which the lock controls access
+        :param wait_time: time to wait between checking status
+        :param timeout: time (in seconds) after which the lock times out.
+            A value of -1 indicates no timeout
+        """
+        self._lock_dir = lock_dir
+        self.build_name = build_name
+        self._wait_time = wait_time
+        self._timeout = timeout
+
+        # create a globally unique lockfile
+        self._lockfile = lock_dir / f"{build_name}-{uuid4()}.lock"
+
+    def set_timeout(self, timeout: float) -> None:
+        """Set the timeout for the lock object.
+
+        :param timeout: the length of the timeout, in seconds
+        """
+        self._timeout = timeout
+
+    def __enter__(self) -> bool:
+        """
+        Attempt to acquire the lock.
+
+        :return: True if lock has been successfully acquired; False if timed out
+        """
+        # Declare intent to take lock
+        self._lockfile.touch()
+
+        first = False
+
+        start = time.time()
+
+        while not first:
+            # Give other instances opportunity to declare intent
+            sleep(self._wait_time)
+
+            if self._timeout >= 0 and (time.time() - start > self._timeout):
+                return False
+
+            first = self._get_earliest() == self._lockfile
+
+        return True
+
+    def __exit__(self, exc_type, exc_val, exc_tb) -> None:
+        # Remove the lockfile, since no longer competing for lock
+        self._lockfile.unlink()
+
+    def _get_earliest(self) -> Path:
+        """Get the path to the lockfile that was created first.
+
+        :return: Path object to whichever lockfile was created first"""
+
+        lockfiles = self._lock_dir.glob(f'{self.build_name}-*.lock')
+
+        # Sort files by creation time, and return oldest
+        return sorted(lockfiles, key=lambda x: x.stat().st_ctime)[0]