db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-12 10:04:14 +02:00
Code Issues Releases Activity

Throw error when unable to read lock process.

Browse Source 
Previously the process id was skipped if it did not exist. Instead, throw an error and handle the errors in downstream code.

This was probably ignored at some point to provide backward-compatibility, but that is no longer required, if it ever was.
This commit is contained in:
David Steele 2022-04-11 14:08:16 -04:00 committed by GitHub
parent fe1ac210bb
commit fa40bcdc5c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 65 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
doc/xml/release.xml
View File

@ -158,14 +158,19 @@
</release-item>
<release-item>
<github-pull-request id="1712"/>
<commit subject="Add lockRead*() functions for reading locks from another process.">
<github-pull-request id="1712"/>
</commit>
<commit subject="Throw error when unable to read process from lock file.">
<github-pull-request id="1715"/>
</commit>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="reid.thompson"/>
</release-item-contributor-list>
<p>Add <code>lockRead*()</code> functions for reading locks from another process.</p>
<p>Lock module refactoring.</p>
</release-item>
</release-development-list>
</release-core-list>

19
src/command/control/stop.c
View File

@ -69,21 +69,18 @@ cmdStop(void)
lockFile = strNewFmt("%s/%s", strZ(lockPath), strZ(lockFile));
LockReadResult lockRead = lockReadFileP(lockFile, .remove = true);
// If we cannot open the lock file for any reason then warn and continue to next file
if (lockRead.status != lockReadStatusValid && lockRead.status != lockReadStatusUnlocked)
// If we cannot read the lock file for any reason then warn and continue to next file
if (lockRead.status != lockReadStatusValid)
{
LOG_WARN_FMT("unable to open lock file %s", strZ(lockFile));
LOG_WARN_FMT("unable to read lock file %s", strZ(lockFile));
continue;
}
// The file is locked so that means there is a running process -- send a term signal to the process if valid
if (lockRead.data.processId != 0)
{
if (kill(lockRead.data.processId, SIGTERM) != 0)
LOG_WARN_FMT("unable to send term signal to process %d", lockRead.data.processId);
else
LOG_INFO_FMT("sent term signal to process %d", lockRead.data.processId);
}
// The lock file is valid so that means there is a running process -- send a term signal to the process
if (kill(lockRead.data.processId, SIGTERM) != 0)
LOG_WARN_FMT("unable to send term signal to process %d", lockRead.data.processId);
else
LOG_INFO_FMT("sent term signal to process %d", lockRead.data.processId);
}
}
}

50
src/common/lock.c
View File

