More modular locking - this is the first part of a series of improvements.

bin/pg_backrest.pl

@@ -19,6 +19,7 @@ use BackRest::Config;
 use BackRest::Remote qw(DB BACKUP NONE);
 use BackRest::Db;
 use BackRest::File;
+use BackRest::Lock;
 use BackRest::Archive;
 use BackRest::Backup;
 use BackRest::Restore;
@@ -77,7 +78,7 @@ pg_backrest.pl [options] [operation]
 =cut
 
 ####################################################################################################################################
-# SAFE_EXIT - terminate all SSH sessions when the script is terminated
+# SAFE_EXIT - terminate all threads and SSH connections when the script is terminated
 ####################################################################################################################################
 sub safe_exit
 {
@@ -124,6 +125,11 @@ if (operationTest(OP_ARCHIVE_PUSH) || operationTest(OP_ARCHIVE_GET))
     safe_exit(new BackRest::Archive()->process());
 }
 
+####################################################################################################################################
+# Acquire the operation lock
+####################################################################################################################################
+lockAcquire(operationGet());
+
 ####################################################################################################################################
 # Open the log file
 ####################################################################################################################################
@@ -155,10 +161,6 @@ if (operationTest(OP_RESTORE))
         confess &log(ASSERT, 'restore operation must be performed locally on the db server');
     }
 
-    # Set the lock path
-    my $strLockPath = optionGet(OPTION_REPO_PATH) .  '/lock/' .
-                                      optionGet(OPTION_STANZA) . '-' . operationGet() . '.lock';
-
     # Do the restore
     new BackRest::Restore
     (
@@ -192,15 +194,6 @@ if (optionRemoteTypeTest(BACKUP))
     confess &log(ERROR, 'backup and expire operations must run on the backup host');
 }
 
-# Set the lock path
-my $strLockPath = optionGet(OPTION_REPO_PATH) .  '/lock/' . optionGet(OPTION_STANZA) . '-' . operationGet() . '.lock';
-
-if (!lock_file_create($strLockPath))
-{
-    &log(ERROR, 'backup process is already running for stanza ' . optionGet(OPTION_STANZA) . ' - exiting');
-    safe_exit(0);
-}
-
 # Initialize the db object
 my $oDb;
 
@@ -264,11 +257,14 @@ if (operationTest(OP_EXPIRE))
         optionGet(OPTION_RETENTION_ARCHIVE_TYPE, false),
         optionGet(OPTION_RETENTION_ARCHIVE, false)
     );
-
-    lock_file_remove();
 }
 
+# Cleanup backup (should be removed when backup becomes an object)
 backup_cleanup();
+
+# Release the operation lock
+lockRelease();
+
 safe_exit(0);
 };
 

lib/BackRest/Archive.pm

@@ -15,6 +15,7 @@ use lib dirname($0);
 use BackRest::Utility;
 use BackRest::Exception;
 use BackRest::Config;
+use BackRest::Lock;
 use BackRest::File;
 use BackRest::Remote;
 
@@ -397,13 +398,7 @@ sub pushProcess
     }
 
     # Create a lock file to make sure async archive-push does not run more than once
-    my $strLockPath = "${strArchivePath}/lock/" . optionGet(OPTION_STANZA) . "-archive.lock";
+    lockAcquire(operationGet());
-
-    if (!lock_file_create($strLockPath))
-    {
-        &log(DEBUG, 'archive-push process is already running - exiting');
-        return 0;
-    }
 
     # Open the log file
     log_file_set(optionGet(OPTION_REPO_PATH) . '/log/' . optionGet(OPTION_STANZA) . '-archive-async');
@@ -428,7 +423,7 @@ sub pushProcess
         }
     }
 
-    lock_file_remove();
+    lockRelease();
     return 0;
 }
 

lib/BackRest/Exception.pm

