From 062e714307de357ae49bcac8c48e397b1debfa0f Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 21 Nov 2017 09:31:15 -0500 Subject: [PATCH] Disable gzip filter when --compress-level-network=0. The filter was used with compress level set to 0 which added overhead without any benefit. --- doc/xml/release.xml | 4 +++ lib/pgBackRest/Protocol/Storage/Remote.pm | 27 +++++++++++++++---- test/expect/mock-all-001.log | 2 +- test/expect/mock-all-002.log | 6 ++--- test/expect/mock-all-003.log | 4 +-- .../pgBackRestTest/Module/Mock/MockAllTest.pm | 2 +- 6 files changed, 33 insertions(+), 12 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 983618a08..c2c19803a 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -59,6 +59,10 @@ + +

Disable gzip filter when --compress-level-network=0. The filter was used with compress level set to 0 which added overhead without any benefit.

+ +

Add list type for options. The hash type was being used for lists with an additional flag (`value-hash`) to indicate that it was not really a hash.

 diff --git a/lib/pgBackRest/Protocol/Storage/Remote.pm b/lib/pgBackRest/Protocol/Storage/Remote.pm index a97b3a02e..e693b30bb 100644 --- a/lib/pgBackRest/Protocol/Storage/Remote.pm +++ b/lib/pgBackRest/Protocol/Storage/Remote.pm @@ -163,7 +163,8 @@ sub openRead {name => 'rhParam', required => false}, ); - my $bProtocolCompress = defined($rhParam->{bProtocolCompress}) && $rhParam->{bProtocolCompress}; + # Determine whether protocol compress will be used + my $bProtocolCompress = protocolCompress($rhParam); # Compress on the remote side if ($bProtocolCompress) @@ -172,7 +173,6 @@ sub openRead @{$rhParam->{rhyFilter}}, {strClass => STORAGE_FILTER_GZIP, rxyParam => [{iLevel => cfgOption(CFGOPT_COMPRESS_LEVEL_NETWORK), bWantGzip => false}]}); - delete($rhParam->{bProtocolCompress}); } my $oSourceFileIo = @@ -215,8 +215,8 @@ sub openWrite {name => 'rhParam', required => false}, ); - # Is protocol compression enabled? - my $bProtocolCompress = defined($rhParam->{bProtocolCompress}) && $rhParam->{bProtocolCompress}; + # Determine whether protocol compress will be used + my $bProtocolCompress = protocolCompress($rhParam); # Decompress on the remote side if ($bProtocolCompress) @@ -224,7 +224,6 @@ sub openWrite push( @{$rhParam->{rhyFilter}}, {strClass => STORAGE_FILTER_GZIP, rxyParam => [{strCompressType => STORAGE_DECOMPRESS, bWantGzip => false}]}); - delete($rhParam->{bProtocolCompress}); } # Open the remote file @@ -336,6 +335,24 @@ sub cipherPassUser ); } +#################################################################################################################################### +# Used internally to determine if protocol compression should be enabled +#################################################################################################################################### +sub protocolCompress +{ + my $rhParam = shift; + + my $bProtocolCompress = false; + + if (defined($rhParam->{bProtocolCompress})) + { + $bProtocolCompress = $rhParam->{bProtocolCompress} && cfgOption(CFGOPT_COMPRESS_LEVEL_NETWORK) > 0 ? true : false; + delete($rhParam->{bProtocolCompress}); + } + + return $bProtocolCompress; +} + #################################################################################################################################### # getters #################################################################################################################################### diff --git a/test/expect/mock-all-001.log b/test/expect/mock-all-001.log index fbb2457f3..a922185a8 100644 --- a/test/expect/mock-all-001.log +++ b/test/expect/mock-all-001.log @@ -1302,7 +1302,7 @@ P00 INFO: restore command end: completed successfully restore_command = '[BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=detail --stanza=db archive-get %f "%p"' restore delta, backup '[BACKUP-FULL-2]' - fix broken symlink (db-master host) -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --link-all --log-level-console=detail --log-level-console=detail --stanza=db restore +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --link-all --log-level-console=detail --stanza=db restore ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: restore command begin [BACKREST-VERSION]: --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db1-path=[TEST_PATH]/db-master/db/base --delta --link-all --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 --protocol-timeout=60 --repo-path=[TEST_PATH]/db-master/repo --set=[BACKUP-FULL-2] --stanza=db P00 INFO: restore backup set [BACKUP-FULL-2] diff --git a/test/expect/mock-all-002.log b/test/expect/mock-all-002.log index 050921865..bf3785612 100644 --- a/test/expect/mock-all-002.log +++ b/test/expect/mock-all-002.log @@ -1486,9 +1486,9 @@ P00 DEBUG: Common::Exit::exitSafe=>: iExitCode = 0 restore_command = '[BACKREST-BIN] --cmd-ssh=/usr/bin/ssh --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get %f "%p"' restore delta, backup '[BACKUP-FULL-2]' - fix broken symlink (db-master host) -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --link-all --log-level-console=detail --log-level-console=detail --stanza=db restore +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --link-all --log-level-console=detail --compress-level-network=0 --stanza=db restore ------------------------------------------------------------------------------------------------------------------------------------ -P00 INFO: restore command begin [BACKREST-VERSION]: --backup-cmd=[BACKREST-BIN] --backup-config=[TEST_PATH]/backup/pgbackrest.conf --backup-host=backup --backup-user=[USER-2] --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db1-path=[TEST_PATH]/db-master/db/base --delta --link-all --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 --protocol-timeout=60 --set=[BACKUP-FULL-2] --stanza=db +P00 INFO: restore command begin [BACKREST-VERSION]: --backup-cmd=[BACKREST-BIN] --backup-config=[TEST_PATH]/backup/pgbackrest.conf --backup-host=backup --backup-user=[USER-2] --no-compress --compress-level=3 --compress-level-network=0 --config=[TEST_PATH]/db-master/pgbackrest.conf --db1-path=[TEST_PATH]/db-master/db/base --delta --link-all --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 --protocol-timeout=60 --set=[BACKUP-FULL-2] --stanza=db P00 INFO: restore backup set [BACKUP-FULL-2] P00 DETAIL: check [TEST_PATH]/db-master/db/base exists P00 DETAIL: check [TEST_PATH]/db-master/db/pg_stat exists @@ -1516,7 +1516,7 @@ P00 INFO: restore command end: completed successfully + supplemental file: [TEST_PATH]/db-master/db/base/recovery.conf ---------------------------------------------------------------- -restore_command = '[BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=detail --stanza=db archive-get %f "%p"' +restore_command = '[BACKREST-BIN] --compress-level-network=0 --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=detail --stanza=db archive-get %f "%p"' restore delta, force, backup '[BACKUP-FULL-2]', expect exit 40 - fail on missing PG_VERSION (db-master host) > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --force --set=[BACKUP-FULL-2] --log-level-console=detail --stanza=db restore diff --git a/test/expect/mock-all-003.log b/test/expect/mock-all-003.log index 5cc425d5c..d6c574812 100644 --- a/test/expect/mock-all-003.log +++ b/test/expect/mock-all-003.log @@ -433,12 +433,12 @@ restore delta, backup '[BACKUP-FULL-2]' - add and delete files (db-master host) restore_command = '[BACKREST-BIN] --cmd-ssh=/usr/bin/ssh --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get %f "%p"' restore delta, backup '[BACKUP-FULL-2]' - fix broken symlink (db-master host) -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --link-all --log-level-console=warn --log-level-console=warn --stanza=db restore +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --set=[BACKUP-FULL-2] --link-all --log-level-console=warn --compress-level-network=0 --stanza=db restore ------------------------------------------------------------------------------------------------------------------------------------ + supplemental file: [TEST_PATH]/db-master/db/base/recovery.conf ---------------------------------------------------------------- -restore_command = '[BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --stanza=db archive-get %f "%p"' +restore_command = '[BACKREST-BIN] --compress-level-network=0 --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --stanza=db archive-get %f "%p"' restore delta, force, backup '[BACKUP-FULL-2]', expect exit 40 - fail on missing PG_VERSION (db-master host) > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --delta --force --set=[BACKUP-FULL-2] --log-level-console=warn --stanza=db restore diff --git a/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm b/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm index 947584699..8fbef9514 100644 --- a/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm +++ b/test/lib/pgBackRestTest/Module/Mock/MockAllTest.pm @@ -574,7 +574,7 @@ sub run $oHostDbMaster->restore( $strFullBackup, \%oManifest, undef, $bDelta, $bForce, undef, undef, undef, undef, undef, undef, - 'fix broken symlink', undef, " --link-all ${strLogReduced} ${strLogReduced}"); + 'fix broken symlink', undef, " --link-all ${strLogReduced}" . ($bRemote ? ' --compress-level-network=0' : '')); # Additional restore tests that don't need to be performed for every permutation if (!$bRemote)