@ -92,29 +92,36 @@ lockReadFileData(const String *const lockFile, const int fd)
LockData result = {0};
MEM_CONTEXT_TEMP_BEGIN()
TRY_BEGIN()
{
// Read contents of file
Buffer *const buffer = bufNew(LOCK_BUFFER_SIZE);
IoWrite *const write = ioBufferWriteNewOpen(buffer);
ioCopyP(ioFdReadNewOpen(lockFile, fd, 0), write);
ioWriteClose(write);
// Parse the file
const StringList *const parse = strLstNewSplitZ(strNewBuf(buffer), LF_Z);
MEM_CONTEXT_PRIOR_BEGIN()
MEM_CONTEXT_TEMP_BEGIN()
{
if (!strEmpty(strTrim(strLstGet(parse, 0))))
result.processId = cvtZToInt(strZ(strLstGet(parse, 0)));
// Read contents of file
Buffer *const buffer = bufNew(LOCK_BUFFER_SIZE);
IoWrite *const write = ioBufferWriteNewOpen(buffer);
if (strLstSize(parse) == 3)
result.execId = strDup(strLstGet(parse, 1));
ioCopyP(ioFdReadNewOpen(lockFile, fd, 0), write);
ioWriteClose(write);
// Parse the file
const StringList *const parse = strLstNewSplitZ(strNewBuf(buffer), LF_Z);
MEM_CONTEXT_PRIOR_BEGIN()
{
if (strLstSize(parse) == 3)
result.execId = strDup(strLstGet(parse, 1));
result.processId = cvtZToInt(strZ(strLstGet(parse, 0)));
}
MEM_CONTEXT_PRIOR_END();
}
MEM_CONTEXT_PRIOR_END();
MEM_CONTEXT_TEMP_END();
}
MEM_CONTEXT_TEMP_END();
CATCH_ANY()
{
THROWP_FMT(errorType(), "unable to read lock file '%s': %s", strZ(lockFile), errorMessage());
}
TRY_END();
FUNCTION_LOG_RETURN_STRUCT(result);
}
@ -286,6 +293,13 @@ lockAcquireFile(const String *const lockFile, const TimeMSec lockTimeout, const
{
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);

23
test/src/module/command/controlTest.c
View File

@ -136,14 +136,14 @@ testRun(void)
TEST_STORAGE_EXISTS(hrnStorage, "lock/db" STOP_FILE_EXT, .remove = true, .comment = "stanza stop file created, remove");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("unable to open lock file");
TEST_TITLE("unable to read lock file");
HRN_STORAGE_PUT_EMPTY(
hrnStorage, "lock/db-archive" LOCK_FILE_EXT, .modeFile = 0222,
.comment = "create a lock file that cannot be opened");
TEST_RESULT_VOID(cmdStop(), "stanza, create stop file but unable to open lock file");
TEST_RESULT_VOID(cmdStop(), "stanza, create stop file but unable to read lock file");
TEST_STORAGE_EXISTS(hrnStorage, "lock/db" STOP_FILE_EXT, .comment = "stanza stop file created");
TEST_RESULT_LOG("P00 WARN: unable to open lock file " HRN_PATH "/lock/db-archive" LOCK_FILE_EXT);
TEST_RESULT_LOG("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-archive" LOCK_FILE_EXT);
HRN_STORAGE_PATH_REMOVE(hrnStorage, "lock", .recurse = true, .errorOnMissing = true, .comment = "remove the lock path");
// -------------------------------------------------------------------------------------------------------------------------
@ -155,6 +155,8 @@ testRun(void)
hrnStorage, "lock", "db" STOP_FILE_EXT "\n",
.comment = "stanza stop file created, no other process lock, lock file was removed");
TEST_RESULT_LOG_FMT("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("empty lock file with another process lock, processId == NULL");
@ -192,6 +194,8 @@ testRun(void)
// Notify child to release lock
HRN_FORK_PARENT_NOTIFY_PUT(0);
TEST_RESULT_LOG_FMT("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock");
}
HRN_FORK_PARENT_END();
}
@ -234,6 +238,8 @@ testRun(void)
// Notify child to release lock
HRN_FORK_PARENT_NOTIFY_PUT(0);
TEST_RESULT_LOG_FMT("P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock");
}
HRN_FORK_PARENT_END();
}
@ -336,9 +342,13 @@ testRun(void)
TEST_STORAGE_EXISTS(hrnStorage, "lock/all" STOP_FILE_EXT, .comment = "stanza stop file created");
TEST_STORAGE_LIST(
hrnStorage, "lock", "all" STOP_FILE_EXT "\n" "db-junk.txt\n", .comment = "stop file created, all lock files processed");
TEST_RESULT_LOG("");
HRN_STORAGE_PATH_REMOVE(hrnStorage, "lock", .recurse = true, .errorOnMissing = true, .comment = "remove the lock path");
TEST_RESULT_LOG_FMT(
"P00 WARN: unable to read lock file " HRN_PATH "/lock/db-archive.lock\n"
"P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock\n"
"P00 WARN: unable to read lock file " HRN_PATH "/lock/db1-backup.lock");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("stanza, force stop = process only stanza lock files, ignore other stanza lock files and other files");
@ -355,8 +365,11 @@ testRun(void)
TEST_STORAGE_LIST(
hrnStorage, "lock", "db-junk.txt\ndb" STOP_FILE_EXT "\n" "db1-backup" LOCK_FILE_EXT "\n",
.comment = "stop file created, stanza lock file was removed, other stanza lock and other files remain");
TEST_RESULT_LOG("");
HRN_STORAGE_PATH_REMOVE(hrnStorage, "lock", .recurse = true, .errorOnMissing = true, .comment = "remove the lock path");
TEST_RESULT_LOG_FMT(
"P00 WARN: unable to read lock file " HRN_PATH "/lock/db-archive.lock\n"
"P00 WARN: unable to read lock file " HRN_PATH "/lock/db-backup.lock");
}
FUNCTION_HARNESS_RETURN_VOID();