@@ -34,14 +34,15 @@ use constant
     ERROR_PARAM_REQUIRED               => 118,
     ERROR_ARCHIVE_MISMATCH             => 119,
     ERROR_ARCHIVE_DUPLICATE            => 120,
-    ERROR_VERSION_NOT_SUPPORTED        => 121
+    ERROR_VERSION_NOT_SUPPORTED        => 121,
+    ERROR_PATH_CREATE                  => 122
 };
 
 our @EXPORT = qw(ERROR_ASSERT ERROR_CHECKSUM ERROR_CONFIG ERROR_FILE_INVALID ERROR_FORMAT ERROR_OPERATION_REQUIRED
                  ERROR_OPTION_INVALID ERROR_OPTION_INVALID_VALUE ERROR_OPTION_INVALID_RANGE ERROR_OPTION_INVALID_PAIR
                  ERROR_OPTION_DUPLICATE_KEY ERROR_OPTION_NEGATE ERROR_OPTION_REQUIRED ERROR_POSTMASTER_RUNNING ERROR_PROTOCOL
                  ERROR_RESTORE_PATH_NOT_EMPTY ERROR_FILE_OPEN ERROR_FILE_READ ERROR_PARAM_REQUIRED ERROR_ARCHIVE_MISMATCH
-                 ERROR_ARCHIVE_DUPLICATE ERROR_VERSION_NOT_SUPPORTED);
+                 ERROR_ARCHIVE_DUPLICATE ERROR_VERSION_NOT_SUPPORTED ERROR_PATH_CREATE);
 
 ####################################################################################################################################
 # CONSTRUCTOR

lib/BackRest/Lock.pm

@@ -0,0 +1,127 @@
+####################################################################################################################################
+# LOCK MODULE
+####################################################################################################################################
+package BackRest::Lock;
+
+use strict;
+use warnings FATAL => qw(all);
+use Carp qw(confess);
+
+use Fcntl qw(:DEFAULT :flock);
+use File::Basename qw(dirname);
+use Exporter qw(import);
+
+use lib dirname($0) . '/../lib';
+use BackRest::Exception;
+use BackRest::Config;
+use BackRest::Utility;
+
+####################################################################################################################################
+# Exported Functions
+####################################################################################################################################
+our @EXPORT = qw(lockAcquire lockRelease);
+
+####################################################################################################################################
+# Global lock type and handle
+####################################################################################################################################
+my $strCurrentLockType;
+my $strCurrentLockFile;
+my $hCurrentLockHandle;
+
+####################################################################################################################################
+# lockPathName
+#
+# Get the base path where the lock file will be stored.
+####################################################################################################################################
+sub lockPathName
+{
+    my $strRepoPath = shift;
+
+    return "${strRepoPath}/lock";
+}
+
+####################################################################################################################################
+# lockFileName
+#
+# Get the lock file name.
+####################################################################################################################################
+sub lockFileName
+{
+    my $strLockType = shift;
+    my $strStanza = shift;
+    my $strRepoPath = shift;
+
+    return lockPathName($strRepoPath) . "/${strStanza}-${strLockType}.lock";
+}
+
+####################################################################################################################################
+# lockAcquire
+#
+# Attempt to acquire the specified lock.  If the lock is taken by another process return false, else take the lock and return true.
+####################################################################################################################################
+sub lockAcquire
+{
+    my $strLockType = shift;
+
+    # Cannot proceed if a lock is currently held
+    if (defined($strCurrentLockType))
+    {
+        confess &lock(ASSERT, "${strCurrentLockType} lock is already held");
+    }
+
+    # Create the lock path if it does not exist
+    if (! -e lockPathName(optionGet(OPTION_REPO_PATH)))
+    {
+        mkdir lockPathName(optionGet(OPTION_REPO_PATH))
+            or confess(ERROR, 'unable to create lock path ' . lockPathName(optionGet(OPTION_REPO_PATH)), ERROR_PATH_CREATE);
+    }
+
+    # Attempt to open the lock file
+    $strCurrentLockFile = lockFileName($strLockType, optionGet(OPTION_STANZA), optionGet(OPTION_REPO_PATH));
+
+    sysopen($hCurrentLockHandle, $strCurrentLockFile, O_WRONLY | O_CREAT)
+        or confess &log(ERROR, "unable to open lock file ${strCurrentLockFile}");
+
+    # Attempt to lock the lock file
+    if (!flock($hCurrentLockHandle, LOCK_EX | LOCK_NB))
+    {
+        close($hCurrentLockHandle);
+        return false;
+    }
+
+    # Set current lock type so we know we have a lock
+    $strCurrentLockType = $strLockType;
+
+    # Lock was successful
+    return true;
+}
+
+####################################################################################################################################
+# lockRelease
+####################################################################################################################################
+sub lockRelease
+{
+    my $strLockType = shift;
+
+    # Fail if there is no lock
+    if (!defined($strCurrentLockType))
+    {
+        confess &log(ASSERT, 'no lock is currently held');
+    }
+
+    # # Fail if the lock being released is not the one held
+    # if ($strLockType ne $strCurrentLockType)
+    # {
+    #     confess &log(ASSERT, "cannot remove lock ${strLockType} since ${strCurrentLockType} is currently held");
+    # }
+
+    # Remove the file
+    unlink($strCurrentLockFile);
+    close($hCurrentLockHandle);
+
+    # Undef lock variables
+    undef($strCurrentLockType);
+    undef($hCurrentLockHandle);
+}
+
+1;

