From 266c9ddcc12c377dd2b3d90b79468eaa6c22f101 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 19 Sep 2017 10:14:18 -0400 Subject: [PATCH] Fixed an issue where some db-* options (e.g. db-port) were not being passed to remotes. --- doc/xml/release.xml | 4 +++ lib/pgBackRest/Protocol/Helper.pm | 30 +++++++++++++++---- test/expect/mock-all-002.log | 12 ++++---- test/expect/mock-all-003.log | 2 +- .../pgBackRestTest/Module/Mock/MockAllTest.pm | 12 ++++++-- 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 75f28fbad..2489160d7 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -16,6 +16,10 @@

Fixed an issue where warnings were being emitted in place of lower priority log messages during backup from standby initialization.

 + + +

Fixed an issue where some db-* options (e.g. db-port) were not being passed to remotes.

+ diff --git a/lib/pgBackRest/Protocol/Helper.pm b/lib/pgBackRest/Protocol/Helper.pm index 63b4dd4fc..76cea3776 100644 --- a/lib/pgBackRest/Protocol/Helper.pm +++ b/lib/pgBackRest/Protocol/Helper.pm @@ -203,6 +203,7 @@ sub protocolGet my $iOptionIdConfig = CFGOPT_BACKUP_CONFIG; my $iOptionIdHost = CFGOPT_BACKUP_HOST; my $iOptionIdUser = CFGOPT_BACKUP_USER; + my $strOptionDbPath = undef; my $strOptionDbPort = undef; my $strOptionDbSocketPath = undef; my $strOptionSshPort = CFGOPT_BACKUP_SSH_PORT; @@ -216,7 +217,15 @@ sub protocolGet $strOptionSshPort = cfgOptionIndex(CFGOPT_DB_SSH_PORT, $iRemoteIdx); } - # Db socket is not valid in all contexts (restore, for instance) + # Db path is not valid in all contexts (restore, for instance) + if (cfgOptionValid(cfgOptionIndex(CFGOPT_DB_PATH, $iRemoteIdx))) + { + $strOptionDbPath = + cfgOptionSource(cfgOptionIndex(CFGOPT_DB_PATH, $iRemoteIdx)) eq CFGDEF_SOURCE_DEFAULT ? + undef : cfgOption(cfgOptionIndex(CFGOPT_DB_PATH, $iRemoteIdx)); + } + + # Db port is not valid in all contexts (restore, for instance) if (cfgOptionValid(cfgOptionIndex(CFGOPT_DB_PORT, $iRemoteIdx))) { $strOptionDbPort = @@ -248,6 +257,7 @@ sub protocolGet # option will be set to 'protocol' which is not a valid value from the command line. &CFGOPT_LOG_LEVEL_STDERR => {}, + &CFGOPT_DB_PATH => {value => $strOptionDbPath}, &CFGOPT_DB_PORT => {value => $strOptionDbPort}, &CFGOPT_DB_SOCKET_PATH => {value => $strOptionDbSocketPath}, @@ -259,11 +269,21 @@ sub protocolGet }; # Override some per-db options that shouldn't be passed to the command. ??? This could be done better as a new rule for - # these options which would then implemented in cfgCommandWrite(). - for (my $iOptionIdx = 1; $iOptionIdx < cfgOptionIndexTotal(CFGOPT_DB_HOST); $iOptionIdx++) + # these options which would then be implemented in cfgCommandWrite(). + for (my $iOptionIdx = 1; $iOptionIdx <= cfgOptionIndexTotal(CFGOPT_DB_HOST); $iOptionIdx++) { - $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_PORT, $iOptionIdx)} = {}; - $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_SOCKET_PATH, $iOptionIdx)} = {}; + if ($iOptionIdx != 1) + { + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_CONFIG, $iOptionIdx)} = {}; + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_HOST, $iOptionIdx)} = {}; + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_PATH, $iOptionIdx)} = {}; + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_PORT, $iOptionIdx)} = {}; + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_SOCKET_PATH, $iOptionIdx)} = {}; + } + + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_CMD, $iOptionIdx)} = {}; + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_USER, $iOptionIdx)} = {}; + $rhCommandOption->{cfgOptionIndex(CFGOPT_DB_SSH_PORT, $iOptionIdx)} = {}; } $oProtocol = new pgBackRest::Protocol::Remote::Master diff --git a/test/expect/mock-all-002.log b/test/expect/mock-all-002.log index 42d3f3d8f..56f889996 100644 --- a/test/expect/mock-all-002.log +++ b/test/expect/mock-all-002.log @@ -68,9 +68,9 @@ P00 ERROR: [070]: link '[TEST_PATH]/db-master/db/base/postgresql.conf.bad' -> ' P00 INFO: backup command end: aborted with exception [070] full backup - create pg_stat link, pg_clog dir (backup host) -> [CONTAINER-EXEC] backup [BACKREST-BIN] --config=[TEST_PATH]/backup/pgbackrest.conf --no-online --manifest-save-threshold=3 --protocol-timeout=2 --db-timeout=1 --cmd-ssh=/usr/bin/ssh --buffer-size=16384 --checksum-page --process-max=1 --repo-type=cifs --type=full --stanza=db backup +> [CONTAINER-EXEC] backup [BACKREST-BIN] --config=[TEST_PATH]/backup/pgbackrest.conf --no-online --manifest-save-threshold=3 --protocol-timeout=2 --db-timeout=1 --cmd-ssh=/usr/bin/ssh --db1-port=[PORT-1] --db1-socket-path =/test_socket_path --buffer-size=16384 --checksum-page --process-max=1 --repo-type=cifs --type=full --stanza=db backup ------------------------------------------------------------------------------------------------------------------------------------ -P00 INFO: backup command begin [BACKREST-VERSION]: --buffer-size=16384 --checksum-page --cmd-ssh=/usr/bin/ssh --no-compress --config=[TEST_PATH]/backup/pgbackrest.conf --db-timeout=1 --db1-cmd=[BACKREST-BIN] --db1-config=[TEST_PATH]/db-master/pgbackrest.conf --db1-host=db-master --db1-path=[TEST_PATH]/db-master/db/base --db1-user=[USER-1] --lock-path=[TEST_PATH]/backup/lock --log-level-console=debug --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/backup/log --manifest-save-threshold=3 --no-online --process-max=1 --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --start-fast --type=full +P00 INFO: backup command begin [BACKREST-VERSION]: --buffer-size=16384 --checksum-page --cmd-ssh=/usr/bin/ssh --no-compress --config=[TEST_PATH]/backup/pgbackrest.conf --db-timeout=1 --db1-cmd=[BACKREST-BIN] --db1-config=[TEST_PATH]/db-master/pgbackrest.conf --db1-host=db-master --db1-path=[TEST_PATH]/db-master/db/base --db1-port=[PORT-1] --db1-socket-path==/test_socket_path --db1-user=[USER-1] --lock-path=[TEST_PATH]/backup/lock --log-level-console=debug --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/backup/log --manifest-save-threshold=3 --no-online --process-max=1 --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --start-fast --type=full P00 WARN: option retention-full is not set, the repository may run out of space HINT: to retain full backups indefinitely (without warning), set option 'retention-full' to the maximum. P00 DEBUG: Storage::Local->new(): bAllowTemp = , hRule = [hash], lBufferMax = 16384, oDriver = [object], strDefaultFileMode = <0640>, strDefaultPathMode = <0750>, strPathBase = [TEST_PATH]/backup/repo, strTempExtension = pgbackrest.tmp @@ -92,8 +92,8 @@ P00 DEBUG: Db::dbObjectGet(): bMasterOnly = P00 DEBUG: Db->new(): iRemoteIdx = 1 P00 DEBUG: Protocol::Helper::protocolGet(): bCache = , iProcessIdx = [undef], iRemoteIdx = 1, strBackRestBin = [undef], strCommand = , strRemoteType = db P00 DEBUG: Protocol::Helper::protocolGet: create (cached) remote protocol -P00 DEBUG: Protocol::Remote::Master->new(): iBufferMax = 16384, iCompressLevel = 6, iCompressLevelNetwork = 3, iProtocolTimeout = 2, iSshPort = [undef], strCommand = [BACKREST-BIN] --buffer-size=16384 --command=backup --compress-level=6 --compress-level-network=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=1 --db1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db remote, strCommandSSH = /usr/bin/ssh, strHost = db-master, strUser = [USER-1] -P00 DEBUG: Protocol::Command::Master->new(): iBufferMax = 16384, iCompressLevel = 6, iCompressLevelNetwork = 3, iProtocolTimeout = 2, strCommand = /usr/bin/ssh -o LogLevel=error -o Compression=no -o PasswordAuthentication=no [USER-1]@db-master '[BACKREST-BIN] --buffer-size=16384 --command=backup --compress-level=6 --compress-level-network=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=1 --db1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db remote', strId = 'db-master remote', strName = remote +P00 DEBUG: Protocol::Remote::Master->new(): iBufferMax = 16384, iCompressLevel = 6, iCompressLevelNetwork = 3, iProtocolTimeout = 2, iSshPort = [undef], strCommand = [BACKREST-BIN] --buffer-size=16384 --command=backup --compress-level=6 --compress-level-network=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=1 --db1-path=[TEST_PATH]/db-master/db/base --db1-port=[PORT-1] --db1-socket-path==/test_socket_path --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db remote, strCommandSSH = /usr/bin/ssh, strHost = db-master, strUser = [USER-1] +P00 DEBUG: Protocol::Command::Master->new(): iBufferMax = 16384, iCompressLevel = 6, iCompressLevelNetwork = 3, iProtocolTimeout = 2, strCommand = /usr/bin/ssh -o LogLevel=error -o Compression=no -o PasswordAuthentication=no [USER-1]@db-master '[BACKREST-BIN] --buffer-size=16384 --command=backup --compress-level=6 --compress-level-network=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=1 --db1-path=[TEST_PATH]/db-master/db/base --db1-port=[PORT-1] --db1-socket-path==/test_socket_path --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db remote', strId = 'db-master remote', strName = remote P00 DEBUG: Db::dbObjectGet=>: iDbMasterIdx = 1, iDbStandbyIdx = [undef], oDbMaster = [object], oDbStandby = [undef] P00 DEBUG: Protocol::Helper::protocolGet(): bCache = , iProcessIdx = [undef], iRemoteIdx = 1, strBackRestBin = [undef], strCommand = , strRemoteType = db P00 DEBUG: Protocol::Helper::protocolGet: found cached protocol @@ -151,8 +151,8 @@ P00 DEBUG: Protocol::Local::Process->queueJob(): iHostConfigIdx = 1, rParam P00 DEBUG: Protocol::Local::Process->queueJob(): iHostConfigIdx = 1, rParam = ([TEST_PATH]/db-master/db/base/base/1/PG_VERSION, pg_data/base/1/PG_VERSION, 3, [undef], 0, [BACKUP-FULL-1], 0, [MODIFICATION-TIME-1], 1, [undef]), strKey = pg_data/base/1/PG_VERSION, strOp = backupFile, strQueue = pg_data P00 DEBUG: Protocol::Local::Process->queueJob(): iHostConfigIdx = 1, rParam = ([TEST_PATH]/db-master/db/base/PG_VERSION, pg_data/PG_VERSION, 3, [undef], 0, [BACKUP-FULL-1], 0, [MODIFICATION-TIME-1], 1, [undef]), strKey = pg_data/PG_VERSION, strOp = backupFile, strQueue = pg_data P00 DEBUG: Protocol::Local::Process->hostConnect: start local process: iHostConfigIdx = 1, iHostIdx = 0, iHostProcessIdx = 0, iProcessId = 1, strHostType = db -P00 DEBUG: Protocol::Local::Master->new(): iProcessIdx = 1, strCommand = [BACKREST-BIN] --buffer-size=16384 --cmd-ssh=/usr/bin/ssh --command=backup --config=[TEST_PATH]/backup/pgbackrest.conf --db-timeout=1 --db1-cmd=[BACKREST-BIN] --db1-config=[TEST_PATH]/db-master/pgbackrest.conf --db1-host=db-master --db1-path=[TEST_PATH]/db-master/db/base --db1-user=[USER-1] --host-id=1 --lock-path=[TEST_PATH]/backup/lock --log-path=[TEST_PATH]/backup/log --process=1 --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db local -P00 DEBUG: Protocol::Command::Master->new(): iBufferMax = 16384, iCompressLevel = 6, iCompressLevelNetwork = 3, iProtocolTimeout = 2, strCommand = [BACKREST-BIN] --buffer-size=16384 --cmd-ssh=/usr/bin/ssh --command=backup --config=[TEST_PATH]/backup/pgbackrest.conf --db-timeout=1 --db1-cmd=[BACKREST-BIN] --db1-config=[TEST_PATH]/db-master/pgbackrest.conf --db1-host=db-master --db1-path=[TEST_PATH]/db-master/db/base --db1-user=[USER-1] --host-id=1 --lock-path=[TEST_PATH]/backup/lock --log-path=[TEST_PATH]/backup/log --process=1 --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db local, strId = 'local-1', strName = local +P00 DEBUG: Protocol::Local::Master->new(): iProcessIdx = 1, strCommand = [BACKREST-BIN] --buffer-size=16384 --cmd-ssh=/usr/bin/ssh --command=backup --config=[TEST_PATH]/backup/pgbackrest.conf --db-timeout=1 --db1-cmd=[BACKREST-BIN] --db1-config=[TEST_PATH]/db-master/pgbackrest.conf --db1-host=db-master --db1-path=[TEST_PATH]/db-master/db/base --db1-socket-path==/test_socket_path --db1-user=[USER-1] --host-id=1 --lock-path=[TEST_PATH]/backup/lock --log-path=[TEST_PATH]/backup/log --process=1 --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db local +P00 DEBUG: Protocol::Command::Master->new(): iBufferMax = 16384, iCompressLevel = 6, iCompressLevelNetwork = 3, iProtocolTimeout = 2, strCommand = [BACKREST-BIN] --buffer-size=16384 --cmd-ssh=/usr/bin/ssh --command=backup --config=[TEST_PATH]/backup/pgbackrest.conf --db-timeout=1 --db1-cmd=[BACKREST-BIN] --db1-config=[TEST_PATH]/db-master/pgbackrest.conf --db1-host=db-master --db1-path=[TEST_PATH]/db-master/db/base --db1-socket-path==/test_socket_path --db1-user=[USER-1] --host-id=1 --lock-path=[TEST_PATH]/backup/lock --log-path=[TEST_PATH]/backup/log --process=1 --protocol-timeout=2 --repo-path=[TEST_PATH]/backup/repo --repo-type=cifs --stanza=db --type=db local, strId = 'local-1', strName = local P00 DEBUG: Protocol::Local::Process->hostConnect=>: bResult = true P00 DEBUG: Protocol::Local::Process->init: init local process: iDirection = 1, iHostIdx = 0, iProcessId = 1, iQueueIdx = 0, iQueueLastIdx = 0 P00 DEBUG: Protocol::Local::Process->init=>: bResult = true diff --git a/test/expect/mock-all-003.log b/test/expect/mock-all-003.log index 114615b8f..12a5d6b78 100644 --- a/test/expect/mock-all-003.log +++ b/test/expect/mock-all-003.log @@ -64,7 +64,7 @@ P00 WARN: option retention-full is not set, the repository may run out of spac P00 ERROR: [070]: link '[TEST_PATH]/db-master/db/base/postgresql.conf.bad' -> '../pg_config/postgresql.conf.link' cannot reference another link full backup - create pg_stat link, pg_clog dir (backup host) -> [CONTAINER-EXEC] backup [BACKREST-BIN] --config=[TEST_PATH]/backup/pgbackrest.conf --no-online --manifest-save-threshold=3 --protocol-timeout=2 --db-timeout=1 --cmd-ssh=/usr/bin/ssh --buffer-size=16384 --checksum-page --process-max=1 --type=full --stanza=db backup +> [CONTAINER-EXEC] backup [BACKREST-BIN] --config=[TEST_PATH]/backup/pgbackrest.conf --no-online --manifest-save-threshold=3 --protocol-timeout=2 --db-timeout=1 --cmd-ssh=/usr/bin/ssh --db1-port=[PORT-1] --db1-socket-path =/test_socket_path --buffer-size=16384 --checksum-page --process-max=1 --type=full --stanza=db backup ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: option retention-full is not set, the repository may run out of space HINT: to retain full backups indefinitely (without warning), set option 'retention-full' to the maximum. diff --git a/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm b/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm index 6affd2164..4eca66b25 100644 --- a/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm +++ b/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm @@ -326,9 +326,15 @@ sub run $strFullBackup = $oHostBackup->backup( $strType, 'create pg_stat link, pg_clog dir', {oExpectedManifest => \%oManifest, - strOptionalParam => $strOptionalParam . ($bRemote ? ' --cmd-ssh=/usr/bin/ssh' : '') . - ' --' . cfgOptionName(CFGOPT_BUFFER_SIZE) . '=16384 --' . cfgOptionName(CFGOPT_CHECKSUM_PAGE) . - ' --' . cfgOptionName(CFGOPT_PROCESS_MAX) . '=1', + strOptionalParam => $strOptionalParam . + # Pass ssh path to make sure it is used + ($bRemote ? ' --' . cfgOptionName(CFGOPT_CMD_SSH) . '=/usr/bin/ssh' : '') . + # Pass bogus ssh port to make sure it is passed through the protocol layer (it won't be used) + ($bRemote ? ' --' . cfgOptionName(CFGOPT_DB_PORT) . '=9999' : '') . + # Pass bogus socket path to make sure it is passed through the protocol layer (it won't be used) + ($bRemote ? ' --' . cfgOptionName(CFGOPT_DB_SOCKET_PATH) . ' =/test_socket_path' : '') . + ' --' . cfgOptionName(CFGOPT_BUFFER_SIZE) . '=16384 --' . cfgOptionName(CFGOPT_CHECKSUM_PAGE) . + ' --' . cfgOptionName(CFGOPT_PROCESS_MAX) . '=1', strRepoType => $bS3 ? undef : CFGOPTVAL_REPO_TYPE_CIFS, strTest => $strTestPoint, fTestDelay => 0}); # Error on backup option to check logging