1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-07-17 01:12:23 +02:00

Convert lockAcquire() to lockAcquireP().

This makes a few current parameters optional and allows for more optional parameters with less code churn.
This commit is contained in:
David Steele
2023-03-24 10:34:42 +08:00
parent c8ec114c8c
commit f1caecc4ff
15 changed files with 67 additions and 57 deletions

View File

@ -464,8 +464,8 @@ HRN_FORK_BEGIN()
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_INT_NE(
lockAcquire(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("999-ffffffff"), lockTypeBackup, 0, true),
-1, "create backup/expire lock");
lockAcquireP(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("999-ffffffff"), lockTypeBackup), -1,
"create backup/expire lock");
// Notify parent that lock has been acquired
HRN_FORK_CHILD_NOTIFY_PUT();

View File

@ -531,8 +531,8 @@ HRN_FORK_BEGIN()
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_INT_NE(
lockAcquire(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("999-ffffffff"), lockTypeBackup, 0, true),
-1, "create backup/expire lock");
lockAcquireP(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("999-ffffffff"), lockTypeBackup), -1,
"create backup/expire lock");
// Notify parent that lock has been acquired
HRN_FORK_CHILD_NOTIFY_PUT();

View File

@ -719,9 +719,9 @@ cmdArchiveGet(void)
// 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) &&
lockAcquire(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType(), 0,
false))
lockAcquireP(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType(),
.returnOnNoLock = true))
{
// Get control info
PgControl pgControl = pgControlFromFile(storagePg(), cfgOptionStrNull(cfgOptPgVersionForce));

View File

@ -362,9 +362,9 @@ cmdArchivePush(void)
// 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 &&
lockAcquire(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType(), 0,
false))
lockAcquireP(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType(),
.returnOnNoLock = true))
{
// The async process should not output on the console at all
KeyValue *optionReplace = kvNew();

View File

@ -77,9 +77,8 @@ cmdRemote(ProtocolServer *const server)
lockStopTest();
// Acquire the lock
lockAcquire(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType(), 0,
true);
lockAcquireP(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType());
}
}

View File

@ -392,15 +392,16 @@ lockAcquireFile(const String *const lockFile, const TimeMSec lockTimeout, const
FN_EXTERN bool
lockAcquire(
const String *lockPath, const String *stanza, const String *execId, LockType lockType, TimeMSec lockTimeout, bool failOnNoLock)
const String *const lockPath, const String *const stanza, const String *const execId, const LockType lockType,
const LockAcquireParam param)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(STRING, lockPath);
FUNCTION_LOG_PARAM(STRING, stanza);
FUNCTION_LOG_PARAM(STRING, execId);
FUNCTION_LOG_PARAM(ENUM, lockType);
FUNCTION_LOG_PARAM(TIMEMSEC, lockTimeout);
FUNCTION_LOG_PARAM(BOOL, failOnNoLock);
FUNCTION_LOG_PARAM(TIMEMSEC, param.timeout);
FUNCTION_LOG_PARAM(BOOL, param.returnOnNoLock);
FUNCTION_LOG_END();
ASSERT(lockPath != NULL);
@ -410,7 +411,7 @@ lockAcquire(
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(failOnNoLock || lockType != lockTypeAll);
ASSERT(!param.returnOnNoLock || lockType != lockTypeAll);
// Don't allow another lock if one is already held
if (lockLocal.held != lockTypeNone)
@ -446,7 +447,7 @@ lockAcquire(
}
MEM_CONTEXT_END();
lockLocal.file[lockIdx].fd = lockAcquireFile(lockLocal.file[lockIdx].name, lockTimeout, failOnNoLock);
lockLocal.file[lockIdx].fd = lockAcquireFile(lockLocal.file[lockIdx].name, param.timeout, !param.returnOnNoLock);
if (lockLocal.file[lockIdx].fd == -1)
{

View File

@ -39,8 +39,18 @@ Functions
***********************************************************************************************************************************/
// 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.
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(lockPath, stanza, execId, lockType, ...) \
lockAcquire(lockPath, stanza, execId, lockType, (LockAcquireParam) {VAR_PARAM_INIT, __VA_ARGS__})
FN_EXTERN bool lockAcquire(
const String *lockPath, const String *stanza, const String *execId, LockType lockType, TimeMSec lockTimeout, bool failOnNoLock);
const String *lockPath, const String *stanza, const String *execId, LockType lockType, LockAcquireParam param);
// Release a lock
FN_EXTERN bool lockRelease(bool failOnNoLock);

View File

@ -503,10 +503,7 @@ cfgLoad(unsigned int argListSize, const char *argList[])
// Acquire a lock if this command requires a lock
if (cfgLockRequired() && !cfgCommandHelp())
{
lockAcquire(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType(), 0, true);
}
lockAcquireP(cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), cfgLockType());
// Update options that have complex rules
cfgLoadUpdateOption();

