Allow pg-path1 to be optional for synchronous archive-push.

If the WAL path is absolute then pg1-path should be optional but in fact it was required to load pg_control.

Skip the pg_control check when pg1-path is not specified. The check against the stanza version/system-id remains to protect the repo from corruption.

doc/xml/release.xml

@@ -64,6 +64,14 @@
                         <p>WAL prior to the first full backup was previously expired after the first full backup. Now it is preserved according to retention settings.</p>
                     </release-item>
 
+                    <release-item>
+                        <release-item-contributor-list>
+                             <release-item-reviewer id="cynthia.shang"/>
+                        </release-item-contributor-list>
+
+                        <p>Allow <br-option>pg-path1</br-option> to be optional for synchronous <cmd>archive-push</cmd>.</p>
+                    </release-item>
+
                     <release-item>
                         <release-item-contributor-list>
                              <release-item-contributor id="cynthia.shang"/>

src/command/archive/push/push.c

@@ -200,9 +200,10 @@ typedef struct ArchivePushCheckResult
 } ArchivePushCheckResult;
 
 static ArchivePushCheckResult
-archivePushCheck(CipherType cipherType, const String *cipherPass)
+archivePushCheck(bool pgPathSet, CipherType cipherType, const String *cipherPass)
 {
     FUNCTION_LOG_BEGIN(logLevelDebug);
+        FUNCTION_LOG_PARAM(BOOL, pgPathSet);
         FUNCTION_LOG_PARAM(ENUM, cipherType);
         FUNCTION_TEST_PARAM(STRING, cipherPass);
     FUNCTION_LOG_END();
@@ -211,9 +212,6 @@ archivePushCheck(CipherType cipherType, const String *cipherPass)
 
     MEM_CONTEXT_TEMP_BEGIN()
     {
-        // Get info from pg_control
-        PgControl controlInfo = pgControlFromFile(storagePg());
-
         // Attempt to load the archive info file
         InfoArchive *info = infoArchiveLoadFile(storageRepo(), INFO_ARCHIVE_PATH_FILE_STR, cipherType, cipherPass);
 
@@ -221,21 +219,27 @@ archivePushCheck(CipherType cipherType, const String *cipherPass)
         String *archiveId = infoPgArchiveId(infoArchivePg(info), infoPgDataCurrentId(infoArchivePg(info)));
         InfoPgData archiveInfo = infoPgData(infoArchivePg(info), infoPgDataCurrentId(infoArchivePg(info)));
 
-        // Ensure that the version and system identifier match
+        // Ensure that stanza version and system identifier match pg_control when available
-        if (controlInfo.version != archiveInfo.version || controlInfo.systemId != archiveInfo.systemId)
+        if (pgPathSet)
         {
-            THROW_FMT(
+            // Get info from pg_control
-                ArchiveMismatchError,
+            PgControl controlInfo = pgControlFromFile(storagePg());
-                "PostgreSQL version %s, system-id %" PRIu64 " do not match stanza version %s, system-id %" PRIu64
+
-                "\nHINT: are you archiving to the correct stanza?",
+            if (controlInfo.version != archiveInfo.version || controlInfo.systemId != archiveInfo.systemId)
-                strPtr(pgVersionToStr(controlInfo.version)), controlInfo.systemId, strPtr(pgVersionToStr(archiveInfo.version)),
+            {
-                archiveInfo.systemId);
+                THROW_FMT(
+                    ArchiveMismatchError,
+                    "PostgreSQL version %s, system-id %" PRIu64 " do not match stanza version %s, system-id %" PRIu64
+                    "\nHINT: are you archiving to the correct stanza?",
+                    strPtr(pgVersionToStr(controlInfo.version)), controlInfo.systemId, strPtr(pgVersionToStr(archiveInfo.version)),
+                    archiveInfo.systemId);
+            }
         }
 
         MEM_CONTEXT_PRIOR_BEGIN()
         {
-            result.pgVersion = controlInfo.version;
+            result.pgVersion = archiveInfo.version;
-            result.pgSystemId = controlInfo.systemId;
+            result.pgSystemId = archiveInfo.systemId;
             result.archiveId = strDup(archiveId);
             result.archiveCipherPass = strDup(infoArchiveCipherPass(info));
         }
@@ -278,6 +282,10 @@ cmdArchivePush(void)
             bool forked = false;                                        // Has the async process been forked yet?
             bool throwOnError = false;                                  // Should we throw errors?
 
+            // pg1-path is not optional for async mode
+            if (!cfgOptionTest(cfgOptPgPath))
+                THROW(OptionRequiredError, "'" CFGCMD_ARCHIVE_PUSH "' command in async mode requires option '" CFGOPT_PG1_PATH "'");
+
             // Loop and wait for the WAL segment to be pushed
             Wait *wait = waitNew((TimeMSec)(cfgOptionDbl(cfgOptArchiveTimeout) * MSEC_PER_SEC));
 
@@ -341,7 +349,8 @@ cmdArchivePush(void)
 
             // Get archive info
             ArchivePushCheckResult archiveInfo = archivePushCheck(
-                cipherType(cfgOptionStr(cfgOptRepoCipherType)), cfgOptionStrNull(cfgOptRepoCipherPass));
+                cfgOptionTest(cfgOptPgPath), cipherType(cfgOptionStr(cfgOptRepoCipherType)),
+                cfgOptionStrNull(cfgOptRepoCipherPass));
 
             // Check if the push queue has been exceeded
             if (cfgOptionTest(cfgOptArchivePushQueueMax) &&
@@ -484,7 +493,7 @@ cmdArchivePushAsync(void)
 
                 // Get archive info
                 jobData.archiveInfo = archivePushCheck(
-                    cipherType(cfgOptionStr(cfgOptRepoCipherType)), cfgOptionStrNull(cfgOptRepoCipherPass));
+                    true, cipherType(cfgOptionStr(cfgOptRepoCipherType)), cfgOptionStrNull(cfgOptRepoCipherPass));
 
                 // Create the parallel executor
                 ProtocolParallel *parallelExec = protocolParallelNew(

test/src/module/command/archivePushTest.c

@@ -133,7 +133,7 @@ testRun(void)
                 "1={\"db-id\":5555555555555555555,\"db-version\":\"9.4\"}\n"));
 
         TEST_ERROR(
-            archivePushCheck(cipherTypeNone, NULL), ArchiveMismatchError,
+            archivePushCheck(true, cipherTypeNone, NULL), ArchiveMismatchError,
             "PostgreSQL version 9.6, system-id 18072658121562454734 do not match stanza version 9.4, system-id 5555555555555555555"
                 "\nHINT: are you archiving to the correct stanza?");
 