lib/BackRest/Utility.pm

@@ -24,7 +24,7 @@ use Exporter qw(import);
 our @EXPORT = qw(version_get
                  data_hash_build trim common_prefix file_size_format execute
                  log log_file_set log_level_set test_set test_get test_check
-                 lock_file_create lock_file_remove hsleep wait_remainder
+                 hsleep wait_remainder
                  ini_save ini_load timestamp_string_get timestamp_file_string_get
                  TRACE DEBUG ERROR ASSERT WARN INFO OFF true false
                  TEST TEST_ENCLOSE TEST_MANIFEST_BUILD TEST_BACKUP_RESUME TEST_BACKUP_NORESUME FORMAT);
@@ -125,62 +125,6 @@ sub version_get
     return $strVersion;
 }
 
-####################################################################################################################################
-# LOCK_FILE_CREATE
-####################################################################################################################################
-sub lock_file_create
-{
-    my $strLockPathParam = shift;
-
-    my $strLockFile = $strLockPathParam . '/process.lock';
-
-    if (defined($hLockFile))
-    {
-        confess &lock(ASSERT, "${strLockFile} lock is already held");
-    }
-
-    $strLockPath = $strLockPathParam;
-
-    unless (-e $strLockPath)
-    {
-        if (system("mkdir -p ${strLockPath}") != 0)
-        {
-            confess &log(ERROR, "Unable to create lock path ${strLockPath}");
-        }
-    }
-
-    sysopen($hLockFile, $strLockFile, O_WRONLY | O_CREAT)
-        or confess &log(ERROR, "unable to open lock file ${strLockFile}");
-
-    if (!flock($hLockFile, LOCK_EX | LOCK_NB))
-    {
-        close($hLockFile);
-        return 0;
-    }
-
-    return $hLockFile;
-}
-
-####################################################################################################################################
-# LOCK_FILE_REMOVE
-####################################################################################################################################
-sub lock_file_remove
-{
-    if (defined($hLockFile))
-    {
-        close($hLockFile);
-
-        remove_tree($strLockPath) or confess &log(ERROR, "unable to delete lock path ${strLockPath}");
-
-        $hLockFile = undef;
-        $strLockPath = undef;
-    }
-    else
-    {
-        confess &log(ASSERT, 'there is no lock to free');
-    }
-}
-
 ####################################################################################################################################
 # WAIT_REMAINDER - Wait the remainder of the current second
 ####################################################################################################################################