diff --git a/doc/xml/release.xml b/doc/xml/release.xml index bc4cfb95c..4cd1a46d7 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -15,6 +15,17 @@ + + + + + + +

Fix archive-push/archive-get when PGDATA is symlinked.

+ +

These commands tried to use cwd() as PGDATA but this would disagree with the path configured in pgBackRest if PGDATA was symlinked. If cwd() does not match the path then chdir() to the path and make sure the next cwd() matches the result from the first call.

+ +

Fix reference list when backup.info is reconstructed in expire command.

@@ -7809,6 +7820,11 @@ M1hacka + + Milosz Suchy + Yuxael + + Mohamad El-Rifai melrifa1 diff --git a/src/command/archive/common.c b/src/command/archive/common.c index a3c9908b2..a13660f15 100644 --- a/src/command/archive/common.c +++ b/src/command/archive/common.c @@ -229,25 +229,56 @@ walIsPartial(const String *walSegment) } /*********************************************************************************************************************************** -Generates the location of the wal directory using either an absolute path or cwd() and a relative path +Generates the location of the wal directory using a relative wal path and the supplied pg path ***********************************************************************************************************************************/ String * -walPath(const String *walFile) +walPath(const String *walFile, const String *pgPath, const String *command) { FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STRING, walFile); + FUNCTION_LOG_PARAM(STRING, pgPath); + FUNCTION_LOG_PARAM(STRING, command); FUNCTION_LOG_END(); ASSERT(walFile != NULL); + ASSERT(command != NULL); String *result = NULL; if (!strBeginsWithZ(walFile, "/")) { - char currentWorkDir[4096]; + // Error if walFile has a relative path and pgPath is not set + if (pgPath == NULL) + { + THROW_FMT( + OptionRequiredError, + "option '" CFGOPT_PG1_PATH "' must be specified when relative wal paths are used\n" + "HINT: is %%f passed to %s instead of %%p?\n" + "HINT: PostgreSQL may pass relative paths even with %%p depending on the environment.", + strPtr(command)); + } + // Get the working directory + char currentWorkDir[4096]; THROW_ON_SYS_ERROR(getcwd(currentWorkDir, sizeof(currentWorkDir)) == NULL, FormatError, "unable to get cwd"); - result = strNewFmt("%s/%s", strcmp(currentWorkDir, "/") != 0 ? currentWorkDir : "", strPtr(walFile)); + + // Check if the working directory is the same as pgPath + if (!strEqZ(pgPath, currentWorkDir)) + { + // If not we'll change the working directory to pgPath and see if that equals the working directory we got called with + THROW_ON_SYS_ERROR_FMT(chdir(strPtr(pgPath)) != 0, PathMissingError, "unable to chdir() to '%s'", strPtr(pgPath)); + + // Get the new working directory + char newWorkDir[4096]; + THROW_ON_SYS_ERROR(getcwd(newWorkDir, sizeof(newWorkDir)) == NULL, FormatError, "unable to get cwd"); + + // Error if the new working directory is not equal to the original current working directory. This means that + // PostgreSQL and pgBackrest have a different idea about where the PostgreSQL data directory is located. + if (strcmp(currentWorkDir, newWorkDir) != 0) + THROW_FMT(AssertError, "working path '%s' is not the same path as '%s'", currentWorkDir, strPtr(pgPath)); + } + + result = strNewFmt("%s/%s", strPtr(pgPath), strPtr(walFile)); } else result = strDup(walFile); diff --git a/src/command/archive/common.h b/src/command/archive/common.h index 25afd1a7b..394c17683 100644 --- a/src/command/archive/common.h +++ b/src/command/archive/common.h @@ -64,7 +64,7 @@ void archiveAsyncStatusErrorWrite(ArchiveMode archiveMode, const String *walSegm bool walIsPartial(const String *walSegment); bool walIsSegment(const String *walSegment); -String *walPath(const String *walFile); +String *walPath(const String *walFile, const String *pgPath, const String *command); String *walSegmentFind(const Storage *storage, const String *archiveId, const String *walSegment, TimeMSec timeout); String *walSegmentNext(const String *walSegment, size_t walSegmentSize, unsigned int pgVersion); StringList *walSegmentRange(const String *walSegmentBegin, size_t walSegmentSize, unsigned int pgVersion, unsigned int range); diff --git a/src/command/archive/get/get.c b/src/command/archive/get/get.c index 8e7954a07..f518ee931 100644 --- a/src/command/archive/get/get.c +++ b/src/command/archive/get/get.c @@ -132,7 +132,8 @@ cmdArchiveGet(void) String *walSegment = strBase(strLstGet(commandParam, 0)); // Destination is wherever we were told to move the WAL segment - const String *walDestination = walPath(strLstGet(commandParam, 1)); + const String *walDestination = + walPath(strLstGet(commandParam, 1), cfgOptionStr(cfgOptPgPath), STR(cfgCommandName(cfgCommand()))); // Async get can only be performed on WAL segments, history or other files must use synchronous mode if (cfgOptionBool(cfgOptArchiveAsync) && walIsSegment(walSegment)) diff --git a/src/command/archive/push/push.c b/src/command/archive/push/push.c index 279dbec77..6df02d71d 100644 --- a/src/command/archive/push/push.c +++ b/src/command/archive/push/push.c @@ -268,7 +268,7 @@ cmdArchivePush(void) lockStopTest(); // Get the segment name - String *walFile = walPath(strLstGet(commandParam, 0)); + String *walFile = walPath(strLstGet(commandParam, 0), cfgOptionStr(cfgOptPgPath), STR(cfgCommandName(cfgCommand()))); String *archiveFile = strBase(walFile); if (cfgOptionBool(cfgOptArchiveAsync)) diff --git a/test/src/module/command/archiveCommonTest.c b/test/src/module/command/archiveCommonTest.c index 5da809cd5..bfab201b5 100644 --- a/test/src/module/command/archiveCommonTest.c +++ b/test/src/module/command/archiveCommonTest.c @@ -172,14 +172,39 @@ testRun(void) // ***************************************************************************************************************************** if (testBegin("walPath()")) { - THROW_ON_SYS_ERROR(chdir("/tmp") != 0, PathMissingError, "unable to chdir()"); + const String *pgPath = storagePathP(storageTest, STRDEF("pg")); + storagePathCreateP(storageTest, pgPath); + + TEST_RESULT_STR( + strPtr(walPath(strNew("/absolute/path"), pgPath, strNew("test"))), "/absolute/path", "absolute path"); + + THROW_ON_SYS_ERROR(chdir(strPtr(pgPath)) != 0, PathMissingError, "unable to chdir()"); + TEST_RESULT_STR_STR( + walPath(strNew("relative/path"), pgPath, strNew("test")), strNewFmt("%s/relative/path", strPtr(pgPath)), + "relative path"); + + + const String *pgPathLink = storagePathP(storageTest, STRDEF("pg-link")); + THROW_ON_SYS_ERROR_FMT( + symlink(strPtr(pgPath), strPtr(pgPathLink)) == -1, FileOpenError, + "unable to create symlink '%s' to '%s'", strPtr(pgPath), strPtr(pgPathLink)); + + THROW_ON_SYS_ERROR(chdir(strPtr(pgPath)) != 0, PathMissingError, "unable to chdir()"); + TEST_RESULT_STR_STR( + walPath(strNew("relative/path"), pgPathLink, strNew("test")), strNewFmt("%s/relative/path", strPtr(pgPathLink)), + "relative path"); - TEST_RESULT_STR(strPtr(walPath(strNew("/absolute/path"))), "/absolute/path", "absolute path"); - TEST_RESULT_STR(strPtr(walPath(strNew("relative/path"))), "/tmp/relative/path", "relative path"); THROW_ON_SYS_ERROR(chdir("/") != 0, PathMissingError, "unable to chdir()"); + TEST_ERROR( + walPath(strNew("relative/path"), pgPathLink, strNew("test")), AssertError, + hrnReplaceKey("working path '/' is not the same path as '{[path]}/pg-link'")); - TEST_RESULT_STR(strPtr(walPath(strNew("relative/path"))), "/relative/path", "relative path"); + TEST_ERROR( + walPath(strNew("relative/path"), NULL, strNew("test")), OptionRequiredError, + "option 'pg1-path' must be specified when relative wal paths are used\n" + "HINT: is %f passed to test instead of %p?\n" + "HINT: PostgreSQL may pass relative paths even with %p depending on the environment."); } // ***************************************************************************************************************************** diff --git a/test/src/module/command/archiveGetTest.c b/test/src/module/command/archiveGetTest.c index 7c9211073..8890cfadd 100644 --- a/test/src/module/command/archiveGetTest.c +++ b/test/src/module/command/archiveGetTest.c @@ -588,6 +588,8 @@ testRun(void) strLstAdd(argList, strNewFmt("--pg1-path=%s/db", testPath())); harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList)); + THROW_ON_SYS_ERROR(chdir(strPtr(cfgOptionStr(cfgOptPgPath))) != 0, PathMissingError, "unable to chdir()"); + HARNESS_FORK_BEGIN() { HARNESS_FORK_CHILD_BEGIN(0, false) @@ -623,8 +625,6 @@ testRun(void) // Write out a WAL segment for success // ------------------------------------------------------------------------------------------------------------------------- - THROW_ON_SYS_ERROR(chdir(strPtr(cfgOptionStr(cfgOptPgPath))) != 0, PathMissingError, "unable to chdir()"); - storagePutP( storageNewWriteP(storageSpoolWrite(), strNewFmt(STORAGE_SPOOL_ARCHIVE_IN "/%s", strPtr(walSegment))), BUFSTRDEF("SHOULD-BE-A-REAL-WAL-FILE")); diff --git a/test/src/module/command/archivePushTest.c b/test/src/module/command/archivePushTest.c index 9f2fddd99..af81175b6 100644 --- a/test/src/module/command/archivePushTest.c +++ b/test/src/module/command/archivePushTest.c @@ -181,12 +181,23 @@ testRun(void) TEST_ERROR(cmdArchivePush(), ParamRequiredError, "WAL segment to push required"); + // ------------------------------------------------------------------------------------------------------------------------- + StringList *argListTemp = strLstDup(argList); + strLstAddZ(argListTemp, "pg_wal/000000010000000100000001"); + harnessCfgLoad(cfgCmdArchivePush, argListTemp); + + TEST_ERROR( + cmdArchivePush(), OptionRequiredError, + "option 'pg1-path' must be specified when relative wal paths are used" + "\nHINT: is %f passed to archive-push instead of %p?" + "\nHINT: PostgreSQL may pass relative paths even with %p depending on the environment."); + // Create pg_control and archive.info // ------------------------------------------------------------------------------------------------------------------------- strLstAdd(argList, strNewFmt("--pg1-path=%s/pg", testPath())); strLstAdd(argList, strNewFmt("--repo1-path=%s/repo", testPath())); - StringList *argListTemp = strLstDup(argList); + argListTemp = strLstDup(argList); strLstAddZ(argListTemp, "pg_wal/000000010000000100000001"); harnessCfgLoad(cfgCmdArchivePush, argListTemp); @@ -415,7 +426,8 @@ testRun(void) strLstAddZ(argList, "pg_wal/bogus"); harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList)); - THROW_ON_SYS_ERROR(chdir(testPath()) != 0, PathMissingError, "unable to chdir()"); + storagePathCreateP(storageTest, cfgOptionStr(cfgOptPgPath)); + THROW_ON_SYS_ERROR(chdir(strPtr(cfgOptionStr(cfgOptPgPath))) != 0, PathMissingError, "unable to chdir()"); TEST_ERROR( cmdArchivePush(), ArchiveTimeoutError,