@@ -148,7 +148,7 @@ testRun(void)
                 "1={\"db-id\":5555555555555555555,\"db-version\":\"9.6\"}\n"));
 
         TEST_ERROR(
-            archivePushCheck(cipherTypeNone, NULL), ArchiveMismatchError,
+            archivePushCheck(true, cipherTypeNone, NULL), ArchiveMismatchError,
             "PostgreSQL version 9.6, system-id 18072658121562454734 do not match stanza version 9.6, system-id 5555555555555555555"
                 "\nHINT: are you archiving to the correct stanza?");
 
@@ -163,7 +163,7 @@ testRun(void)
                 "1={\"db-id\":18072658121562454734,\"db-version\":\"9.6\"}\n"));
 
         ArchivePushCheckResult result = {0};
-        TEST_ASSIGN(result, archivePushCheck(cipherTypeNone, NULL), "get archive check result");
+        TEST_ASSIGN(result, archivePushCheck(true, cipherTypeNone, NULL), "get archive check result");
 
         TEST_RESULT_UINT(result.pgVersion, PG_VERSION_96, "check pg version");
         TEST_RESULT_UINT(result.pgSystemId, 0xFACEFACEFACEFACE, "check pg system id");
@@ -290,12 +290,16 @@ testRun(void)
 
         TEST_ERROR(cmdArchivePush(), ArchiveDuplicateError, "WAL file '000000010000000100000001' already exists in the archive");
 
-        // Save it to a new file instead
+        // -------------------------------------------------------------------------------------------------------------------------
-        argListTemp = strLstDup(argList);
+        TEST_TITLE("WAL with absolute path and no pg1-path");
-        strLstAddZ(argListTemp, "pg_wal/000000010000000100000002");
+
+        argListTemp = strLstNew();
+        strLstAddZ(argListTemp, "--" CFGOPT_STANZA "=test");
+        strLstAdd(argListTemp, strNewFmt("--" CFGOPT_REPO1_PATH "=%s/repo", testPath()));
+        strLstAdd(argListTemp, strNewFmt("%s/pg/pg_wal/000000010000000100000002", testPath()));
         harnessCfgLoad(cfgCmdArchivePush, argListTemp);
 
-        storagePutP(storageNewWriteP(storagePgWrite(), strNew("pg_wal/000000010000000100000002")), walBuffer2);
+        TEST_RESULT_VOID(storagePutP(storageNewWriteP(storageTest, strNew("pg/pg_wal/000000010000000100000002")), walBuffer2), "write WAL");
 
         TEST_RESULT_VOID(cmdArchivePush(), "push the WAL segment");
         harnessLogResult("P00   INFO: pushed WAL file '000000010000000100000002' to the archive");
@@ -433,6 +437,18 @@ testRun(void)
 
         TEST_ERROR(cmdArchivePush(), HostInvalidError, "archive-push command must be run on the PostgreSQL host");
 
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("pg1-path must be set when async");
+
+        argList = strLstNew();
+        strLstAddZ(argList, "--" CFGOPT_SPOOL_PATH "=/spool");
+        strLstAddZ(argList, "--" CFGOPT_STANZA "=test2");
+        strLstAddZ(argList, "--" CFGOPT_ARCHIVE_ASYNC);
+        strLstAddZ(argList, "/000000010000000100000001");
+        harnessCfgLoadRole(cfgCmdArchivePush, cfgCmdRoleAsync, argList);
+
+        TEST_ERROR(cmdArchivePush(), OptionRequiredError, "'archive-push' command in async mode requires option 'pg1-path'");
+
         // Call with a bogus exe name so the async process will error out and we can make sure timeouts work
         // -------------------------------------------------------------------------------------------------------------------------
         argList = strLstNew();