db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-14 10:13:05 +02:00
Code Issues Releases Activity

Cleanup local/remote protocol interaction from 9367cc46.

Browse Source 
The command option was not being set correctly when a remote was started from a local.  It was being set as 'local' rather than the command that the local was running as.

Also automatically select the remote protocol id based on whether it is started from a local (use the local protocol id) or from the main process (use 0).

These were not live issues but could cause strange behaviors as new features are added that might be hard to diagnose.
This commit is contained in:
David Steele 2019-02-28 09:51:19 +02:00
parent db4b447be8
commit 0913523096
7 changed files with 55 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/main.c
View File

@ -75,8 +75,7 @@ main(int argListSize, const char *argList[])
else if (cfgCommand() == cfgCmdRemote &&
(strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdArchiveGet)) ||
strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdArchiveGetAsync)) ||
strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdInfo)) ||
strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdLocal))))
strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdInfo))))
{
cmdRemote(STDIN_FILENO, STDOUT_FILENO);
}

22
src/protocol/helper.c
View File

@ -88,7 +88,7 @@ protocolLocalParam(ProtocolStorageType protocolStorageType, unsigned int protoco
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptCommand))), varNewStr(strNew(cfgCommandName(cfgCommand()))));
// Add the process id -- used when more than one process will be called
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptProcess))), varNewInt(0));
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptProcess))), varNewInt((int)protocolId));
// Add the host id -- for now this is hard-coded to 1
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptHostId))), varNewInt(1));
@ -202,11 +202,13 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int protoc
if (cfgOptionSource(cfgOptRepoHostConfigPath) != cfgSourceDefault)
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptConfigPath))), cfgOption(cfgOptRepoHostConfigPath));
// Add the command option
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptCommand))), varNewStr(strNew(cfgCommandName(cfgCommand()))));
// Add the command option (or use the current command option if it is valid)
if (!cfgOptionTest(cfgOptCommand))
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptCommand))), varNewStr(strNew(cfgCommandName(cfgCommand()))));
// Add the process id -- used when more than one process will be called
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptProcess))), varNewInt(0));
// Add the process id (or use the current process id if it is valid)
if (!cfgOptionTest(cfgOptProcess))
kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptProcess))), varNewInt((int)protocolId));
// Don't pass the stanza if it is set. It is better if the remote is stanza-agnostic so the client can operate on multiple
// stanzas without starting a new remote. Once the Perl code is removed the stanza option can be removed from the remote
@ -227,11 +229,10 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int protoc
Get the remote protocol client
***********************************************************************************************************************************/
ProtocolClient *
protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int protocolId)
protocolRemoteGet(ProtocolStorageType protocolStorageType)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(ENUM, protocolStorageType);
FUNCTION_LOG_PARAM(UINT, protocolId);
FUNCTION_LOG_END();
protocolHelperInit();
@ -253,6 +254,13 @@ protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int protocol
MEM_CONTEXT_END();
}
// Determine protocol id for the remote. If the process option is set then use that since we want to remote protocol id to
// match the local protocol id. Otherwise set to 0 since the remote is being started from a main process.
unsigned int protocolId = 0;
if (cfgOptionTest(cfgOptProcess))
protocolId = (unsigned int)cfgOptionInt(cfgOptProcess);
ASSERT(protocolId < protocolHelper.clientRemoteSize);
// Create protocol object

2
src/protocol/helper.h
View File

