From 270dce41b6fff0d0ad0769b0ea5df0011140f0b9 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 18 Jun 2024 10:43:54 +0800 Subject: [PATCH] Refactor lock module. Refactor the lock module to split command-specific logic from the basic file locking functionality. Command specific logic is now in command/lock.c. This will make it easier to implement new features such as repository locking and updating lock file contents on remotes. This implementation is essentially a drop-in replacement but there are a few differences. First, the lock names no longer require a path (the path is added in the lock module). Second, the timeout functionality has been removed since it was not being used. --- CONTRIBUTING.md | 4 +- doc/src/meson.build | 1 + doc/xml/contributing.xml | 4 +- doc/xml/release/2024/2.53.xml | 13 + src/Makefile.in | 1 + src/command/archive/get/get.c | 5 +- src/command/archive/push/push.c | 5 +- src/command/backup/backup.c | 10 +- src/command/control/stop.c | 7 +- src/command/exit.c | 4 +- src/command/info/info.c | 4 +- src/command/lock.c | 149 +++++++ src/command/lock.h | 35 ++ src/command/remote/remote.c | 3 +- src/common/lock.c | 486 +++++++++------------- src/common/lock.h | 64 ++- src/config/config.h | 12 +- src/config/load.c | 16 +- src/main.c | 3 +- src/meson.build | 1 + test/define.yaml | 18 +- test/src/common/harnessBackup.c | 7 +- test/src/common/harnessConfig.c | 4 +- test/src/common/harnessLock.c | 6 +- test/src/meson.build | 1 + test/src/module/command/archiveGetTest.c | 6 +- test/src/module/command/archivePushTest.c | 6 +- test/src/module/command/backupTest.c | 6 +- test/src/module/command/controlTest.c | 14 +- test/src/module/command/infoTest.c | 36 +- test/src/module/command/lockTest.c | 83 ++++ test/src/module/common/lockTest.c | 385 ++++------------- test/src/module/config/loadTest.c | 10 +- 33 files changed, 689 insertions(+), 720 deletions(-) create mode 100644 src/command/lock.c create mode 100644 src/command/lock.h create mode 100644 test/src/module/command/lockTest.c diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 40936a7cd..c8ef43242 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -470,7 +470,7 @@ HRN_FORK_BEGIN() { HRN_FORK_CHILD_BEGIN() { - TEST_RESULT_INT_NE(lockAcquireP(), -1, "create backup/expire lock"); + TEST_RESULT_BOOL(cmdLockAcquireP(), true, "create backup/expire lock"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -478,7 +478,7 @@ HRN_FORK_BEGIN() // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + cmdLockReleaseP(); } HRN_FORK_CHILD_END(); diff --git a/doc/src/meson.build b/doc/src/meson.build index 5ad5c9f87..756c24a48 100644 --- a/doc/src/meson.build +++ b/doc/src/meson.build @@ -30,6 +30,7 @@ src_doc = [ '../../src/command/command.c', '../../src/command/exit.c', '../../src/command/help/help.c', + '../../src/command/lock.c', '../../src/common/compress/bz2/common.c', '../../src/common/compress/bz2/compress.c', '../../src/common/compress/bz2/decompress.c', diff --git a/doc/xml/contributing.xml b/doc/xml/contributing.xml index f33e86ae3..73c91ad58 100644 --- a/doc/xml/contributing.xml +++ b/doc/xml/contributing.xml @@ -526,7 +526,7 @@ HRN_FORK_BEGIN() { HRN_FORK_CHILD_BEGIN() { - TEST_RESULT_INT_NE(lockAcquireP(), -1, "create backup/expire lock"); + TEST_RESULT_BOOL(cmdLockAcquireP(), true, "create backup/expire lock"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -534,7 +534,7 @@ HRN_FORK_BEGIN() // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + cmdLockReleaseP(); } HRN_FORK_CHILD_END(); diff --git a/doc/xml/release/2024/2.53.xml b/doc/xml/release/2024/2.53.xml index d89151187..cfc0e9a46 100644 --- a/doc/xml/release/2024/2.53.xml +++ b/doc/xml/release/2024/2.53.xml @@ -27,5 +27,18 @@

Allow alternative WAL segment sizes for PostgreSQL ≤ 10.

+ + + + + + + + + + +

Refactor lock module.

+ + diff --git a/src/Makefile.in b/src/Makefile.in index 259fdd189..07fec55fa 100644 --- a/src/Makefile.in +++ b/src/Makefile.in @@ -78,6 +78,7 @@ SRCS = \ command/control/start.c \ command/control/stop.c \ command/local/local.c \ + command/lock.c \ command/manifest/manifest.c \ command/repo/common.c \ command/repo/create.c \ diff --git a/src/command/archive/get/get.c b/src/command/archive/get/get.c index fd6503edd..bf624b573 100644 --- a/src/command/archive/get/get.c +++ b/src/command/archive/get/get.c @@ -13,6 +13,7 @@ Archive Get Command #include "command/archive/get/file.h" #include "command/archive/get/protocol.h" #include "command/command.h" +#include "command/lock.h" #include "common/debug.h" #include "common/log.h" #include "common/memContext.h" @@ -730,7 +731,7 @@ cmdArchiveGet(void) // If the WAL segment has not already been found then start the async process to get it. There's no point in forking // the async process off more than once so track that as well. Use an archive lock to prevent forking if the async // process was launched by another process. - if (!forked && (!found || !queueFull) && lockAcquireP(.returnOnNoLock = true)) + if (!forked && (!found || !queueFull) && cmdLockAcquireP(.returnOnNoLock = true)) { // Get control info const PgControl pgControl = pgControlFromFile(storagePg(), cfgOptionStrNull(cfgOptPgVersionForce)); @@ -761,7 +762,7 @@ cmdArchiveGet(void) archiveAsyncErrorClear(archiveModeGet, walSegment); // Release the lock so the child process can acquire it - lockRelease(true); + cmdLockReleaseP(); // Execute the async process archiveAsyncExec(archiveModeGet, commandExec); diff --git a/src/command/archive/push/push.c b/src/command/archive/push/push.c index 2d1c0885b..d528ca80b 100644 --- a/src/command/archive/push/push.c +++ b/src/command/archive/push/push.c @@ -11,6 +11,7 @@ Archive Push Command #include "command/archive/push/protocol.h" #include "command/command.h" #include "command/control/common.h" +#include "command/lock.h" #include "common/compress/helper.h" #include "common/debug.h" #include "common/log.h" @@ -350,7 +351,7 @@ cmdArchivePush(void) // If the WAL segment has not already been pushed then start the async process to push it. There's no point in // forking the async process off more than once so track that as well. Use an archive lock to prevent more than one // async process being launched. - if (!pushed && !forked && lockAcquireP(.returnOnNoLock = true)) + if (!pushed && !forked && cmdLockAcquireP(.returnOnNoLock = true)) { // The async process should not output on the console at all KeyValue *const optionReplace = kvNew(); @@ -367,7 +368,7 @@ cmdArchivePush(void) archiveAsyncErrorClear(archiveModePush, archiveFile); // Release the lock so the child process can acquire it - lockRelease(true); + cmdLockReleaseP(); // Execute the async process archiveAsyncExec(archiveModePush, commandExec); diff --git a/src/command/backup/backup.c b/src/command/backup/backup.c index f422874c1..97573f3f2 100644 --- a/src/command/backup/backup.c +++ b/src/command/backup/backup.c @@ -15,12 +15,12 @@ Backup Command #include "command/backup/protocol.h" #include "command/check/common.h" #include "command/control/common.h" +#include "command/lock.h" #include "command/stanza/common.h" #include "common/compress/helper.h" #include "common/crypto/cipherBlock.h" #include "common/debug.h" #include "common/io/filter/size.h" -#include "common/lock.h" #include "common/log.h" #include "common/regExp.h" #include "common/time.h" @@ -1604,8 +1604,8 @@ backupJobResult( if (percentComplete - *currentPercentComplete > 10) { *currentPercentComplete = percentComplete; - lockWriteDataP( - lockTypeBackup, .percentComplete = VARUINT(*currentPercentComplete), .sizeComplete = VARUINT64(*sizeProgress), + cmdLockWriteP( + .percentComplete = VARUINT(*currentPercentComplete), .sizeComplete = VARUINT64(*sizeProgress), .size = VARUINT64(sizeTotal)); } } @@ -2218,8 +2218,8 @@ backupProcess(const BackupData *const backupData, Manifest *const manifest, cons // Initialize percent complete and bytes completed/total unsigned int currentPercentComplete = 0; - lockWriteDataP( - lockTypeBackup, .percentComplete = VARUINT(currentPercentComplete), .sizeComplete = VARUINT64(sizeProgress), + cmdLockWriteP( + .percentComplete = VARUINT(currentPercentComplete), .sizeComplete = VARUINT64(sizeProgress), .size = VARUINT64(sizeTotal)); MEM_CONTEXT_TEMP_RESET_BEGIN() diff --git a/src/command/control/stop.c b/src/command/control/stop.c index a30aff17b..b5ecad062 100644 --- a/src/command/control/stop.c +++ b/src/command/control/stop.c @@ -53,20 +53,19 @@ cmdStop(void) // Find each lock file and send term signals to the processes for (unsigned int lockPathFileIdx = 0; lockPathFileIdx < strLstSize(lockPathFileList); lockPathFileIdx++) { - const String *lockFile = strLstGet(lockPathFileList, lockPathFileIdx); + const String *const lockFile = strLstGet(lockPathFileList, lockPathFileIdx); // Skip any file that is not a lock file. Skip lock files for other stanzas if a stanza is provided. if (!strEndsWithZ(lockFile, LOCK_FILE_EXT) || (stanzaPrefix != NULL && !strBeginsWith(lockFile, stanzaPrefix))) continue; // Read the lock file - lockFile = strNewFmt("%s/%s", strZ(lockPath), strZ(lockFile)); - const LockReadResult lockResult = lockReadFileP(lockFile, .remove = true); + const LockReadResult lockResult = lockReadP(lockFile, .remove = true); // If we cannot read the lock file for any reason then warn and continue to next file if (lockResult.status != lockReadStatusValid) { - LOG_WARN_FMT("unable to read lock file %s", strZ(lockFile)); + LOG_WARN_FMT("unable to read lock file %s/%s", strZ(lockPath), strZ(lockFile)); continue; } diff --git a/src/command/exit.c b/src/command/exit.c index 219d27795..a21ea17d7 100644 --- a/src/command/exit.c +++ b/src/command/exit.c @@ -8,8 +8,8 @@ Exit Routines #include "command/command.h" #include "command/exit.h" +#include "command/lock.h" #include "common/debug.h" -#include "common/lock.h" #include "common/log.h" #include "config/config.h" #include "protocol/helper.h" @@ -169,7 +169,7 @@ exitSafe(int result, const bool error, const SignalType signalType) // Release any locks but ignore errors TRY_BEGIN() { - lockRelease(false); + cmdLockReleaseP(.returnOnNoLock = true); } TRY_END(); diff --git a/src/command/info/info.c b/src/command/info/info.c index 20f3bc58e..37ab55726 100644 --- a/src/command/info/info.c +++ b/src/command/info/info.c @@ -10,10 +10,10 @@ Info Command #include "command/archive/common.h" #include "command/info/info.h" +#include "command/lock.h" #include "common/crypto/common.h" #include "common/debug.h" #include "common/io/fdWrite.h" -#include "common/lock.h" #include "common/log.h" #include "common/memContext.h" #include "common/type/json.h" @@ -1299,7 +1299,7 @@ infoUpdateStanza( if (!stanzaRepo->backupLockChecked) { // If there is a valid backup lock for this stanza then backup/expire must be running - const LockReadResult lockResult = lockRead(cfgOptionStr(cfgOptLockPath), stanzaRepo->name, lockTypeBackup); + const LockReadResult lockResult = cmdLockRead(lockTypeBackup, stanzaRepo->name); stanzaRepo->backupLockHeld = lockResult.status == lockReadStatusValid; stanzaRepo->backupLockChecked = true; diff --git a/src/command/lock.c b/src/command/lock.c new file mode 100644 index 000000000..b56a71c41 --- /dev/null +++ b/src/command/lock.c @@ -0,0 +1,149 @@ +/*********************************************************************************************************************************** +Command Lock Handler +***********************************************************************************************************************************/ +#include "build.auto.h" + +#include "command/lock.h" +#include "common/debug.h" +#include "common/log.h" +#include "config/config.h" + +/*********************************************************************************************************************************** +Local variables +***********************************************************************************************************************************/ +static struct CmdLockLocal +{ + bool held; // Is the lock being held? +} cmdLockLocal; + +/*********************************************************************************************************************************** +Lock type names +***********************************************************************************************************************************/ +static const char *const lockTypeName[] = +{ + "archive", // lockTypeArchive + "backup", // lockTypeBackup +}; + +/**********************************************************************************************************************************/ +static String * +cmdLockFileName(const String *const stanza, const LockType lockType) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(STRING, stanza); + FUNCTION_TEST_PARAM(ENUM, lockType); + FUNCTION_TEST_END(); + + ASSERT(stanza != NULL); + ASSERT(lockType < lockTypeAll); + + FUNCTION_TEST_RETURN(STRING, strNewFmt("%s-%s" LOCK_FILE_EXT, strZ(stanza), lockTypeName[lockType])); +} + +/**********************************************************************************************************************************/ +FN_EXTERN bool +cmdLockAcquire(const LockAcquireParam param) +{ + FUNCTION_LOG_BEGIN(logLevelDebug); + FUNCTION_LOG_PARAM(BOOL, param.returnOnNoLock); + FUNCTION_LOG_END(); + + bool result = true; + + // Don't allow return on no lock when locking more than one file. This makes cleanup difficult and there are no known use cases. + const LockType lockType = cfgLockType(); + ASSERT(!param.returnOnNoLock || lockType != lockTypeAll); + + // Don't allow another lock if one is already held + if (cmdLockLocal.held) + THROW(AssertError, "lock is already held by this process"); + + // Lock files + const LockType lockMin = lockType == lockTypeAll ? lockTypeArchive : lockType; + const LockType lockMax = lockType == lockTypeAll ? (lockTypeAll - 1) : lockType; + + for (LockType lockIdx = lockMin; lockIdx <= lockMax; lockIdx++) + { + String *const lockFileName = cmdLockFileName(cfgOptionStr(cfgOptStanza), lockIdx); + + result = lockAcquire(lockFileName, param); + strFree(lockFileName); + } + + // Set lock held flag + cmdLockLocal.held = result; + + FUNCTION_LOG_RETURN(BOOL, result); +} + +/**********************************************************************************************************************************/ +FN_EXTERN void +cmdLockWrite(const LockWriteParam param) +{ + FUNCTION_LOG_BEGIN(logLevelTrace); + FUNCTION_LOG_PARAM(VARIANT, param.percentComplete); + FUNCTION_LOG_PARAM(VARIANT, param.sizeComplete); + FUNCTION_LOG_PARAM(VARIANT, param.size); + FUNCTION_LOG_END(); + + String *const lockFileName = cmdLockFileName(cfgOptionStr(cfgOptStanza), cfgLockType()); + + lockWrite(lockFileName, param); + strFree(lockFileName); + + FUNCTION_LOG_RETURN_VOID(); +} + +/**********************************************************************************************************************************/ +FN_EXTERN LockReadResult +cmdLockRead(const LockType lockType, const String *const stanza) +{ + FUNCTION_LOG_BEGIN(logLevelDebug); + FUNCTION_LOG_PARAM(ENUM, lockType); + FUNCTION_LOG_PARAM(STRING, stanza); + FUNCTION_LOG_END(); + + ASSERT(lockType == lockTypeBackup); + ASSERT(stanza != NULL); + + FUNCTION_AUDIT_STRUCT(); + + LockReadResult result = {0}; + + MEM_CONTEXT_TEMP_BEGIN() + { + const String *const lockFileName = cmdLockFileName(stanza, lockType); + + MEM_CONTEXT_PRIOR_BEGIN() + { + result = lockReadP(lockFileName); + } + MEM_CONTEXT_PRIOR_END(); + } + MEM_CONTEXT_TEMP_END(); + + FUNCTION_LOG_RETURN_STRUCT(result); +} + +/**********************************************************************************************************************************/ +FN_EXTERN bool +cmdLockRelease(const LockReleaseParam param) +{ + FUNCTION_LOG_BEGIN(logLevelTrace); + FUNCTION_LOG_PARAM(BOOL, param.returnOnNoLock); + FUNCTION_LOG_END(); + + bool result = true; + + // Release lock(s) + if (cmdLockLocal.held) + { + result = lockRelease(param); + cmdLockLocal.held = false; + } + // Else error when requested + else if (!param.returnOnNoLock) + THROW(AssertError, "no lock is held by this process"); + + FUNCTION_LOG_RETURN(BOOL, result); +} diff --git a/src/command/lock.h b/src/command/lock.h new file mode 100644 index 000000000..cce998ea0 --- /dev/null +++ b/src/command/lock.h @@ -0,0 +1,35 @@ +/*********************************************************************************************************************************** +Command Lock Handler +***********************************************************************************************************************************/ +#ifndef COMMAND_LOCK_H +#define COMMAND_LOCK_H + +#include "common/lock.h" +#include "config/config.h" + +/*********************************************************************************************************************************** +Functions +***********************************************************************************************************************************/ +// Acquire a command lock. This will involve locking one or more files on disk depending on the lock type. Most operations only +// acquire a single lock type (archive or backup), but the stanza commands all need to lock both. +#define cmdLockAcquireP(...) \ + cmdLockAcquire((LockAcquireParam){VAR_PARAM_INIT, __VA_ARGS__}) + +FN_EXTERN bool cmdLockAcquire(LockAcquireParam param); + +// Write data to command lock file +#define cmdLockWriteP(...) \ + cmdLockWrite((LockWriteParam){VAR_PARAM_INIT, __VA_ARGS__}) + +FN_EXTERN void cmdLockWrite(LockWriteParam param); + +// Read a command lock file held by another process to get information about what the process is doing +FN_EXTERN LockReadResult cmdLockRead(LockType lockType, const String *stanza); + +// Release command lock(s) +#define cmdLockReleaseP(...) \ + cmdLockRelease((LockReleaseParam){VAR_PARAM_INIT, __VA_ARGS__}) + +FN_EXTERN bool cmdLockRelease(LockReleaseParam param); + +#endif diff --git a/src/command/remote/remote.c b/src/command/remote/remote.c index 4a3368f65..738acc293 100644 --- a/src/command/remote/remote.c +++ b/src/command/remote/remote.c @@ -8,6 +8,7 @@ Remote Command #include "command/backup/blockIncr.h" #include "command/backup/pageChecksum.h" #include "command/control/common.h" +#include "command/lock.h" #include "command/restore/blockChecksum.h" #include "common/crypto/cipherBlock.h" #include "common/crypto/hash.h" @@ -77,7 +78,7 @@ cmdRemote(ProtocolServer *const server) lockStopTest(); // Acquire the lock - lockAcquireP(); + cmdLockAcquireP(); } } diff --git a/src/common/lock.c b/src/common/lock.c index 465cb2dbb..cc3d6fb63 100644 --- a/src/common/lock.c +++ b/src/common/lock.c @@ -38,42 +38,32 @@ Constants #define LOCK_KEY_SIZE_COMPLETE STRID6("szCplt", 0x50c4286931) #define LOCK_KEY_SIZE STRID5("sz", 0x3530) -/*********************************************************************************************************************************** -Lock type names -***********************************************************************************************************************************/ -static const char *const lockTypeName[] = -{ - "archive", // lockTypeArchive - "backup", // lockTypeBackup -}; - /*********************************************************************************************************************************** Mem context and local variables ***********************************************************************************************************************************/ +typedef struct LockFile +{ + String *name; // Name of lock file + int fd; // File descriptor for lock file + bool written; // Has the lock file been written? +} LockFile; + static struct LockLocal { MemContext *memContext; // Mem context for locks const String *path; // Lock path const String *execId; // Process exec id - const String *stanza; // Stanza - LockType type; // Lock type - bool held; // Is the lock being held? - - struct - { - String *name; // Name of lock file - int fd; // File descriptor for lock file - } file[lockTypeAll]; + List *lockList; // List of locks held + const Storage *storage; // Storage object for lock path } lockLocal; /**********************************************************************************************************************************/ FN_EXTERN void -lockInit(const String *const path, const String *const execId, const String *const stanza, const LockType type) +lockInit(const String *const path, const String *const execId) { FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STRING, path); FUNCTION_LOG_PARAM(STRING, execId); - FUNCTION_LOG_PARAM(ENUM, type); FUNCTION_LOG_END(); ASSERT(lockLocal.memContext == NULL); @@ -86,8 +76,8 @@ lockInit(const String *const path, const String *const execId, const String *con lockLocal.memContext = MEM_CONTEXT_NEW(); lockLocal.path = strDup(path); lockLocal.execId = strDup(execId); - lockLocal.stanza = strDup(stanza); - lockLocal.type = type; + lockLocal.lockList = lstNewP(sizeof(LockFile), .comparator = lstComparatorStr); + lockLocal.storage = storagePosixNewP(lockLocal.path, .write = true); } MEM_CONTEXT_NEW_END(); } @@ -96,38 +86,24 @@ lockInit(const String *const path, const String *const execId, const String *con FUNCTION_LOG_RETURN_VOID(); } -/**********************************************************************************************************************************/ -FN_EXTERN String * -lockFileName(const String *const stanza, const LockType lockType) -{ - FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM(STRING, stanza); - FUNCTION_TEST_PARAM(ENUM, lockType); - FUNCTION_TEST_END(); - - FUNCTION_TEST_RETURN(STRING, strNewFmt("%s-%s" LOCK_FILE_EXT, strZ(stanza), lockTypeName[lockType])); -} - /*********************************************************************************************************************************** Read contents of lock file - -If a seek is required to get to the beginning of the data, that must be done before calling this function. ***********************************************************************************************************************************/ // Size of initial buffer used to load lock file #define LOCK_BUFFER_SIZE 128 // Helper to read data static LockData -lockReadFileData(const String *const lockFile, const int fd) +lockReadData(const String *const lockFileName, const int fd) { FUNCTION_LOG_BEGIN(logLevelTrace); - FUNCTION_LOG_PARAM(STRING, lockFile); + FUNCTION_LOG_PARAM(STRING, lockFileName); FUNCTION_LOG_PARAM(INT, fd); FUNCTION_LOG_END(); FUNCTION_AUDIT_STRUCT(); - ASSERT(lockFile != NULL); + ASSERT(lockFileName != NULL); ASSERT(fd != -1); LockData result = {0}; @@ -140,7 +116,7 @@ lockReadFileData(const String *const lockFile, const int fd) Buffer *const buffer = bufNew(LOCK_BUFFER_SIZE); IoWrite *const write = ioBufferWriteNewOpen(buffer); - ioCopyP(ioFdReadNewOpen(lockFile, fd, 0), write); + ioCopyP(ioFdReadNewOpen(lockFileName, fd, 0), write); ioWriteClose(write); JsonRead *const json = jsonReadNew(strNewBuf(buffer)); @@ -167,34 +143,165 @@ lockReadFileData(const String *const lockFile, const int fd) } CATCH_ANY() { - THROWP_FMT(errorType(), "unable to read lock file '%s': %s", strZ(lockFile), errorMessage()); + THROWP_FMT(errorType(), "unable to read lock file '%s': %s", strZ(lockFileName), errorMessage()); } TRY_END(); FUNCTION_LOG_RETURN_STRUCT(result); } +/**********************************************************************************************************************************/ +FN_EXTERN bool +lockAcquire(const String *const lockFileName, const LockAcquireParam param) +{ + FUNCTION_LOG_BEGIN(logLevelTrace); + FUNCTION_LOG_PARAM(STRING, lockFileName); + FUNCTION_LOG_PARAM(BOOL, param.returnOnNoLock); + FUNCTION_LOG_END(); + + ASSERT(lockFileName != NULL); + ASSERT(lockLocal.memContext != NULL); + + bool result = false; + + if (lstFind(lockLocal.lockList, &lockFileName) != NULL) + THROW_FMT(AssertError, "lock on file '%s' already held", strZ(lockFileName)); + + MEM_CONTEXT_TEMP_BEGIN() + { + const String *const lockFilePath = storagePathP(lockLocal.storage, lockFileName); + bool retry; + int errNo = 0; + int fd = -1; + + do + { + // Assume there will be no retry + retry = false; + + // Attempt to open the file + if ((fd = open(strZ(lockFilePath), O_RDWR | O_CREAT, STORAGE_MODE_FILE_DEFAULT)) == -1) + { + // Save the error for reporting outside the loop + errNo = errno; + + // If the path does not exist then create it + if (errNo == ENOENT) + { + storagePathCreateP(storagePosixNewP(FSLASH_STR, .write = true), strPath(lockFilePath)); + retry = true; + } + } + else + { + // Attempt to lock the file + if (flock(fd, LOCK_EX | LOCK_NB) == -1) + { + // Save the error for reporting outside the loop + errNo = errno; + + // Get execId from lock file and close it + const String *execId = NULL; + + TRY_BEGIN() + { + execId = lockReadData(lockFileName, fd).execId; + } + CATCH_ANY() + { + // Any errors will be reported as unable to acquire a lock. If a process is trying to get a lock but is not + // synchronized with the process holding the actual lock, it is possible that it could see a short read or + // have some other problem reading. Reporting the error will likely be misleading when the actual problem is + // that another process owns the lock file. + } + FINALLY() + { + close(fd); + } + TRY_END(); + + // Even though we were unable to lock the file, it may be that it is already locked by another process with the + // same exec-id, i.e. spawned by the same original main process. If so, report the lock as successful. + if (strEq(execId, lockLocal.execId)) + fd = LOCK_ON_EXEC_ID; + else + fd = -1; + } + } + } + while (fd == -1 && retry); + + // If the lock was not successful + if (fd == -1) + { + // Error when requested + if (!param.returnOnNoLock || errNo != EWOULDBLOCK) + { + const String *errorHint = NULL; + + if (errNo == EWOULDBLOCK) + errorHint = strNewZ("\nHINT: is another " PROJECT_NAME " process running?"); + else if (errNo == EACCES) + { + // Get information for the current user + userInit(); + + errorHint = strNewFmt( + "\nHINT: does '%s:%s' running " PROJECT_NAME " have permissions on the '%s' file?", strZ(userName()), + strZ(groupName()), strZ(lockFilePath)); + } + + THROW_FMT( + LockAcquireError, "unable to acquire lock on file '%s': %s%s", strZ(lockFilePath), strerror(errNo), + errorHint == NULL ? "" : strZ(errorHint)); + } + } + // Else add lock to list + else + { + MEM_CONTEXT_OBJ_BEGIN(lockLocal.lockList) + { + lstAdd(lockLocal.lockList, &(LockFile){.name = strDup(lockFileName), .fd = fd}); + } + MEM_CONTEXT_OBJ_END(); + + // Write lock data unless lock was acquired by matching execId + if (fd != LOCK_ON_EXEC_ID) + lockWriteP(lockFileName); + + // Success + result = true; + } + } + MEM_CONTEXT_TEMP_END(); + + FUNCTION_LOG_RETURN(BOOL, result); +} + /**********************************************************************************************************************************/ FN_EXTERN LockReadResult -lockReadFile(const String *const lockFile, const LockReadFileParam param) +lockRead(const String *const lockFileName, const LockReadParam param) { FUNCTION_LOG_BEGIN(logLevelDebug); - FUNCTION_LOG_PARAM(STRING, lockFile); + FUNCTION_LOG_PARAM(STRING, lockFileName); FUNCTION_LOG_PARAM(BOOL, param.remove); FUNCTION_LOG_END(); FUNCTION_AUDIT_STRUCT(); - ASSERT(lockFile != NULL); + ASSERT(lockLocal.memContext != NULL); + ASSERT(lockFileName != NULL); LockReadResult result = {.status = lockReadStatusValid}; MEM_CONTEXT_TEMP_BEGIN() { - // If we cannot open the lock file for any reason then warn and continue to next file + const String *const lockFilePath = storagePathP(lockLocal.storage, lockFileName); + + // If we cannot open the lock file for any reason then warn int fd = -1; - if ((fd = open(strZ(lockFile), O_RDONLY, 0)) == -1) + if ((fd = open(strZ(lockFilePath), O_RDONLY, 0)) == -1) { result.status = lockReadStatusMissing; } @@ -212,7 +319,7 @@ lockReadFile(const String *const lockFile, const LockReadFileParam param) { MEM_CONTEXT_PRIOR_BEGIN() { - result.data = lockReadFileData(lockFile, fd); + result.data = lockReadData(lockFilePath, fd); } MEM_CONTEXT_PRIOR_END(); } @@ -225,7 +332,7 @@ lockReadFile(const String *const lockFile, const LockReadFileParam param) // Remove lock file if requested but do not report failures if (param.remove) - unlink(strZ(lockFile)); + unlink(strZ(lockFilePath)); // Close after unlinking to prevent a race condition where another process creates the file as we remove it close(fd); @@ -237,50 +344,17 @@ lockReadFile(const String *const lockFile, const LockReadFileParam param) } /**********************************************************************************************************************************/ -FN_EXTERN LockReadResult -lockRead(const String *const lockPath, const String *const stanza, const LockType lockType) -{ - FUNCTION_LOG_BEGIN(logLevelDebug); - FUNCTION_LOG_PARAM(STRING, lockPath); - FUNCTION_LOG_PARAM(STRING, stanza); - FUNCTION_LOG_PARAM(ENUM, lockType); - FUNCTION_LOG_END(); - - FUNCTION_AUDIT_STRUCT(); - - LockReadResult result = {0}; - - MEM_CONTEXT_TEMP_BEGIN() - { - const String *const lockFile = strNewFmt("%s/%s", strZ(lockPath), strZ(lockFileName(stanza, lockType))); - - MEM_CONTEXT_PRIOR_BEGIN() - { - result = lockReadFileP(lockFile); - } - MEM_CONTEXT_PRIOR_END(); - } - MEM_CONTEXT_TEMP_END(); - - FUNCTION_LOG_RETURN_STRUCT(result); -} - -/*********************************************************************************************************************************** -Write contents of lock file -***********************************************************************************************************************************/ FN_EXTERN void -lockWriteData(const LockType lockType, const LockWriteDataParam param) +lockWrite(const String *const lockFileName, const LockWriteParam param) { FUNCTION_LOG_BEGIN(logLevelTrace); - FUNCTION_LOG_PARAM(ENUM, lockType); + FUNCTION_LOG_PARAM(STRING, lockFileName); FUNCTION_LOG_PARAM(VARIANT, param.percentComplete); FUNCTION_LOG_PARAM(VARIANT, param.sizeComplete); FUNCTION_LOG_PARAM(VARIANT, param.size); FUNCTION_LOG_END(); - ASSERT(lockType < lockTypeAll); - ASSERT(lockLocal.file[lockType].name != NULL); - ASSERT(lockLocal.file[lockType].fd != -1); + ASSERT(lockFileName != NULL); MEM_CONTEXT_TEMP_BEGIN() { @@ -303,24 +377,31 @@ lockWriteData(const LockType lockType, const LockWriteDataParam param) jsonWriteObjectEnd(json); - if (lockType == lockTypeBackup && lockLocal.held) - { - // Seek to beginning of backup lock file - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockType].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockType].name)); + // Write to lock file + LockFile *const lockFile = lstFind(lockLocal.lockList, &lockFileName); - // In case the current write is ever shorter than the previous one + if (lockFile == NULL) + THROW_FMT(AssertError, "lock file '%s' not found", strZ(lockFileName)); + + const String *const lockFilePath = storagePathP(lockLocal.storage, lockFileName); + + if (lockFile->written) + { + // Seek to beginning of lock file THROW_ON_SYS_ERROR_FMT( - ftruncate(lockLocal.file[lockType].fd, 0) == -1, FileWriteError, "unable to truncate '%s'", - strZ(lockLocal.file[lockType].name)); + lseek(lockFile->fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, strZ(lockFilePath)); + + // In case the current write is shorter than before + THROW_ON_SYS_ERROR_FMT(ftruncate(lockFile->fd, 0) == -1, FileWriteError, "unable to truncate '%s'", strZ(lockFilePath)); } // Write lock file data - IoWrite *const write = ioFdWriteNewOpen(lockLocal.file[lockType].name, lockLocal.file[lockType].fd, 0); + IoWrite *const write = ioFdWriteNewOpen(lockFilePath, lockFile->fd, 0); ioCopyP(ioBufferReadNewOpen(BUFSTR(jsonWriteResult(json))), write); ioWriteClose(write); + + lockFile->written = true; } MEM_CONTEXT_TEMP_END(); @@ -328,219 +409,44 @@ lockWriteData(const LockType lockType, const LockWriteDataParam param) } /**********************************************************************************************************************************/ -// Helper to acquire a file lock -static int -lockAcquireFile(const String *const lockFile, const TimeMSec lockTimeout, const bool failOnNoLock) +FN_EXTERN bool +lockRelease(const LockReleaseParam param) { FUNCTION_LOG_BEGIN(logLevelTrace); - FUNCTION_LOG_PARAM(STRING, lockFile); - FUNCTION_LOG_PARAM(TIMEMSEC, lockTimeout); - FUNCTION_LOG_PARAM(BOOL, failOnNoLock); - FUNCTION_LOG_END(); - - ASSERT(lockFile != NULL); - - int result = -1; - - MEM_CONTEXT_TEMP_BEGIN() - { - Wait *const wait = waitNew(lockTimeout); - bool retry; - int errNo = 0; - - do - { - // Assume there will be no retry - retry = false; - - // Attempt to open the file - if ((result = open(strZ(lockFile), O_RDWR | O_CREAT, STORAGE_MODE_FILE_DEFAULT)) == -1) - { - // Save the error for reporting outside the loop - errNo = errno; - - // If the path does not exist then create it - if (errNo == ENOENT) - { - storagePathCreateP(storagePosixNewP(FSLASH_STR, .write = true), strPath(lockFile)); - retry = true; - } - } - else - { - // Attempt to lock the file - if (flock(result, LOCK_EX | LOCK_NB) == -1) - { - // Save the error for reporting outside the loop - errNo = errno; - - // Get execId from lock file and close it - const String *execId = NULL; - - TRY_BEGIN() - { - execId = lockReadFileData(lockFile, result).execId; - } - CATCH_ANY() - { - // Any errors will be reported as unable to acquire a lock. If a process is trying to get a lock but is not - // synchronized with the process holding the actual lock, it is possible that it could see a short read or - // have some other problem reading. Reporting the error will likely be misleading when the actual problem is - // that another process owns the lock file. - } - FINALLY() - { - close(result); - } - TRY_END(); - - // Even though we were unable to lock the file, it may be that it is already locked by another process with the - // same exec-id, i.e. spawned by the same original main process. If so, report the lock as successful. - if (strEq(execId, lockLocal.execId)) - result = LOCK_ON_EXEC_ID; - else - result = -1; - } - } - } - while (result == -1 && (waitMore(wait) || retry)); - - // If the lock was not successful - if (result == -1) - { - // Error when requested - if (failOnNoLock || errNo != EWOULDBLOCK) - { - const String *errorHint = NULL; - - if (errNo == EWOULDBLOCK) - errorHint = strNewZ("\nHINT: is another " PROJECT_NAME " process running?"); - else if (errNo == EACCES) - { - // Get information for the current user - userInit(); - - errorHint = strNewFmt( - "\nHINT: does '%s:%s' running " PROJECT_NAME " have permissions on the '%s' file?", strZ(userName()), - strZ(groupName()), strZ(lockFile)); - } - - THROW_FMT( - LockAcquireError, "unable to acquire lock on file '%s': %s%s", strZ(lockFile), strerror(errNo), - errorHint == NULL ? "" : strZ(errorHint)); - } - } - } - MEM_CONTEXT_TEMP_END(); - - FUNCTION_LOG_RETURN(INT, result); -} - -FN_EXTERN bool -lockAcquire(const LockAcquireParam param) -{ - FUNCTION_LOG_BEGIN(logLevelDebug); - FUNCTION_LOG_PARAM(TIMEMSEC, param.timeout); FUNCTION_LOG_PARAM(BOOL, param.returnOnNoLock); FUNCTION_LOG_END(); ASSERT(lockLocal.memContext != NULL); - bool result = true; - - // Don't allow failures when locking more than one file. This makes cleanup difficult and there are no known use cases. - ASSERT(!param.returnOnNoLock || lockLocal.type != lockTypeAll); - - // Don't allow another lock if one is already held - if (lockLocal.held) - THROW(AssertError, "lock is already held by this process"); - - // Lock files - const LockType lockMin = lockLocal.type == lockTypeAll ? lockTypeArchive : lockLocal.type; - const LockType lockMax = lockLocal.type == lockTypeAll ? (lockTypeAll - 1) : lockLocal.type; - - for (LockType lockIdx = lockMin; lockIdx <= lockMax; lockIdx++) - { - MEM_CONTEXT_BEGIN(lockLocal.memContext) - { - strFree(lockLocal.file[lockIdx].name); - lockLocal.file[lockIdx].name = strNewFmt("%s/%s", strZ(lockLocal.path), strZ(lockFileName(lockLocal.stanza, lockIdx))); - } - MEM_CONTEXT_END(); - - lockLocal.file[lockIdx].fd = lockAcquireFile(lockLocal.file[lockIdx].name, param.timeout, !param.returnOnNoLock); - - if (lockLocal.file[lockIdx].fd == -1) - { - // Free the lock - lockLocal.held = false; - result = false; - break; - } - // Else write lock data unless we locked an execId match - else if (lockLocal.file[lockIdx].fd != LOCK_ON_EXEC_ID) - lockWriteDataP(lockIdx); - } - - if (result) - lockLocal.held = true; - - FUNCTION_LOG_RETURN(BOOL, result); -} - -/**********************************************************************************************************************************/ -// Helper to release a file lock -static void -lockReleaseFile(const int lockFd, const String *const lockFile) -{ - FUNCTION_LOG_BEGIN(logLevelTrace); - FUNCTION_LOG_PARAM(INT, lockFd); - FUNCTION_LOG_PARAM(STRING, lockFile); - FUNCTION_LOG_END(); - - // Can't release lock if there isn't one - ASSERT(lockFd >= 0); - - MEM_CONTEXT_TEMP_BEGIN() - { - // Remove file first and then close it to release the lock. If we close it first then another process might grab the lock - // right before the delete which means the file locked by the other process will get deleted. - storageRemoveP(storagePosixNewP(FSLASH_STR, .write = true), lockFile); - close(lockFd); - } - MEM_CONTEXT_TEMP_END(); - - FUNCTION_LOG_RETURN_VOID(); -} - -FN_EXTERN bool -lockRelease(bool failOnNoLock) -{ - FUNCTION_LOG_BEGIN(logLevelTrace); - FUNCTION_LOG_PARAM(BOOL, failOnNoLock); - FUNCTION_LOG_END(); - bool result = false; - if (!lockLocal.held) + // Nothing to do if lock list is empty + if (lstEmpty(lockLocal.lockList)) { - if (failOnNoLock) + // Fail if requested + if (!param.returnOnNoLock) THROW(AssertError, "no lock is held by this process"); } + // Else release all locks else { - // Release locks - const LockType lockMin = lockLocal.type == lockTypeAll ? lockTypeArchive : lockLocal.type; - const LockType lockMax = lockLocal.type == lockTypeAll ? (lockTypeAll - 1) : lockLocal.type; - - for (LockType lockIdx = lockMin; lockIdx <= lockMax; lockIdx++) + // Release until list is empty + while (!lstEmpty(lockLocal.lockList)) { - if (lockLocal.file[lockIdx].fd != LOCK_ON_EXEC_ID) - lockReleaseFile(lockLocal.file[lockIdx].fd, lockLocal.file[lockIdx].name); + LockFile *const lockFile = lstGet(lockLocal.lockList, 0); + + // Remove lock file if this lock was not acquired by matching execId + if (lockFile->fd != LOCK_ON_EXEC_ID) + { + storageRemoveP(lockLocal.storage, lockFile->name); + close(lockFile->fd); + } + + strFree(lockFile->name); + lstRemoveIdx(lockLocal.lockList, 0); } - // Free the lock context - lockLocal.held = false; + // Success result = true; } diff --git a/src/common/lock.h b/src/common/lock.h index 48df7a409..b205fac13 100644 --- a/src/common/lock.h +++ b/src/common/lock.h @@ -7,16 +7,8 @@ Lock Handler #include /*********************************************************************************************************************************** -Lock types +Lock data ***********************************************************************************************************************************/ -typedef enum -{ - lockTypeArchive, - lockTypeBackup, - lockTypeAll, - lockTypeNone, -} LockType; - #include "common/type/variant.h" // Lock data @@ -40,44 +32,35 @@ Constants Functions ***********************************************************************************************************************************/ // Initialize lock module -FN_EXTERN void lockInit(const String *path, const String *execId, const String *stanza, LockType type); +FN_EXTERN void lockInit(const String *path, const String *execId); -// Acquire a lock type. This will involve locking one or more files on disk depending on the lock type. Most operations only take a -// single lock (archive or backup), but the stanza commands all need to lock both. +// Acquire a lock typedef struct LockAcquireParam { VAR_PARAM_HEADER; - TimeMSec timeout; // Lock timeout bool returnOnNoLock; // Return when no lock acquired (rather than throw an error) } LockAcquireParam; -#define lockAcquireP(...) \ - lockAcquire((LockAcquireParam) {VAR_PARAM_INIT, __VA_ARGS__}) +#define lockAcquireP(lockFileName, ...) \ + lockAcquire(lockFileName, (LockAcquireParam){VAR_PARAM_INIT, __VA_ARGS__}) -FN_EXTERN bool lockAcquire(LockAcquireParam param); - -// Release a lock -FN_EXTERN bool lockRelease(bool failOnNoLock); - -// Build lock file name -FN_EXTERN String *lockFileName(const String *stanza, LockType lockType); +FN_EXTERN bool lockAcquire(const String *lockFileName, LockAcquireParam param); // Write data to a lock file -typedef struct LockWriteDataParam +typedef struct LockWriteParam { VAR_PARAM_HEADER; const Variant *percentComplete; // Percentage of backup complete * 100 (when not NULL) const Variant *sizeComplete; // Completed size of the backup in bytes const Variant *size; // Total size of the backup in bytes -} LockWriteDataParam; +} LockWriteParam; -#define lockWriteDataP(lockType, ...) \ - lockWriteData(lockType, (LockWriteDataParam) {VAR_PARAM_INIT, __VA_ARGS__}) +#define lockWriteP(lockFileName, ...) \ + lockWrite(lockFileName, (LockWriteParam){VAR_PARAM_INIT, __VA_ARGS__}) -FN_EXTERN void lockWriteData(LockType lockType, LockWriteDataParam param); +FN_EXTERN void lockWrite(const String *lockFileName, LockWriteParam param); -// Read a lock file held by another process to get information about what the process is doing. This is a lower-level version to use -// when the lock file name is already known and the lock file may need to be removed. +// Read a lock file held by another process to get information about what the process is doing typedef enum { lockReadStatusMissing, // File is missing @@ -92,18 +75,27 @@ typedef struct LockReadResult LockData data; // Lock data } LockReadResult; -typedef struct LockReadFileParam +typedef struct LockReadParam { VAR_PARAM_HEADER; bool remove; // Remove the lock file after locking/reading -} LockReadFileParam; +} LockReadParam; -#define lockReadFileP(lockFile, ...) \ - lockReadFile(lockFile, (LockReadFileParam){VAR_PARAM_INIT, __VA_ARGS__}) +#define lockReadP(lockFileName, ...) \ + lockRead(lockFileName, (LockReadParam){VAR_PARAM_INIT, __VA_ARGS__}) -FN_EXTERN LockReadResult lockReadFile(const String *lockFile, LockReadFileParam param); +FN_EXTERN LockReadResult lockRead(const String *lockFileName, LockReadParam param); -// Wrapper that generates the lock filename before calling lockReadFile() -FN_EXTERN LockReadResult lockRead(const String *lockPath, const String *stanza, LockType lockType); +// Release lock(s) +typedef struct LockReleaseParam +{ + VAR_PARAM_HEADER; + bool returnOnNoLock; // Return when no lock is held (rather than throw an error) +} LockReleaseParam; + +#define lockReleaseP(...) \ + lockRelease((LockReleaseParam){VAR_PARAM_INIT, __VA_ARGS__}) + +FN_EXTERN bool lockRelease(LockReleaseParam param); #endif diff --git a/src/config/config.h b/src/config/config.h index 76c14b4a3..a623a85f0 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -7,7 +7,6 @@ sets the command and options and determines which options are valid for a comman #ifndef CONFIG_CONFIG_H #define CONFIG_CONFIG_H -#include "common/lock.h" #include "common/log.h" #include "common/type/stringId.h" #include "common/type/stringList.h" @@ -42,6 +41,17 @@ typedef enum #define CFG_COMMAND_ROLE_TOTAL 4 +/*********************************************************************************************************************************** +Lock types +***********************************************************************************************************************************/ +typedef enum +{ + lockTypeArchive, + lockTypeBackup, + lockTypeAll, + lockTypeNone, +} LockType; + /*********************************************************************************************************************************** Command Functions diff --git a/src/config/load.c b/src/config/load.c index 5bb3a42cc..1a12e4ca9 100644 --- a/src/config/load.c +++ b/src/config/load.c @@ -8,12 +8,12 @@ Configuration Load #include #include "command/command.h" +#include "command/lock.h" #include "common/compress/helper.intern.h" #include "common/crypto/common.h" #include "common/debug.h" #include "common/io/io.h" #include "common/io/socket/common.h" -#include "common/lock.h" #include "common/log.h" #include "common/memContext.h" #include "config/config.intern.h" @@ -566,15 +566,13 @@ cfgLoad(const unsigned int argListSize, const char *argList[]) // Begin the command cmdBegin(); - // Init lock module if this command can lock - if (cfgLockType() != lockTypeNone && !cfgCommandHelp()) - { - lockInit(cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptExecId), cfgOptionStr(cfgOptStanza), cfgLockType()); + // Initialize the lock module + if (cfgOptionTest(cfgOptLockPath)) + lockInit(cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptExecId)); - // Acquire a lock if this command requires a lock - if (cfgLockRequired()) - lockAcquireP(); - } + // Acquire a lock if this command requires a lock + if (cfgLockType() != lockTypeNone && !cfgCommandHelp() && cfgLockRequired()) + cmdLockAcquireP(); // Update options that have complex rules cfgLoadUpdateOption(); diff --git a/src/main.c b/src/main.c index 4ba534d2d..47b99a78a 100644 --- a/src/main.c +++ b/src/main.c @@ -20,6 +20,7 @@ Main #include "command/help/help.h" #include "command/info/info.h" #include "command/local/local.h" +#include "command/lock.h" #include "command/manifest/manifest.h" #include "command/remote/remote.h" #include "command/repo/create.h" @@ -180,7 +181,7 @@ main(int argListSize, const char *argList[]) cmdBegin(); // Null out any backup percent complete value in the backup lock file - lockWriteDataP(lockTypeBackup); + cmdLockWriteP(); // Run expire cmdExpire(); diff --git a/src/meson.build b/src/meson.build index e65626ded..81e8ff9fe 100644 --- a/src/meson.build +++ b/src/meson.build @@ -146,6 +146,7 @@ src_pgbackrest = [ 'command/control/start.c', 'command/control/stop.c', 'command/local/local.c', + 'command/lock.c', 'command/manifest/manifest.c', 'command/repo/common.c', 'command/repo/create.c', diff --git a/test/define.yaml b/test/define.yaml index ad7e488e5..75fbb9199 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -425,7 +425,7 @@ unit: # ---------------------------------------------------------------------------------------------------------------------------- - name: lock - total: 3 + total: 1 harness: name: config shim: @@ -441,7 +441,7 @@ unit: - common/lock depend: - - common/lock + - command/lock - config/common - config/config - config/parse @@ -789,6 +789,14 @@ unit: - name: command test: + + # ---------------------------------------------------------------------------------------------------------------------------- + - name: lock + total: 1 + + coverage: + - command/lock + # ---------------------------------------------------------------------------------------------------------------------------- - name: control total: 4 @@ -798,6 +806,9 @@ unit: - command/control/start - command/control/stop + include: + - command/lock + # ---------------------------------------------------------------------------------------------------------------------------- - name: annotate total: 1 @@ -892,6 +903,9 @@ unit: coverage: - command/info/info + include: + - command/lock + # ---------------------------------------------------------------------------------------------------------------------------- - name: backup total: 12 diff --git a/test/src/common/harnessBackup.c b/test/src/common/harnessBackup.c index 34bef9439..4bfbe8cc6 100644 --- a/test/src/common/harnessBackup.c +++ b/test/src/common/harnessBackup.c @@ -9,7 +9,6 @@ Harness for Creating Test Backups #include "common/compress/helper.h" #include "common/crypto/common.h" #include "common/crypto/hash.h" -#include "common/lock.h" #include "config/config.h" #include "info/infoArchive.h" #include "info/manifest.h" @@ -153,8 +152,8 @@ hrnCmdBackup(void) { FUNCTION_HARNESS_VOID(); - lockInit(STR(testPath()), cfgOptionStr(cfgOptExecId), cfgOptionStr(cfgOptStanza), lockTypeBackup); - lockAcquireP(); + lockInit(STR(testPath()), cfgOptionStr(cfgOptExecId)); + cmdLockAcquireP(); TRY_BEGIN() { @@ -162,7 +161,7 @@ hrnCmdBackup(void) } FINALLY() { - lockRelease(true); + cmdLockReleaseP(); } TRY_END(); diff --git a/test/src/common/harnessConfig.c b/test/src/common/harnessConfig.c index 1031eccdd..5c1bbde43 100644 --- a/test/src/common/harnessConfig.c +++ b/test/src/common/harnessConfig.c @@ -108,8 +108,8 @@ hrnCfgLoad(ConfigCommand commandId, const StringList *argListParam, const HrnCfg if (cfgOptionValid(cfgOptExecId) && !cfgOptionTest(cfgOptExecId)) cfgOptionSet(cfgOptExecId, cfgSourceParam, VARSTRDEF("1-test")); - if (cfgOptionTest(cfgOptExecId) && cfgOptionTest(cfgOptLockPath) && cfgOptionTest(cfgOptStanza)) - lockInit(cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptExecId), cfgOptionStr(cfgOptStanza), cfgLockType()); + if (cfgOptionTest(cfgOptExecId) && cfgOptionTest(cfgOptLockPath)) + lockInit(cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptExecId)); else hrnLockUnInit(); diff --git a/test/src/common/harnessLock.c b/test/src/common/harnessLock.c index 5b9873c54..0f6d271ca 100644 --- a/test/src/common/harnessLock.c +++ b/test/src/common/harnessLock.c @@ -13,17 +13,15 @@ Include shimmed C modules /**********************************************************************************************************************************/ FN_EXTERN void -lockInit(const String *const path, const String *const execId, const String *const stanza, const LockType type) +lockInit(const String *const path, const String *const execId) { FUNCTION_HARNESS_BEGIN(); FUNCTION_HARNESS_PARAM(STRING, path); FUNCTION_HARNESS_PARAM(STRING, execId); - FUNCTION_HARNESS_PARAM(STRING, stanza); - FUNCTION_HARNESS_PARAM(ENUM, type); FUNCTION_HARNESS_END(); hrnLockUnInit(); - lockInit_SHIMMED(path, execId, stanza, type); + lockInit_SHIMMED(path, execId); FUNCTION_HARNESS_RETURN_VOID(); } diff --git a/test/src/meson.build b/test/src/meson.build index 9926af9ae..83be9e68d 100644 --- a/test/src/meson.build +++ b/test/src/meson.build @@ -27,6 +27,7 @@ src_test = [ '../../src/command/command.c', '../../src/command/exit.c', '../../src/command/help/help.c', + '../../src/command/lock.c', '../../src/common/compress/bz2/common.c', '../../src/common/compress/bz2/decompress.c', '../../src/common/fork.c', diff --git a/test/src/module/command/archiveGetTest.c b/test/src/module/command/archiveGetTest.c index 8c525e4bf..620fc1f5a 100644 --- a/test/src/module/command/archiveGetTest.c +++ b/test/src/module/command/archiveGetTest.c @@ -742,8 +742,8 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-dededede"), cfgOptionStr(cfgOptStanza), cfgLockType()); - TEST_RESULT_VOID(lockAcquireP(.timeout = 30000, .returnOnNoLock = true), "acquire lock"); + lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-dededede")); + TEST_RESULT_VOID(cmdLockAcquireP(.returnOnNoLock = true), "acquire lock"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -751,7 +751,7 @@ testRun(void) // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + cmdLockReleaseP(); } HRN_FORK_CHILD_END(); diff --git a/test/src/module/command/archivePushTest.c b/test/src/module/command/archivePushTest.c index 8eae4fc3b..a2c772270 100644 --- a/test/src/module/command/archivePushTest.c +++ b/test/src/module/command/archivePushTest.c @@ -752,8 +752,8 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("555-fefefefe"), cfgOptionStr(cfgOptStanza), cfgLockType()); - lockAcquireP(.timeout = 30000, .returnOnNoLock = true); + lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("555-fefefefe")); + cmdLockAcquireP(.returnOnNoLock = true); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -761,7 +761,7 @@ testRun(void) // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + cmdLockReleaseP(); } HRN_FORK_CHILD_END(); diff --git a/test/src/module/command/backupTest.c b/test/src/module/command/backupTest.c index a84678505..28f7e9cce 100644 --- a/test/src/module/command/backupTest.c +++ b/test/src/module/command/backupTest.c @@ -1862,15 +1862,15 @@ testRun(void) uint64_t sizeProgress = 0; currentPercentComplete = 4567; - lockInit(TEST_PATH_STR, cfgOptionStr(cfgOptExecId), cfgOptionStr(cfgOptStanza), lockTypeBackup); - TEST_RESULT_VOID(lockAcquireP(), "acquire backup lock"); + lockInit(TEST_PATH_STR, cfgOptionStr(cfgOptExecId)); + TEST_RESULT_VOID(cmdLockAcquireP(), "acquire backup lock"); TEST_RESULT_VOID( backupJobResult( manifest, STRDEF("host"), storageTest, strLstNew(), job, false, pgPageSize8, 0, &sizeProgress, ¤tPercentComplete), "log noop result"); - TEST_RESULT_VOID(lockRelease(true), "release backup lock"); + TEST_RESULT_VOID(cmdLockReleaseP(), "release backup lock"); TEST_RESULT_LOG("P00 DETAIL: match file from prior backup host:" TEST_PATH "/test (0B, 100.00%)"); } diff --git a/test/src/module/command/controlTest.c b/test/src/module/command/controlTest.c index 445b4825e..106f80334 100644 --- a/test/src/module/command/controlTest.c +++ b/test/src/module/command/controlTest.c @@ -1,13 +1,15 @@ /*********************************************************************************************************************************** Test Command Control ***********************************************************************************************************************************/ -#include "common/harnessConfig.h" -#include "common/harnessFork.h" -#include "common/harnessStorage.h" +#include "command/lock.h" #include "common/io/fdRead.h" #include "common/io/fdWrite.h" #include "storage/posix/storage.h" +#include "common/harnessConfig.h" +#include "common/harnessFork.h" +#include "common/harnessStorage.h" + /*********************************************************************************************************************************** Test Run ***********************************************************************************************************************************/ @@ -253,8 +255,9 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(STRDEF(HRN_PATH "/lock"), cfgOptionStr(cfgOptExecId), cfgOptionStr(cfgOptStanza), lockTypeArchive); - TEST_RESULT_BOOL(lockAcquireP(.timeout = 30000), true, "child process acquires lock"); + String *lockFileName = cmdLockFileName(cfgOptionStr(cfgOptStanza), lockTypeArchive); + lockInit(STRDEF(HRN_PATH "/lock"), cfgOptionStr(cfgOptExecId)); + TEST_RESULT_BOOL(lockAcquireP(lockFileName), true, "create archive lock"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -339,6 +342,7 @@ testRun(void) HRN_STORAGE_PUT_EMPTY(hrnStorage, "lock/db-archive" LOCK_FILE_EXT, .comment = "create empty archive lock file for stanza"); HRN_STORAGE_PUT_Z(hrnStorage, "lock/db1-backup" LOCK_FILE_EXT, " ", .comment = "create non-empty lock file other stanza"); + lockInit(STRDEF(HRN_PATH "/lock"), STRDEF("1")); TEST_RESULT_VOID(cmdStop(), "no stanza, create stop file, ignore non lock file"); TEST_STORAGE_EXISTS(hrnStorage, "lock/all" STOP_FILE_EXT, .comment = "stanza stop file created"); TEST_STORAGE_LIST( diff --git a/test/src/module/command/infoTest.c b/test/src/module/command/infoTest.c index 826c967f5..326bead13 100644 --- a/test/src/module/command/infoTest.c +++ b/test/src/module/command/infoTest.c @@ -236,8 +236,9 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-ffffffff"), STRDEF("stanza1"), lockTypeBackup); - TEST_RESULT_INT_NE(lockAcquireP(), -1, "create backup/expire lock"); + lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-ffffffff")); + TEST_RESULT_BOOL( + lockAcquireP(cmdLockFileName(STRDEF("stanza1"), lockTypeBackup)), true, "create backup/expire lock"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -245,7 +246,7 @@ testRun(void) // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + lockReleaseP(); } HRN_FORK_CHILD_END(); @@ -428,9 +429,10 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("777-afafafaf"), STRDEF("stanza1"), lockTypeBackup); - TEST_RESULT_INT_NE(lockAcquireP(), -1, "create backup/expire lock"); - TEST_RESULT_VOID(lockWriteDataP(lockTypeBackup), "write lock data"); + String *lockFileName = cmdLockFileName(STRDEF("stanza1"), lockTypeBackup); + lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("777-afafafaf")); + TEST_RESULT_BOOL(lockAcquireP(lockFileName), true, "create backup/expire lock"); + TEST_RESULT_VOID(lockWriteP(lockFileName), "write lock data"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -438,7 +440,7 @@ testRun(void) // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + lockReleaseP(); } HRN_FORK_CHILD_END(); @@ -1037,11 +1039,12 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-ffffffff"), STRDEF("stanza2"), lockTypeBackup); - TEST_RESULT_INT_NE(lockAcquireP(), -1, "create backup/expire lock"); + String *lockFileName = cmdLockFileName(STRDEF("stanza2"), lockTypeBackup); + lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-ffffffff")); + TEST_RESULT_BOOL(lockAcquireP(lockFileName), true, "create backup/expire lock"); TEST_RESULT_VOID( - lockWriteDataP( - lockTypeBackup, .percentComplete = VARUINT(4545), .sizeComplete = VARUINT64(1435765), + lockWriteP( + lockFileName, .percentComplete = VARUINT(4545), .sizeComplete = VARUINT64(1435765), .size = VARUINT64(3159000)), "write lock data"); @@ -1051,7 +1054,7 @@ testRun(void) // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + lockReleaseP(); } HRN_FORK_CHILD_END(); @@ -1480,9 +1483,10 @@ testRun(void) { HRN_FORK_CHILD_BEGIN() { - lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-ffffffff"), STRDEF("stanza2"), lockTypeBackup); - TEST_RESULT_INT_NE(lockAcquireP(), -1, "create backup/expire lock"); - TEST_RESULT_VOID(lockWriteDataP(lockTypeBackup, .percentComplete = VARUINT(5555)), "write lock data"); + String *lockFileName = cmdLockFileName(STRDEF("stanza2"), lockTypeBackup); + lockInit(cfgOptionStr(cfgOptLockPath), STRDEF("999-ffffffff")); + TEST_RESULT_BOOL(lockAcquireP(lockFileName), true, "create backup/expire lock"); + TEST_RESULT_VOID(lockWriteP(lockFileName, .percentComplete = VARUINT(5555)), "write lock data"); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -1490,7 +1494,7 @@ testRun(void) // Wait for parent to allow release lock HRN_FORK_CHILD_NOTIFY_GET(); - lockRelease(true); + lockReleaseP(); } HRN_FORK_CHILD_END(); diff --git a/test/src/module/command/lockTest.c b/test/src/module/command/lockTest.c new file mode 100644 index 000000000..af358a129 --- /dev/null +++ b/test/src/module/command/lockTest.c @@ -0,0 +1,83 @@ +/*********************************************************************************************************************************** +Test Command Lock Handler +***********************************************************************************************************************************/ +#include "storage/posix/storage.h" + +#include "common/harnessConfig.h" +#include "common/harnessStorage.h" + +/*********************************************************************************************************************************** +Test Run +***********************************************************************************************************************************/ +static void +testRun(void) +{ + FUNCTION_HARNESS_VOID(); + + // Create default storage object for testing + Storage *storageTest = storagePosixNewP(TEST_PATH_STR, .write = true); + + // ***************************************************************************************************************************** + if (testBegin("cmdLock*()")) + { + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("acquire backup command lock"); + + StringList *argList = strLstNew(); + hrnCfgArgRawZ(argList, cfgOptStanza, "test"); + hrnCfgArgRawZ(argList, cfgOptPgPath, "/pg1"); + hrnCfgArgRawZ(argList, cfgOptLockPath, TEST_PATH); + hrnCfgArgRawZ(argList, cfgOptRepoRetentionFull, "1"); + HRN_CFG_LOAD(cfgCmdBackup, argList, .noStd = true); + + TEST_RESULT_VOID(cmdLockAcquireP(), "acquire backup lock"); + TEST_ERROR(cmdLockAcquireP(), AssertError, "lock is already held by this process"); + TEST_STORAGE_LIST(storageTest, NULL, "test-backup.lock\n", .comment = "check lock file"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("write backup data"); + + TEST_RESULT_VOID( + cmdLockWriteP(.percentComplete = VARUINT(5555), .sizeComplete = VARUINT64(1754824), .size = VARUINT64(3159000)), + "write backup data"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("read backup data"); + + LockReadResult lockReadResult; + TEST_ASSIGN(lockReadResult, cmdLockRead(lockTypeBackup, STRDEF("test")), "read backup data"); + TEST_RESULT_UINT(lockReadResult.status, lockReadStatusValid, "check status"); + TEST_RESULT_UINT(varUInt(lockReadResult.data.percentComplete), 5555, "check percent complete"); + TEST_RESULT_UINT(varUInt64(lockReadResult.data.sizeComplete), 1754824, "check size complete"); + TEST_RESULT_UINT(varUInt64(lockReadResult.data.size), 3159000, "check size"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("lock release"); + + TEST_RESULT_VOID(cmdLockReleaseP(), "release locks"); + TEST_STORAGE_LIST(storageTest, NULL, NULL, .comment = "check lock file does not exist"); + + TEST_ERROR(cmdLockReleaseP(), AssertError, "no lock is held by this process"); + TEST_RESULT_VOID(cmdLockReleaseP(.returnOnNoLock = true), "ignore no lock held"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("acquire stanza-create command lock"); + + argList = strLstNew(); + hrnCfgArgRawZ(argList, cfgOptStanza, "test"); + hrnCfgArgRawZ(argList, cfgOptLockPath, TEST_PATH); + hrnCfgArgRawZ(argList, cfgOptPgPath, "/pg1"); + HRN_CFG_LOAD(cfgCmdStanzaCreate, argList, .noStd = true); + + TEST_RESULT_VOID(cmdLockAcquireP(), "acquire stanza-create lock"); + TEST_STORAGE_LIST(storageTest, NULL, "test-archive.lock\ntest-backup.lock\n", .comment = "check lock file"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("lock release"); + + TEST_RESULT_VOID(cmdLockReleaseP(), "release locks"); + TEST_STORAGE_LIST(storageTest, NULL, NULL, .comment = "check lock file does not exist"); + } + + FUNCTION_HARNESS_RETURN_VOID(); +} diff --git a/test/src/module/common/lockTest.c b/test/src/module/common/lockTest.c index 203f19c1d..d9d15657d 100644 --- a/test/src/module/common/lockTest.c +++ b/test/src/module/common/lockTest.c @@ -1,7 +1,6 @@ /*********************************************************************************************************************************** Test Lock Handler ***********************************************************************************************************************************/ -#include "common/time.h" #include "storage/posix/storage.h" #include "common/harnessFork.h" @@ -16,103 +15,108 @@ testRun(void) FUNCTION_HARNESS_VOID(); // Create default storage object for testing - Storage *storageTest = storagePosixNewP(TEST_PATH_STR, .write = true); + const Storage *const storageTest = storagePosixNewP(TEST_PATH_STR, .write = true); // ***************************************************************************************************************************** - if (testBegin("lockAcquireFile() and lockReleaseFile()")) + if (testBegin("lock*()")) { - const String *archiveLock = STRDEF(TEST_PATH "/main-archive" LOCK_FILE_EXT); - int lockFdTest = -1; + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("acquire lock"); - lockLocal.execId = STRDEF("1-test"); + const String *const lockFile1Name = STRDEF("test1" LOCK_FILE_EXT); - TEST_ASSIGN(lockFdTest, lockAcquireFile(archiveLock, 0, true), "get lock"); - TEST_RESULT_BOOL(lockFdTest != -1, true, "lock succeeds"); - TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLock), true, "lock file was created"); + TEST_RESULT_VOID(lockInit(TEST_PATH_STR, STRDEF("1-test")), "init lock module"); + TEST_RESULT_BOOL(lockAcquireP(lockFile1Name), true, "acquire lock"); + TEST_ERROR_FMT(lockAcquireP(lockFile1Name), AssertError, "lock on file 'test1.lock' already held"); - lockLocal.file[lockTypeArchive].fd = lockFdTest; - lockLocal.file[lockTypeArchive].name = strDup(archiveLock); - TEST_RESULT_VOID(lockWriteDataP(lockTypeArchive), "write lock data"); - - lockLocal.execId = STRDEF("2-test"); - - TEST_ERROR_FMT( - lockAcquireFile(archiveLock, 0, true), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" - "HINT: is another pgBackRest process running?", - strZ(archiveLock)); - TEST_RESULT_BOOL(lockAcquireFile(archiveLock, 0, false) == -1, true, "lock is already held"); + TEST_ERROR(lockAcquireP(TEST_PATH_STR), LockAcquireError, "unable to acquire lock on file '" TEST_PATH "': Is a directory"); // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("acquire file lock on the same exec-id"); + TEST_TITLE("read errors"); - lockLocal.execId = STRDEF("1-test"); + TEST_ERROR( + lockReadData(STRDEF(BOGUS_STR), 999999999), FileReadError, + "unable to read lock file 'BOGUS': unable to read from BOGUS: [9] Bad file descriptor"); + TEST_RESULT_UINT(lockReadP(STRDEF(BOGUS_STR)).status, lockReadStatusMissing, "missing lock file"); - TEST_RESULT_INT(lockAcquireFile(archiveLock, 0, true), -2, "allow lock with same exec id"); + HRN_STORAGE_PUT_EMPTY(storageTest, "unlocked.lock"); + TEST_RESULT_UINT(lockReadP(STRDEF("unlocked.lock"), .remove = true).status, lockReadStatusUnlocked, "unlocked lock file"); // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("fail file lock on the same exec-id when lock file is empty"); + TEST_TITLE("read lock file (no data set)"); - HRN_SYSTEM_FMT("echo '' > %s", strZ(archiveLock)); - - TEST_ERROR_FMT( - lockAcquireFile(archiveLock, 0, true), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" - "HINT: is another pgBackRest process running?", - strZ(archiveLock)); + LockReadResult lockReadResult; + TEST_ASSIGN(lockReadResult, lockReadP(lockFile1Name), "read lock file"); + TEST_RESULT_UINT(lockReadResult.status, lockReadStatusValid, "check status"); + TEST_RESULT_PTR(lockReadResult.data.percentComplete, NULL, "check percent complete is NULL"); + TEST_RESULT_PTR(lockReadResult.data.sizeComplete, NULL, "check size complete is NULL"); + TEST_RESULT_PTR(lockReadResult.data.size, NULL, "check size is NULL"); // ------------------------------------------------------------------------------------------------------------------------- - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, archiveLock), "release lock"); + TEST_TITLE("invalidate lock file by truncating"); - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, archiveLock), "release lock"); - TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLock), false, "lock file was removed"); - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, archiveLock), "release lock again without error"); + LockFile *lockFile1 = lstFind(lockLocal.lockList, &lockFile1Name); + THROW_ON_SYS_ERROR_FMT( + ftruncate(lockFile1->fd, 0) == -1, FileWriteError, "unable to truncate '" TEST_PATH "/%s'", strZ(lockFile1Name)); + + TEST_RESULT_UINT(lockReadP(lockFile1Name).status, lockReadStatusInvalid, "invalid lock file"); // ------------------------------------------------------------------------------------------------------------------------- - String *subPathLock = strNewZ(TEST_PATH "/sub1/sub2/db-backup" LOCK_FILE_EXT); + TEST_TITLE("write errors"); - TEST_ASSIGN(lockFdTest, lockAcquireFile(subPathLock, 0, true), "get lock in subpath"); - TEST_RESULT_BOOL(storageExistsP(storageTest, subPathLock), true, "lock file was created"); - TEST_RESULT_BOOL(lockFdTest != -1, true, "lock succeeds"); - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, subPathLock), "release lock"); - TEST_RESULT_BOOL(storageExistsP(storageTest, subPathLock), false, "lock file was removed"); + TEST_ERROR(lockWriteP(STRDEF(BOGUS_STR)), AssertError, "lock file 'BOGUS' not found"); // ------------------------------------------------------------------------------------------------------------------------- - String *dirLock = strNewZ(TEST_PATH "/dir" LOCK_FILE_EXT); + TEST_TITLE("write lock file"); - HRN_SYSTEM_FMT("mkdir -p %s", strZ(dirLock)); + TEST_RESULT_VOID( + lockWriteP( + lockFile1Name, .percentComplete = VARUINT(5555), .sizeComplete = VARUINT64(1754824), .size = VARUINT64(3159000)), + "write lock data"); - TEST_ERROR_FMT( - lockAcquireFile(dirLock, 0, true), LockAcquireError, "unable to acquire lock on file '%s': Is a directory", - strZ(dirLock)); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("read lock file (data set)"); + + TEST_ASSIGN(lockReadResult, lockReadP(lockFile1Name), "read lock file"); + TEST_RESULT_UINT(lockReadResult.status, lockReadStatusValid, "check status"); + TEST_RESULT_UINT(varUInt(lockReadResult.data.percentComplete), 5555, "check percent complete"); + TEST_RESULT_UINT(varUInt64(lockReadResult.data.sizeComplete), 1754824, "check size complete"); + TEST_RESULT_UINT(varUInt64(lockReadResult.data.size), 3159000, "check size"); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("permissions error throw regardless of failOnLock"); - String *noPermLock = strNewZ(TEST_PATH "/noperm/noperm"); + const String *const noPermLock = strNewZ(TEST_PATH "/noperm/noperm"); HRN_SYSTEM_FMT("mkdir -p %s", strZ(strPath(noPermLock))); HRN_SYSTEM_FMT("chmod 000 %s", strZ(strPath(noPermLock))); TEST_ERROR_FMT( - lockAcquireFile(noPermLock, 100, true), LockAcquireError, + lockAcquireP(noPermLock), LockAcquireError, "unable to acquire lock on file '%s': Permission denied\n" "HINT: does '" TEST_USER ":" TEST_GROUP "' running pgBackRest have permissions on the '%s' file?", strZ(noPermLock), strZ(noPermLock)); TEST_ERROR_FMT( - lockAcquireFile(noPermLock, 100, false), LockAcquireError, + lockAcquireP(noPermLock, .returnOnNoLock = true), LockAcquireError, "unable to acquire lock on file '%s': Permission denied\n" "HINT: does '" TEST_USER ":" TEST_GROUP "' running pgBackRest have permissions on the '%s' file?", strZ(noPermLock), strZ(noPermLock)); // ------------------------------------------------------------------------------------------------------------------------- - String *backupLock = strNewZ(TEST_PATH "/main-backup" LOCK_FILE_EXT); + TEST_TITLE("acquire file lock on the same exec-id"); + + const String *const lockFileExecName = STRDEF("sub/exec" LOCK_FILE_EXT); + const String *const lockFileExec2Name = STRDEF("sub/exec2" LOCK_FILE_EXT); HRN_FORK_BEGIN() { HRN_FORK_CHILD_BEGIN() { - TEST_RESULT_INT_NE(lockAcquireFile(backupLock, 0, true), -1, "lock on fork"); + TEST_RESULT_BOOL(lockAcquireP(lockFileExecName), true, "lock on fork"); + TEST_RESULT_BOOL(lockAcquireP(lockFileExec2Name), true, "lock on fork"); + + // Corrupt exec2.lock + HRN_SYSTEM_FMT("echo '' > " TEST_PATH "/%s", strZ(lockFileExec2Name)); // Notify parent that lock has been acquired HRN_FORK_CHILD_NOTIFY_PUT(); @@ -130,237 +134,22 @@ testRun(void) lockLocal.execId = STRDEF("2-test"); TEST_ERROR_FMT( - lockAcquireFile(backupLock, 0, true), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" + lockAcquireP(lockFileExecName), LockAcquireError, + "unable to acquire lock on file '" TEST_PATH "/%s': Resource temporarily unavailable\n" "HINT: is another pgBackRest process running?", - strZ(backupLock)); + strZ(lockFileExecName)); - TEST_RESULT_VOID(lockAcquireFile(backupLock, 0, false), "success when failOnLock = false"); + TEST_RESULT_BOOL(lockAcquireP(lockFileExecName, .returnOnNoLock = true), false, "fail but return"); - // Notify child to release lock - HRN_FORK_PARENT_NOTIFY_PUT(0); - } - HRN_FORK_PARENT_END(); - } - HRN_FORK_END(); - } + lockLocal.execId = STRDEF("1-test"); - // ***************************************************************************************************************************** - if (testBegin("lockAcquireP(), lockRelease()")) - { - const String *stanza = STRDEF("test"); - String *archiveLockFile = strNewFmt(TEST_PATH "/%s-archive" LOCK_FILE_EXT, strZ(stanza)); - String *backupLockFile = strNewFmt(TEST_PATH "/%s-backup" LOCK_FILE_EXT, strZ(stanza)); - int lockFdTest = -1; + TEST_RESULT_BOOL(lockAcquireP(lockFileExecName), true, "succeed on same execId"); - // ------------------------------------------------------------------------------------------------------------------------- - TEST_ERROR(lockRelease(true), AssertError, "no lock is held by this process"); - TEST_RESULT_BOOL(lockRelease(false), false, "release when there is no lock"); - - // ------------------------------------------------------------------------------------------------------------------------- - lockInit(TEST_PATH_STR, STRDEF("1-test"), stanza, lockTypeArchive); - - TEST_ASSIGN(lockFdTest, lockAcquireFile(archiveLockFile, 0, true), "archive lock by file"); - TEST_RESULT_BOOL(lockAcquireP(.returnOnNoLock = true), false, "archive already locked"); - TEST_ERROR_FMT( - lockAcquireP(), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" - "HINT: is another pgBackRest process running?", - strZ(archiveLockFile)); - - lockLocal.type = lockTypeAll; - - TEST_ERROR_FMT( - lockAcquireP(), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" - "HINT: is another pgBackRest process running?", strZ(archiveLockFile)); - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, archiveLockFile), "release lock"); - - // ------------------------------------------------------------------------------------------------------------------------- - lockInit(TEST_PATH_STR, STRDEF("1-test"), stanza, lockTypeArchive); - - TEST_RESULT_BOOL(lockAcquireP(), true, "archive lock"); - TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLockFile), true, "archive lock file was created"); - TEST_ERROR(lockAcquireP(.returnOnNoLock = true), AssertError, "lock is already held by this process"); - TEST_RESULT_VOID(lockRelease(true), "release archive lock"); - - // ------------------------------------------------------------------------------------------------------------------------- - lockInit(TEST_PATH_STR, STRDEF("1-test"), stanza, lockTypeBackup); - - TEST_ASSIGN(lockFdTest, lockAcquireFile(backupLockFile, 0, true), "backup lock by file"); - - TEST_ERROR_FMT( - lockAcquireP(), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" - "HINT: is another pgBackRest process running?", strZ(backupLockFile)); - - lockLocal.type = lockTypeAll; - - TEST_ERROR_FMT( - lockAcquireP(), LockAcquireError, - "unable to acquire lock on file '%s': Resource temporarily unavailable\n" - "HINT: is another pgBackRest process running?", strZ(backupLockFile)); - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, archiveLockFile), "release archive lock"); - TEST_RESULT_VOID(lockReleaseFile(lockFdTest, backupLockFile), "release backup lock"); - - // ------------------------------------------------------------------------------------------------------------------------- - lockInit(TEST_PATH_STR, STRDEF("1-test"), stanza, lockTypeAll); - - TEST_RESULT_BOOL(lockAcquireP(), true, "all lock"); - TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLockFile), true, "archive lock file was created"); - TEST_RESULT_BOOL(storageExistsP(storageTest, backupLockFile), true, "backup lock file was created"); - - // Seek to start of file before read - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockTypeBackup].name)); - TEST_RESULT_STR( - lockReadFileData(backupLockFile, lockLocal.file[lockTypeBackup].fd).execId, STRDEF("1-test"), "verify execId"); - - TEST_RESULT_VOID(lockWriteDataP(lockTypeBackup), "write lock data"); - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockTypeBackup].name)); - TEST_RESULT_PTR( - lockReadFileData(backupLockFile, lockLocal.file[lockTypeBackup].fd).percentComplete, NULL, "verify percentComplete"); - - TEST_RESULT_VOID( - lockWriteDataP( - lockTypeBackup, .percentComplete = VARUINT(5555), .sizeComplete = VARUINT64(1754824), .size = VARUINT64(3159000)), - "write lock data"); - - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockTypeBackup].name)); - LockData lockDataResult = lockReadFileData(backupLockFile, lockLocal.file[lockTypeBackup].fd); - TEST_RESULT_UINT(varUInt(lockDataResult.percentComplete), 5555, "verify percentComplete"); - TEST_RESULT_UINT(varUInt64(lockDataResult.sizeComplete), 1754824, "verify sizeProgress"); - TEST_RESULT_UINT(varUInt64(lockDataResult.size), 3159000, "verify sizeTotal"); - - // The size/sizeComplete values should be large enough to overflow a uint32 to ensure uint64 is used for read/write - TEST_RESULT_VOID( - lockWriteDataP( - lockTypeBackup, .percentComplete = VARUINT(8888), .sizeComplete = VARUINT64(3223802441008), - .size = VARUINT64(6126216975438)), - "write lock data"); - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockTypeBackup].name)); - lockDataResult = lockReadFileData(backupLockFile, lockLocal.file[lockTypeBackup].fd); - TEST_RESULT_UINT(varUInt(lockDataResult.percentComplete), 8888, "verify percentComplete"); - TEST_RESULT_UINT(varUInt64(lockDataResult.sizeComplete), 3223802441008, "verify sizeProgress"); - TEST_RESULT_UINT(varUInt64(lockDataResult.size), 6126216975438, "verify sizeTotal"); - - TEST_ERROR( - lockAcquireP(.returnOnNoLock = true), AssertError, - "assertion '!param.returnOnNoLock || lockLocal.type != lockTypeAll' failed"); - TEST_RESULT_VOID(lockRelease(true), "release all locks"); - - TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLockFile), false, "archive lock file was removed"); - TEST_RESULT_BOOL(storageExistsP(storageTest, backupLockFile), false, "backup lock file was removed"); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("acquire lock on the same exec-id and release"); - - lockInit(TEST_PATH_STR, STRDEF("1-test"), stanza, lockTypeBackup); - - TEST_RESULT_BOOL(lockAcquireP(), true, "backup lock"); - - // Make it look there is no lock - lockFdTest = lockLocal.file[lockTypeBackup].fd; - String *lockFileTest = strDup(lockLocal.file[lockTypeBackup].name); - lockLocal.held = false; - - TEST_RESULT_BOOL(lockAcquireP(), true, "backup lock again"); - TEST_RESULT_VOID(lockRelease(true), "release backup lock"); - - // Release lock manually - lockReleaseFile(lockFdTest, lockFileTest); - } - - // ***************************************************************************************************************************** - if (testBegin("lockRead*()")) - { - TEST_TITLE("missing lock file"); - - TEST_RESULT_UINT(lockReadFileP(STRDEF(TEST_PATH "/missing.lock")).status, lockReadStatusMissing, "lock read"); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("unlocked file"); - - HRN_STORAGE_PUT_EMPTY(storageTest, "unlocked.lock"); - TEST_RESULT_UINT(lockReadFileP(STRDEF(TEST_PATH "/unlocked.lock")).status, lockReadStatusUnlocked, "lock read"); - TEST_STORAGE_LIST(storageTest, NULL, "unlocked.lock\n", .remove = true); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("execId && pid valid file"); - - lockInit(TEST_PATH_STR, STRDEF("1-test"), STRDEF("1-test"), lockTypeBackup); - - TEST_RESULT_BOOL(lockAcquireP(), true, "backup lock"); - TEST_RESULT_BOOL(storageExistsP(storageTest, lockLocal.file[lockTypeBackup].name), true, "backup lock file was created"); - - // Overwrite backup lock file with execId of 1-test and pid of 12345 - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockTypeBackup].name)); - - TEST_RESULT_INT(ftruncate(lockLocal.file[lockTypeBackup].fd, 0), 0, "truncate lockLocal.file[lockTypeBackup].fd"); - - IoWrite *const write = ioFdWriteNewOpen(lockLocal.file[lockTypeBackup].name, lockLocal.file[lockTypeBackup].fd, 0); - - ioCopyP(ioBufferReadNewOpen(BUFSTRDEF("{\"execId\":\"1-test\",\"pid\":12345}")), write); - ioWriteClose(write); - - // Seek to start of file before read - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, (uint64_t)0, - strZ(lockLocal.file[lockTypeBackup].name)); - - LockReadResult result = {0}; - TEST_ASSIGN(result, lockReadFileP(lockLocal.file[lockTypeBackup].name), "lock read"); - TEST_RESULT_STR(result.data.execId, STRDEF("1-test"), "lock read execId 1-test"); - TEST_RESULT_UINT((uint64_t)result.data.processId, 12345, "lock read pid 12345"); - TEST_RESULT_VOID(lockRelease(true), "release backup lock"); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("invalid locked file"); - - HRN_FORK_BEGIN() - { - HRN_FORK_CHILD_BEGIN() - { - lockInit(TEST_PATH_STR, STRDEF("1-test"), STRDEF("test"), lockTypeBackup); - - TEST_RESULT_BOOL(lockAcquireP(), true, "acquire lock"); - - // Overwrite lock file with bogus data - THROW_ON_SYS_ERROR_FMT( - lseek(lockLocal.file[lockTypeBackup].fd, 0, SEEK_SET) == -1, FileOpenError, STORAGE_ERROR_READ_SEEK, - (uint64_t)0, strZ(lockLocal.file[lockTypeBackup].name)); - - IoWrite *const write = ioFdWriteNewOpen(lockLocal.file[lockTypeBackup].name, lockLocal.file[lockTypeBackup].fd, 0); - - ioCopyP(ioBufferReadNewOpen(BUFSTRDEF("BOGUS")), write); - ioWriteClose(write); - - // Notify parent that lock has been acquired - HRN_FORK_CHILD_NOTIFY_PUT(); - - // Wait for parent to allow release lock - HRN_FORK_CHILD_NOTIFY_GET(); - } - HRN_FORK_CHILD_END(); - - HRN_FORK_PARENT_BEGIN() - { - // Wait for child to acquire lock - HRN_FORK_PARENT_NOTIFY_GET(0); - - TEST_RESULT_UINT( - lockReadFileP(STRDEF(TEST_PATH "/test-backup.lock"), .remove = true).status, lockReadStatusInvalid, - "lock read"); - TEST_STORAGE_LIST(storageTest, NULL, NULL); + TEST_ERROR_FMT( + lockAcquireP(lockFileExec2Name), LockAcquireError, + "unable to acquire lock on file '" TEST_PATH "/%s': Resource temporarily unavailable\n" + "HINT: is another pgBackRest process running?", + strZ(lockFileExec2Name)); // Notify child to release lock HRN_FORK_PARENT_NOTIFY_PUT(0); @@ -370,43 +159,11 @@ testRun(void) HRN_FORK_END(); // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("valid locked file"); + TEST_TITLE("lock release"); - HRN_FORK_BEGIN() - { - HRN_FORK_CHILD_BEGIN() - { - lockInit(TEST_PATH_STR, STRDEF("1-test"), STRDEF("test"), lockTypeBackup); - - TEST_RESULT_BOOL(lockAcquireP(), true, "acquire lock"); - - // Notify parent that lock has been acquired - HRN_FORK_CHILD_NOTIFY_PUT(); - - // Wait for parent to allow release lock - HRN_FORK_CHILD_NOTIFY_GET(); - } - HRN_FORK_CHILD_END(); - - HRN_FORK_PARENT_BEGIN() - { - // Wait for child to acquire lock - HRN_FORK_PARENT_NOTIFY_GET(0); - - LockReadResult result = {0}; - TEST_ASSIGN(result, lockRead(TEST_PATH_STR, STRDEF("test"), lockTypeBackup), "lock read"); - - TEST_RESULT_BOOL(result.data.processId != 0, true, "check processId"); - TEST_RESULT_STR_Z(result.data.execId, "1-test", "check execId"); - - TEST_STORAGE_LIST(storageTest, NULL, "test-backup.lock\n"); - - // Notify child to release lock - HRN_FORK_PARENT_NOTIFY_PUT(0); - } - HRN_FORK_PARENT_END(); - } - HRN_FORK_END(); + TEST_RESULT_VOID(lockReleaseP(), "release locks"); + TEST_ERROR(lockReleaseP(), AssertError, "no lock is held by this process"); + TEST_RESULT_VOID(lockReleaseP(.returnOnNoLock = true), "ignore no lock held"); } FUNCTION_HARNESS_RETURN_VOID(); diff --git a/test/src/module/config/loadTest.c b/test/src/module/config/loadTest.c index e2014cc39..dec243eb0 100644 --- a/test/src/module/config/loadTest.c +++ b/test/src/module/config/loadTest.c @@ -755,7 +755,7 @@ testRun(void) hrnCfgArgRawZ(argList, cfgOptLogLevelStderr, CFGOPTVAL_ARCHIVE_MODE_OFF_Z); strLstAddZ(argList, CFGCMD_BACKUP); TEST_RESULT_VOID(cfgLoad(strLstSize(argList), strLstPtr(argList)), "load config for backup"); - lockRelease(true); + cmdLockReleaseP(); // Only the error case is tested here, success is tested in cfgLoad() TEST_RESULT_VOID(cfgLoadLogFile(), "attempt to open bogus log file"); @@ -777,7 +777,7 @@ testRun(void) TEST_RESULT_VOID(cfgLoad(strLstSize(argList), strLstPtr(argList)), "load config"); TEST_RESULT_VOID(storageRepoWrite(), "check writable storage"); - lockRelease(true); + cmdLockReleaseP(); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("dry-run valid, dry-run"); @@ -787,7 +787,7 @@ testRun(void) TEST_RESULT_VOID(cfgLoad(strLstSize(argList), strLstPtr(argList)), "load config"); TEST_ERROR( storageRepoWrite(), AssertError, "unable to get writable storage in dry-run mode or before dry-run is initialized"); - lockRelease(true); + cmdLockReleaseP(); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("command does not have umask and disables keep-alives"); @@ -928,7 +928,7 @@ testRun(void) TEST_RESULT_INT(socketLocal.tcpKeepAliveIdle, 2222, "check socketLocal.tcpKeepAliveIdle"); TEST_RESULT_INT(socketLocal.tcpKeepAliveInterval, 888, "check socketLocal.tcpKeepAliveInterval"); - lockRelease(true); + cmdLockReleaseP(); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("local command opens log file with special filename"); @@ -1011,7 +1011,7 @@ testRun(void) TEST_RESULT_VOID(cfgLoad(strLstSize(argList), strLstPtr(argList)), "open log file"); TEST_RESULT_INT(lstat(TEST_PATH "/test-archive-get-async.log", &statLog), 0, "check log file exists"); - lockRelease(true); + cmdLockReleaseP(); } FUNCTION_HARNESS_RETURN_VOID();