View File

@ -29,7 +29,7 @@ hrnCmdBackup(void)
{
FUNCTION_HARNESS_VOID();
lockAcquire(STR(testPath()), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), lockTypeBackup, 0, true);
lockAcquireP(STR(testPath()), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), lockTypeBackup);
TRY_BEGIN()
{

View File

@ -726,9 +726,9 @@ testRun(void)
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_VOID(
lockAcquire(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), STRDEF("999-dededede"), cfgLockType(), 30000,
true),
lockAcquireP(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), STRDEF("999-dededede"), cfgLockType(),
.timeout = 30000, .returnOnNoLock = true),
"acquire lock");
// Notify parent that lock has been acquired

View File

@ -749,8 +749,9 @@ testRun(void)
{
HRN_FORK_CHILD_BEGIN()
{
lockAcquire(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), STRDEF("555-fefefefe"), cfgLockType(), 30000, true);
lockAcquireP(
cfgOptionStr(cfgOptLockPath), cfgOptionStr(cfgOptStanza), STRDEF("555-fefefefe"), cfgLockType(),
.timeout = 30000, .returnOnNoLock = true);
// Notify parent that lock has been acquired
HRN_FORK_CHILD_NOTIFY_PUT();

View File

@ -2127,7 +2127,7 @@ testRun(void)
currentPercentComplete = 4567;
TEST_RESULT_VOID(
lockAcquire(TEST_PATH_STR, cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), lockTypeBackup, 0, true),
lockAcquireP(TEST_PATH_STR, cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), lockTypeBackup),
"acquire backup lock");
TEST_RESULT_VOID(
backupJobResult(

View File

@ -254,7 +254,9 @@ testRun(void)
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_BOOL(
lockAcquire(STRDEF(HRN_PATH "/lock"), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), 0, 30000, true),
lockAcquireP(
STRDEF(HRN_PATH "/lock"), cfgOptionStr(cfgOptStanza), cfgOptionStr(cfgOptExecId), lockTypeArchive,
.timeout = 30000),
true, "child process acquires lock");
// Notify parent that lock has been acquired

View File

@ -237,8 +237,8 @@ testRun(void)
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_INT_NE(
lockAcquire(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("999-ffffffff"), lockTypeBackup, 0, true),
-1, "create backup/expire lock");
lockAcquireP(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("999-ffffffff"), lockTypeBackup), -1,
"create backup/expire lock");
// Notify parent that lock has been acquired
HRN_FORK_CHILD_NOTIFY_PUT();
@ -430,8 +430,8 @@ testRun(void)
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_INT_NE(
lockAcquire(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("777-afafafaf"), lockTypeBackup, 0, true),
-1, "create backup/expire lock");
lockAcquireP(cfgOptionStr(cfgOptLockPath), STRDEF("stanza1"), STRDEF("777-afafafaf"), lockTypeBackup), -1,
"create backup/expire lock");
TEST_RESULT_VOID(lockWriteDataP(lockTypeBackup), "write lock data");
// Notify parent that lock has been acquired
@ -1026,8 +1026,8 @@ testRun(void)
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_INT_NE(
lockAcquire(cfgOptionStr(cfgOptLockPath), STRDEF("stanza2"), STRDEF("999-ffffffff"), lockTypeBackup, 0, true),
-1, "create backup/expire lock");
lockAcquireP(cfgOptionStr(cfgOptLockPath), STRDEF("stanza2"), STRDEF("999-ffffffff"), lockTypeBackup), -1,
"create backup/expire lock");
TEST_RESULT_VOID(lockWriteDataP(lockTypeBackup, .percentComplete = VARUINT(4545)), "write lock data");
// Notify parent that lock has been acquired
@ -1466,8 +1466,8 @@ testRun(void)
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_INT_NE(
lockAcquire(cfgOptionStr(cfgOptLockPath), STRDEF("stanza2"), STRDEF("999-ffffffff"), lockTypeBackup, 0, true),
-1, "create backup/expire lock");
lockAcquireP(cfgOptionStr(cfgOptLockPath), STRDEF("stanza2"), STRDEF("999-ffffffff"), lockTypeBackup), -1,
"create backup/expire lock");
TEST_RESULT_VOID(lockWriteDataP(lockTypeBackup, .percentComplete = VARUINT(5555)), "write lock data");
// Notify parent that lock has been acquired

View File

