diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 8f69071f7..5defd48fc 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -99,6 +99,14 @@

Improve JSON to Variant conversion and add Variant to JSON conversion.

+ + + + + +

Allow NULL stanza in storage helper.

+
+

Allow I/O read interface to explicitly request blocking reads.

diff --git a/src/storage/helper.c b/src/storage/helper.c index 1e6f26632..bf111632c 100644 --- a/src/storage/helper.c +++ b/src/storage/helper.c @@ -17,6 +17,8 @@ Storage path constants STRING_EXTERN(STORAGE_SPOOL_ARCHIVE_IN_STR, STORAGE_SPOOL_ARCHIVE_IN); STRING_EXTERN(STORAGE_SPOOL_ARCHIVE_OUT_STR, STORAGE_SPOOL_ARCHIVE_OUT); +#define STORAGE_PATH_ARCHIVE "archive" + /*********************************************************************************************************************************** Local variables ***********************************************************************************************************************************/ @@ -31,6 +33,7 @@ static struct Storage *storageSpoolWrite; // Spool write storage String *stanza; // Stanza for storage + bool stanzaInit; // Has the stanza been initialized? RegExp *walRegExp; // Regular expression for identifying wal files } storageHelper; @@ -58,19 +61,25 @@ storageHelperInit(void) Initialize the stanza and error if it changes ***********************************************************************************************************************************/ static void -storageHelperStanzaInit(void) +storageHelperStanzaInit(const bool stanzaRequired) { FUNCTION_TEST_VOID(); - if (storageHelper.stanza == NULL) + // If the stanza is NULL and the storage has not already been initialized then initialize the stanza + if (storageHelper.stanza == NULL && !storageHelper.stanzaInit) { + if (stanzaRequired && cfgOptionStr(cfgOptStanza) == NULL) + THROW(AssertError, "stanza cannot be NULL for this storage object"); + MEM_CONTEXT_BEGIN(storageHelper.memContext) { storageHelper.stanza = strDup(cfgOptionStr(cfgOptStanza)); + storageHelper.stanzaInit = true; } MEM_CONTEXT_END(); } - else if (!strEq(storageHelper.stanza, cfgOptionStr(cfgOptStanza))) + else if ((storageHelper.stanza == NULL && cfgOptionStr(cfgOptStanza) != NULL) || + (cfgOptionStr(cfgOptStanza) != NULL && !strEq(storageHelper.stanza, cfgOptionStr(cfgOptStanza)))) { THROW_FMT( AssertError, "stanza has changed from '%s' to '%s'", strPtr(storageHelper.stanza), strPtr(cfgOptionStr(cfgOptStanza))); @@ -130,7 +139,7 @@ storageLocalWrite(void) } /*********************************************************************************************************************************** -Get a spool storage object +Construct a repo path from an expression and path ***********************************************************************************************************************************/ static String * storageRepoPathExpression(const String *expression, const String *path) @@ -146,8 +155,13 @@ storageRepoPathExpression(const String *expression, const String *path) if (strEqZ(expression, STORAGE_REPO_ARCHIVE)) { - result = strNewFmt("archive/%s", strPtr(storageHelper.stanza)); + // Contruct the base path + if (storageHelper.stanza != NULL) + result = strNewFmt(STORAGE_PATH_ARCHIVE "/%s", strPtr(storageHelper.stanza)); + else + result = strNew(STORAGE_PATH_ARCHIVE); + // If a subpath should be appended, determine if it is WAL path, else just append the subpath if (path != NULL) { StringList *pathSplit = strLstNewSplitZ(path, "/"); @@ -219,7 +233,7 @@ storageRepo(void) if (storageHelper.storageRepo == NULL) { storageHelperInit(); - storageHelperStanzaInit(); + storageHelperStanzaInit(false); MEM_CONTEXT_BEGIN(storageHelper.memContext) { @@ -243,6 +257,7 @@ storageSpoolPathExpression(const String *expression, const String *path) FUNCTION_TEST_PARAM(STRING, path); FUNCTION_TEST_ASSERT(expression != NULL); + FUNCTION_TEST_ASSERT(storageHelper.stanza != NULL); FUNCTION_TEST_END(); String *result = NULL; @@ -250,16 +265,16 @@ storageSpoolPathExpression(const String *expression, const String *path) if (strEqZ(expression, STORAGE_SPOOL_ARCHIVE_IN)) { if (path == NULL) - result = strNewFmt("archive/%s/in", strPtr(storageHelper.stanza)); + result = strNewFmt(STORAGE_PATH_ARCHIVE "/%s/in", strPtr(storageHelper.stanza)); else - result = strNewFmt("archive/%s/in/%s", strPtr(storageHelper.stanza), strPtr(path)); + result = strNewFmt(STORAGE_PATH_ARCHIVE "/%s/in/%s", strPtr(storageHelper.stanza), strPtr(path)); } else if (strEqZ(expression, STORAGE_SPOOL_ARCHIVE_OUT)) { if (path == NULL) - result = strNewFmt("archive/%s/out", strPtr(storageHelper.stanza)); + result = strNewFmt(STORAGE_PATH_ARCHIVE "/%s/out", strPtr(storageHelper.stanza)); else - result = strNewFmt("archive/%s/out/%s", strPtr(storageHelper.stanza), strPtr(path)); + result = strNewFmt(STORAGE_PATH_ARCHIVE "/%s/out/%s", strPtr(storageHelper.stanza), strPtr(path)); } else THROW_FMT(AssertError, "invalid expression '%s'", strPtr(expression)); @@ -278,7 +293,7 @@ storageSpool(void) if (storageHelper.storageSpool == NULL) { storageHelperInit(); - storageHelperStanzaInit(); + storageHelperStanzaInit(true); MEM_CONTEXT_BEGIN(storageHelper.memContext) { @@ -304,7 +319,7 @@ storageSpoolWrite(void) if (storageHelper.storageSpoolWrite == NULL) { storageHelperInit(); - storageHelperStanzaInit(); + storageHelperStanzaInit(true); MEM_CONTEXT_BEGIN(storageHelper.memContext) { diff --git a/test/src/module/storage/posixTest.c b/test/src/module/storage/posixTest.c index 4b53ac79f..7119e87b5 100644 --- a/test/src/module/storage/posixTest.c +++ b/test/src/module/storage/posixTest.c @@ -961,7 +961,7 @@ testRun(void) TEST_RESULT_STR( strPtr(storagePathNP(storage, strNew(STORAGE_REPO_ARCHIVE "/9.4-1/000000010000014C0000001A.00000028.backup"))), strPtr(strNewFmt("%s/archive/db/9.4-1/000000010000014C/000000010000014C0000001A.00000028.backup", testPath())), - "check backup path"); + "check archive backup path"); // Change the stanza name and make sure helper fails // ------------------------------------------------------------------------------------------------------------------------- @@ -975,6 +975,46 @@ testRun(void) harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); TEST_ERROR(storageRepo(), AssertError, "stanza has changed from 'db' to 'other'"); + + // Change the stanza to NULL with the stanzaInit flag still true and make sure helper fails + // ------------------------------------------------------------------------------------------------------------------------- + storageHelper.storageRepo = NULL; + storageHelper.stanza = NULL; + TEST_RESULT_BOOL(storageHelper.stanzaInit, true, "stanza initialized"); + + argList = strLstNew(); + strLstAddZ(argList, "pgbackrest"); + strLstAddZ(argList, "--stanza=other"); + strLstAdd(argList, strNewFmt("--repo-path=%s", testPath())); + strLstAddZ(argList, "archive-get"); + harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); + + TEST_ERROR(storageRepo(), AssertError, "stanza has changed from '(null)' to 'other'"); + + // Change the stanza to NULL with the stanzaInit flag still true, make sure helper does not fail when stanza option not set + // ------------------------------------------------------------------------------------------------------------------------- + storageHelper.storageRepo = NULL; + storageHelper.stanza = NULL; + TEST_RESULT_BOOL(storageHelper.stanzaInit, true, "stanza initialized"); + + argList = strLstNew(); + strLstAddZ(argList, "pgbackrest"); + strLstAdd(argList, strNewFmt("--repo-path=%s", testPath())); + strLstAddZ(argList, "info"); + harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); + + TEST_ASSIGN(storage, storageRepo(), "new repo storage no stanza"); + TEST_RESULT_PTR(storageHelper.stanza, NULL, "stanza NULL"); + + TEST_RESULT_STR( + strPtr(storagePathNP(storage, strNew(STORAGE_REPO_ARCHIVE))), strPtr(strNewFmt("%s/archive", testPath())), + "check archive path - NULL stanza"); + TEST_RESULT_STR( + strPtr(storagePathNP(storage, strNew(STORAGE_REPO_ARCHIVE "/simple"))), + strPtr(strNewFmt("%s/archive/simple", testPath())), "check simple archive path - NULL stanza"); + + // Reset init flag + storageHelper.stanzaInit = false; } // ***************************************************************************************************************************** @@ -1023,6 +1063,22 @@ testRun(void) TEST_RESULT_PTR(storageSpoolWrite(), storage, "get cached storage"); TEST_RESULT_VOID(storageNewWriteNP(storage, writeFile), "writes are allowed"); + + // Change the stanza to NULL, stanzaInit flag to false and make sure helper fails because stanza is required + // ------------------------------------------------------------------------------------------------------------------------- + storageHelper.storageSpool = NULL; + storageHelper.storageSpoolWrite = NULL; + storageHelper.stanzaInit = false; + storageHelper.stanza = NULL; + + argList = strLstNew(); + strLstAddZ(argList, "pgbackrest"); + strLstAdd(argList, strNewFmt("--repo-path=%s", testPath())); + strLstAddZ(argList, "info"); + harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); + + TEST_ERROR(storageSpool(), AssertError, "stanza cannot be NULL for this storage object"); + TEST_ERROR(storageSpoolWrite(), AssertError, "stanza cannot be NULL for this storage object"); } FUNCTION_HARNESS_RESULT_VOID();