1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-01-30 05:39:12 +02:00

Error on all lock failures except another process holding the lock.

The archive-get/archive-push commands would not error for, .e.g permissions errors, when attempting to get a lock before launching the async process. Since the async process was not launched there would be no error status file and the user would get a generic failure message. Also, there would be no async log.

Refactor lockAcquireFile() to throw an error when failOnNoLock = false unless the file is locked by another process. This seems to be the original intent of this parameter and there may have been a mistake when porting from Perl. In any case it looks wrong enough to be considered a bug.
This commit is contained in:
David Steele 2022-05-03 10:13:32 -04:00 committed by GitHub
parent eb435becb3
commit 9629908694
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 1 deletions

View File

@ -16,6 +16,23 @@
<release-list>
<release date="XXXX-XX-XX" version="2.39dev" title="UNDER DEVELOPMENT">
<release-core-list>
<release-bug-list>
<release-item>
<github-issue id="1722"/>
<github-pull-request id="1730"/>
<release-item-contributor-list>
<release-item-ideator id="geir.raness"/>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="reid.thompson"/>
<!-- Actually tester, but we don't have a tag for that yet -->
<release-item-reviewer id="geir.raness"/>
</release-item-contributor-list>
<p>Error on all lock failures except another process holding the lock.</p>
</release-item>
</release-bug-list>
<release-feature-list>
<release-item>
<commit subject="Refactor target type checking for clarity in restore module."/>
@ -11250,6 +11267,11 @@
<contributor-id type="github">fpa-postgres</contributor-id>
</contributor>
<contributor id="geir.raness">
<contributor-name-display>Geir R&amp;aring;ness</contributor-name-display>
<contributor-id type="github">geirra</contributor-id>
</contributor>
<contributor id="greg.sabino.mullane">
<contributor-name-display>Greg Sabino Mullane</contributor-name-display>
<contributor-id type="github">turnstep</contributor-id>

View File

@ -322,7 +322,7 @@ lockAcquireFile(const String *const lockFile, const TimeMSec lockTimeout, const
if (result == -1)
{
// Error when requested
if (failOnNoLock)
if (failOnNoLock || errNo != EWOULDBLOCK)
{
const String *errorHint = NULL;

View File

@ -87,6 +87,8 @@ testRun(void)
strZ(strNewFmt("unable to acquire lock on file '%s': Is a directory", strZ(dirLock))));
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("permissions error throw regardless of failOnLock");
String *noPermLock = strNewZ(TEST_PATH "/noperm/noperm");
HRN_SYSTEM_FMT("mkdir -p 750 %s", strZ(strPath(noPermLock)));
@ -97,6 +99,11 @@ testRun(void)
"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,
"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);
@ -130,6 +137,8 @@ testRun(void)
"unable to acquire lock on file '%s': Resource temporarily unavailable\n"
"HINT: is another pgBackRest process running?", strZ(backupLock))));
TEST_RESULT_VOID(lockAcquireFile(backupLock, 0, false), "success when failOnLock = false");
// Notify child to release lock
HRN_FORK_PARENT_NOTIFY_PUT(0);
}