@ -146,7 +146,7 @@ testRun(void)
}
// *****************************************************************************************************************************
if (testBegin("lockAcquire(), lockRelease()"))
if (testBegin("lockAcquireP(), lockRelease()"))
{
const String *stanza = STRDEF("test");
String *archiveLockFile = strNewFmt(TEST_PATH "/%s-archive" LOCK_FILE_EXT, strZ(stanza));
@ -162,14 +162,15 @@ testRun(void)
TEST_ASSIGN(lockFdTest, lockAcquireFile(archiveLockFile, 0, true), "archive lock by file");
TEST_RESULT_BOOL(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeArchive, 0, false), false, "archive already locked");
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeArchive, .returnOnNoLock = true), false,
"archive already locked");
TEST_ERROR_FMT(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeArchive, 0, true), LockAcquireError,
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeArchive), LockAcquireError,
"unable to acquire lock on file '%s': Resource temporarily unavailable\n"
"HINT: is another pgBackRest process running?",
strZ(archiveLockFile));
TEST_ERROR_FMT(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeAll, 0, true), LockAcquireError,
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeAll), 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");
@ -177,10 +178,10 @@ testRun(void)
// -------------------------------------------------------------------------------------------------------------------------
lockLocal.execId = STRDEF("1-test");
TEST_RESULT_BOOL(lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeArchive, 0, true), true, "archive lock");
TEST_RESULT_BOOL(lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeArchive), true, "archive lock");
TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLockFile), true, "archive lock file was created");
TEST_ERROR(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeArchive, 0, false), AssertError,
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeArchive, .returnOnNoLock = true), AssertError,
"lock is already held by this process");
TEST_RESULT_VOID(lockRelease(true), "release archive lock");
@ -190,11 +191,11 @@ testRun(void)
TEST_ASSIGN(lockFdTest, lockAcquireFile(backupLockFile, 0, true), "backup lock by file");
TEST_ERROR_FMT(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeBackup, 0, true), LockAcquireError,
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeBackup), LockAcquireError,
"unable to acquire lock on file '%s': Resource temporarily unavailable\n"
"HINT: is another pgBackRest process running?", strZ(backupLockFile));
TEST_ERROR_FMT(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeAll, 0, true), LockAcquireError,
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("2-test"), lockTypeAll), 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");
@ -203,7 +204,7 @@ testRun(void)
// -------------------------------------------------------------------------------------------------------------------------
lockLocal.execId = STRDEF("1-test");
TEST_RESULT_BOOL(lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeAll, 0, true), true, "all lock");
TEST_RESULT_BOOL(lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeAll), 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");
@ -238,8 +239,8 @@ testRun(void)
"verify percentComplete");
TEST_ERROR(
lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeAll, 0, false), AssertError,
"assertion 'failOnNoLock || lockType != lockTypeAll' failed");
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeAll, .returnOnNoLock = true), AssertError,
"assertion '!param.returnOnNoLock || lockType != lockTypeAll' failed");
TEST_RESULT_VOID(lockRelease(true), "release all locks");
TEST_RESULT_BOOL(storageExistsP(storageTest, archiveLockFile), false, "archive lock file was removed");
@ -248,14 +249,15 @@ testRun(void)
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("acquire lock on the same exec-id and release");
TEST_RESULT_BOOL(lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeBackup, 0, true), true, "backup lock");
TEST_RESULT_BOOL(lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeBackup), true, "backup lock");
// Make it look there is no lock
lockFdTest = lockLocal.file[lockTypeBackup].fd;
String *lockFileTest = strDup(lockLocal.file[lockTypeBackup].name);
lockLocal.held = lockTypeNone;
TEST_RESULT_BOOL(lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeBackup, 0, true), true, "backup lock again");
TEST_RESULT_BOOL(
lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeBackup), true, "backup lock again");
TEST_RESULT_VOID(lockRelease(true), "release backup lock");
// Release lock manually
@ -282,7 +284,7 @@ testRun(void)
const String *stanza = STRDEF("1-test");
lockLocal.execId = STRDEF("1-test");
TEST_RESULT_BOOL(lockAcquire(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeBackup, 0, true), true, "backup lock");
TEST_RESULT_BOOL(lockAcquireP(TEST_PATH_STR, stanza, STRDEF("1-test"), lockTypeBackup), 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
@ -315,8 +317,7 @@ testRun(void)
{
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_BOOL(
lockAcquire(TEST_PATH_STR, STRDEF("test"), STRDEF("test"), lockTypeBackup, 0, true), true, "acquire lock");
TEST_RESULT_BOOL(lockAcquireP(TEST_PATH_STR, STRDEF("test"), STRDEF("test"), lockTypeBackup), true, "acquire lock");
// Overwrite lock file with bogus data
THROW_ON_SYS_ERROR_FMT(
@ -360,8 +361,7 @@ testRun(void)
{
HRN_FORK_CHILD_BEGIN()
{
TEST_RESULT_BOOL(
lockAcquire(TEST_PATH_STR, STRDEF("test"), STRDEF("test"), lockTypeBackup, 0, true), true, "acquire lock");
TEST_RESULT_BOOL(lockAcquireP(TEST_PATH_STR, STRDEF("test"), STRDEF("test"), lockTypeBackup), true, "acquire lock");
// Notify parent that lock has been acquired
HRN_FORK_CHILD_NOTIFY_PUT();