@ -27,7 +27,7 @@ Constants
Functions
***********************************************************************************************************************************/
ProtocolClient *protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int protocolId);
ProtocolClient *protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int protocolId);
ProtocolClient *protocolRemoteGet(ProtocolStorageType protocolStorageType);
/***********************************************************************************************************************************
Getters

2
src/storage/helper.c
View File

@ -210,7 +210,7 @@ storageRepoGet(const String *type, bool write)
result = storageDriverRemoteInterface(
storageDriverRemoteNew(
STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, write, storageRepoPathExpression,
protocolRemoteGet(protocolStorageTypeRepo, 1)));
protocolRemoteGet(protocolStorageTypeRepo)));
}
// For now treat posix and cifs drivers as if they are the same. This won't be true once the repository storage becomes
// writable but for now it's OK. The assertion above should pop if we try to create writable repo storage.

4
test/expect/mock-archive-002.log
View File

@ -14,8 +14,8 @@ P00 INFO: archive-push command end: aborted with exception [055]
------------------------------------------------------------------------------------------------------------------------------------
P00 INFO: archive-get command begin [BACKREST-VERSION]: [000000010000000100000001, [TEST_PATH]/db-master/db/base/pg_xlog/RECOVERYXLOG] --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-host=backup --repo1-host-cmd=[BACKREST-BIN] --repo1-host-config=[TEST_PATH]/backup/pgbackrest.conf --repo1-host-user=[USER-1] --stanza=db
P00 ERROR: [055]: unable to load info file 'archive/db/archive.info' or 'archive/db/archive.info.copy':
FileMissingError: raised from remote-1 protocol on 'backup': unable to open '/archive/db/archive.info': No such file or directory
FileMissingError: raised from remote-1 protocol on 'backup': unable to open '/archive/db/archive.info.copy': No such file or directory
FileMissingError: raised from remote-0 protocol on 'backup': unable to open '/archive/db/archive.info': No such file or directory
FileMissingError: raised from remote-0 protocol on 'backup': unable to open '/archive/db/archive.info.copy': No such file or directory
HINT: archive.info cannot be opened but is required to push/get WAL segments.
HINT: is archive_command configured correctly in postgresql.conf?
HINT: has a stanza-create been performed?

42
test/src/module/protocol/protocolTest.c
View File

@ -87,7 +87,7 @@ testRun(void)
}
// *****************************************************************************************************************************
if (testBegin("protocolParam()"))
if (testBegin("protocolRemoteParam()"))
{
StringList *argList = strLstNew();
strLstAddZ(argList, "pgbackrest");
@ -98,7 +98,7 @@ testRun(void)
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
TEST_RESULT_STR(
strPtr(strLstJoin(protocolRemoteParam(protocolStorageTypeRepo, 1), "|")),
strPtr(strLstJoin(protocolRemoteParam(protocolStorageTypeRepo, 0), "|")),
strPtr(
strNew(
"-o|LogLevel=error|-o|Compression=no|-o|PasswordAuthentication=no|repo-host-user@repo-host"
@ -124,8 +124,28 @@ testRun(void)
strNew(
"-o|LogLevel=error|-o|Compression=no|-o|PasswordAuthentication=no|-p|444|repo-host-user@repo-host"
"|pgbackrest --command=archive-get --config=/path/pgbackrest.conf --config-include-path=/path/include"
" --config-path=/path/config --process=0 --type=backup remote")),
" --config-path=/path/config --process=1 --type=backup remote")),
"remote protocol params with replacements");
// -------------------------------------------------------------------------------------------------------------------------
argList = strLstNew();
strLstAddZ(argList, "pgbackrest");
strLstAddZ(argList, "--stanza=test1");
strLstAddZ(argList, "--command=archive-get");
strLstAddZ(argList, "--process=3");
strLstAddZ(argList, "--host-id=1");
strLstAddZ(argList, "--type=db");
strLstAddZ(argList, "--repo1-host=repo-host");
strLstAddZ(argList, "local");
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
TEST_RESULT_STR(
strPtr(strLstJoin(protocolRemoteParam(protocolStorageTypeRepo, 66), "|")),
strPtr(
strNew(
"-o|LogLevel=error|-o|Compression=no|-o|PasswordAuthentication=no|pgbackrest@repo-host"
"|pgbackrest --command=archive-get --process=3 --type=backup remote")),
"remote protocol params for local");
}
// *****************************************************************************************************************************
@ -586,8 +606,9 @@ testRun(void)
TEST_RESULT_VOID(protocolFree(), "free protocol objects before anything has been created");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 1), "get remote protocol");
TEST_RESULT_PTR(protocolRemoteGet(protocolStorageTypeRepo, 1), client, "get remote cached protocol");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo), "get remote protocol");
TEST_RESULT_PTR(protocolRemoteGet(protocolStorageTypeRepo), client, "get remote cached protocol");
TEST_RESULT_PTR(protocolHelper.clientRemote[0].client, client, "check position in cache");
TEST_RESULT_VOID(protocolFree(), "free remote protocol objects");
TEST_RESULT_VOID(protocolFree(), "free remote protocol objects again");
@ -608,11 +629,16 @@ testRun(void)
strLstAdd(argList, strNewFmt("--config=%s/pgbackrest.conf", testPath()));
strLstAddZ(argList, "--repo1-host=localhost");
strLstAdd(argList, strNewFmt("--repo1-path=%s", testPath()));
strLstAddZ(argList, "info");
strLstAddZ(argList, "--process=4");
strLstAddZ(argList, "--command=archive-get");
strLstAddZ(argList, "--host-id=1");
strLstAddZ(argList, "--type=db");
strLstAddZ(argList, "local");
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "acbd", "check cipher pass before");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 1), "get remote protocol");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo), "get remote protocol");
TEST_RESULT_PTR(protocolHelper.clientRemote[4].client, client, "check position in cache");
TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "acbd", "check cipher pass after");
TEST_RESULT_VOID(protocolFree(), "free remote protocol objects");
@ -638,7 +664,7 @@ testRun(void)
harnessCfgLoad(strLstSize(argList), strLstPtr(argList));
TEST_RESULT_PTR(cfgOptionStr(cfgOptRepoCipherPass), NULL, "check cipher pass before");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 1), "get remote protocol");
TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo), "get remote protocol");
TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "dcba", "check cipher pass after");
// Start local protocol

2
test/src/module/storage/remoteTest.c
View File

@ -117,7 +117,7 @@ testRun(void)
strPtr(strNewBuf(storageGetNP(storageNewReadNP(storageRemote, strNew("test.txt"))))), FileMissingError,
strPtr(
strNewFmt(
"raised from remote-1 protocol on 'localhost': unable to open '%s/repo/test.txt' for read:"
"raised from remote-0 protocol on 'localhost': unable to open '%s/repo/test.txt' for read:"
" [2] No such file or directory",
testPath())));