Skip to content

Commit

Permalink
Error when archive-get/archive-push/restore are not run on a PostgreS…
Browse files Browse the repository at this point in the history
…QL host.

This error was lost during the migration to C.  The error that occurred instead (generally an SSH auth error) was hard to debug.

Restore the original behavior by throwing an error immediately if pg1-host is configured for any of these commands.  reset-pg1-host can be used to suppress the error when required.
  • Loading branch information
dwsteele committed Feb 13, 2020
1 parent dac8119 commit 6353e94
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 2 deletions.
9 changes: 9 additions & 0 deletions doc/xml/release.xml
Expand Up @@ -25,6 +25,15 @@
<p>Prevent defunct processes in asynchronous archive commands.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-ideator id="jesper.st.john"/>
<release-item-reviewer id="stephen.frost"/>
</release-item-contributor-list>

<p>Error when <cmd>archive-get</cmd>/<cmd>archive-push</cmd>/<cmd>restore</cmd> are not run on a <postgres/> host.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-ideator id="christian.roux"/>
Expand Down
6 changes: 6 additions & 0 deletions src/command/archive/get/get.c
Expand Up @@ -107,6 +107,9 @@ cmdArchiveGet(void)
{
FUNCTION_LOG_VOID(logLevelDebug);

// PostgreSQL must be local
pgIsLocalVerify();

// Set the result assuming the archive file will not be found
int result = 1;

Expand Down Expand Up @@ -315,6 +318,9 @@ cmdArchiveGetAsync(void)
{
FUNCTION_LOG_VOID(logLevelDebug);

// PostgreSQL must be local
pgIsLocalVerify();

MEM_CONTEXT_TEMP_BEGIN()
{
TRY_BEGIN()
Expand Down
6 changes: 6 additions & 0 deletions src/command/archive/push/push.c
Expand Up @@ -257,6 +257,9 @@ cmdArchivePush(void)

ASSERT(cfgCommand() == cfgCmdArchivePush);

// PostgreSQL must be local
pgIsLocalVerify();

MEM_CONTEXT_TEMP_BEGIN()
{
// Make sure there is a parameter to retrieve the WAL segment from
Expand Down Expand Up @@ -428,6 +431,9 @@ cmdArchivePushAsync(void)

ASSERT(cfgCommand() == cfgCmdArchivePush && cfgCommandRole() == cfgCmdRoleAsync);

// PostgreSQL must be local
pgIsLocalVerify();

MEM_CONTEXT_TEMP_BEGIN()
{
// Make sure there is a parameter with the wal path
Expand Down
31 changes: 30 additions & 1 deletion test/src/module/command/archiveGetTest.c
Expand Up @@ -330,6 +330,21 @@ testRun(void)

TEST_ERROR(cmdArchiveGetAsync(), ParamInvalidError, "at least one wal segment is required");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("command must be run on the pg host");

StringList *argList = strLstNew();
strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host");
strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg");
strLstAddZ(argList, "--" CFGOPT_REPO1_PATH "=/repo");
strLstAddZ(argList, "--" CFGOPT_SPOOL_PATH "=/spool");
strLstAddZ(argList, "--" CFGOPT_ARCHIVE_ASYNC);
strLstAddZ(argList, "--" CFGOPT_STANZA "=test2");
strLstAddZ(argList, "000000010000000100000001");
harnessCfgLoadRole(cfgCmdArchiveGet, cfgCmdRoleAsync, argList);

TEST_ERROR(cmdArchiveGetAsync(), HostInvalidError, "archive-get command must be run on the PostgreSQL host");

// Create pg_control file and archive.info
// -------------------------------------------------------------------------------------------------------------------------
storagePutP(
Expand All @@ -347,7 +362,7 @@ testRun(void)

// Get a single segment
// -------------------------------------------------------------------------------------------------------------------------
StringList *argList = strLstDup(argCleanList);
argList = strLstDup(argCleanList);
strLstAddZ(argList, "000000010000000100000001");
harnessCfgLoadRole(cfgCmdArchiveGet, cfgCmdRoleAsync, argList);

Expand Down Expand Up @@ -474,7 +489,21 @@ testRun(void)
// *****************************************************************************************************************************
if (testBegin("cmdArchiveGet()"))
{
TEST_TITLE("command must be run on the pg host");

StringList *argList = strLstNew();
strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host");
strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg");
strLstAddZ(argList, "--" CFGOPT_REPO1_PATH "=/repo");
strLstAddZ(argList, "--" CFGOPT_STANZA "=test2");
strLstAddZ(argList, "000000010000000100000001");
strLstAddZ(argList, "pg_wal/000000010000000100000001");
harnessCfgLoadRole(cfgCmdArchiveGet, cfgCmdRoleDefault, argList);

TEST_ERROR(cmdArchiveGet(), HostInvalidError, "archive-get command must be run on the PostgreSQL host");

// -------------------------------------------------------------------------------------------------------------------------
argList = strLstNew();
strLstAddZ(argList, "pgbackrest-bogus"); // Break this until async tests are setup correctly
strLstAddZ(argList, "--archive-timeout=1");
strLstAdd(argList, strNewFmt("--lock-path=%s/lock", testPath()));
Expand Down
25 changes: 24 additions & 1 deletion test/src/module/command/archivePushTest.c
Expand Up @@ -176,7 +176,18 @@ testRun(void)
// *****************************************************************************************************************************
if (testBegin("Synchronous cmdArchivePush(), archivePushFile() and archivePushProtocol()"))
{
TEST_TITLE("command must be run on the pg host");

StringList *argList = strLstNew();
strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host");
strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg");
strLstAddZ(argList, "--" CFGOPT_STANZA "=test2");
harnessCfgLoadRole(cfgCmdArchivePush, cfgCmdRoleDefault, argList);

TEST_ERROR(cmdArchivePush(), HostInvalidError, "archive-push command must be run on the PostgreSQL host");

// -------------------------------------------------------------------------------------------------------------------------
argList = strLstNew();
strLstAddZ(argList, "--stanza=test");
harnessCfgLoad(cfgCmdArchivePush, argList);

Expand Down Expand Up @@ -412,9 +423,21 @@ testRun(void)
{
harnessLogLevelSet(logLevelDetail);

TEST_TITLE("command must be run on the pg host");

StringList *argList = strLstNew();
strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host");
strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg");
strLstAddZ(argList, "--" CFGOPT_SPOOL_PATH "=/spool");
strLstAddZ(argList, "--" CFGOPT_STANZA "=test2");
strLstAddZ(argList, "--" CFGOPT_ARCHIVE_ASYNC);
harnessCfgLoadRole(cfgCmdArchivePush, cfgCmdRoleAsync, argList);

TEST_ERROR(cmdArchivePush(), HostInvalidError, "archive-push command must be run on the PostgreSQL host");

// Call with a bogus exe name so the async process will error out and we can make sure timeouts work
// -------------------------------------------------------------------------------------------------------------------------
StringList *argList = strLstNew();
argList = strLstNew();
strLstAddZ(argList, "pgbackrest-bogus");
strLstAddZ(argList, "--stanza=test");
strLstAddZ(argList, "--archive-async");
Expand Down

0 comments on commit 6353e94

Please sign in to comment.