From 41b00dc204a04a87b337aa5bebe7cfd4032d5eae Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 27 Oct 2018 16:57:57 +0100 Subject: [PATCH] Fix issue with archive-push-queue-max not being honored on connection error. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an error occurred while acquiring a lock on a remote server the error would be reported correctly, but the queue max detection code was not reached. The tests failed to detect this because they fixed the connection before queue max, allowing the ccde to be reached. Move the queue max code before the lock so it will run even when remote connections are not working. This means that no attempt will be made to transfer WAL once queue max has been exceeded, but it makes it much more likely that the code will be reach without error. Update tests to continue errors up to the point where queue max is exceeded. Reported by Lardière Sébastien. --- doc/xml/release.xml | 8 ++++ lib/pgBackRest/Archive/Push/Async.pm | 43 +++++++++---------- src/perl/embed.auto.c | 41 +++++++++--------- test/expect/mock-archive-stop-001.log | 2 +- test/expect/mock-archive-stop-002.log | 2 +- test/expect/mock-archive-stop-003.log | 2 +- test/expect/mock-archive-stop-004.log | 2 +- test/expect/mock-archive-stop-005.log | 2 +- test/expect/mock-archive-stop-006.log | 2 +- test/expect/mock-archive-stop-007.log | 2 +- .../Env/Host/HostDbCommonTest.pm | 5 ++- .../Module/Archive/ArchivePushPerlTest.pm | 13 +++--- .../Module/Mock/MockArchiveStopTest.pm | 10 ++--- 13 files changed, 70 insertions(+), 64 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 4e896240e..69314b72d 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -15,6 +15,14 @@ + + + + + +

Fix issue with archive-push-queue-max not being honored on connection error.

+ + diff --git a/lib/pgBackRest/Archive/Push/Async.pm b/lib/pgBackRest/Archive/Push/Async.pm index 4c512a3f5..926cb170c 100644 --- a/lib/pgBackRest/Archive/Push/Async.pm +++ b/lib/pgBackRest/Archive/Push/Async.pm @@ -121,6 +121,27 @@ sub processQueue # Assign function parameters, defaults, and log debug info my ($strOperation) = logDebugParam(__PACKAGE__ . '->processQueue'); + # If queue max is exceeded then drop all WAL in the queue + my $iDropTotal = 0; + + if (cfgOptionTest(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX)) + { + my $stryDropList = $self->dropList($self->readyList()); + + if (@{$stryDropList} > 0) + { + foreach my $strDropFile (@{$stryDropList}) + { + archiveAsyncStatusWrite( + WAL_STATUS_OK, $self->{strSpoolPath}, $strDropFile, 0, + "dropped WAL file ${strDropFile} because archive queue exceeded " . + cfgOption(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX) . ' bytes'); + + $iDropTotal++; + } + } + } + # Get jobs to process my $stryWalFile = $self->readyList(); @@ -135,7 +156,6 @@ sub processQueue # Process jobs if there are any my $iOkTotal = 0; my $iErrorTotal = 0; - my $iDropTotal = 0; if ($self->{oArchiveProcess}->jobTotal() > 0) { @@ -186,27 +206,6 @@ sub processQueue &log(DETAIL, "pushed WAL file ${strWalFile} to archive", undef, undef, undef, $hJob->{iProcessId}); } } - - # Drop any jobs that exceed the queue max - if (cfgOptionTest(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX)) - { - my $stryDropList = $self->dropList($self->readyList()); - - if (@{$stryDropList} > 0) - { - foreach my $strDropFile (@{$stryDropList}) - { - archiveAsyncStatusWrite( - WAL_STATUS_OK, $self->{strSpoolPath}, $strDropFile, 0, - "dropped WAL file ${strDropFile} because archive queue exceeded " . - cfgOption(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX) . ' bytes'); - - $iDropTotal++; - } - - $self->{oArchiveProcess}->dequeueJobs(1, 'default'); - } - } } return 1; diff --git a/src/perl/embed.auto.c b/src/perl/embed.auto.c index 3f2adb0ed..46710e2c5 100644 --- a/src/perl/embed.auto.c +++ b/src/perl/embed.auto.c @@ -1511,6 +1511,26 @@ static const EmbeddedModule embeddedModule[] = "\n\n" "my ($strOperation) = logDebugParam(__PACKAGE__ . '->processQueue');\n" "\n\n" + "my $iDropTotal = 0;\n" + "\n" + "if (cfgOptionTest(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX))\n" + "{\n" + "my $stryDropList = $self->dropList($self->readyList());\n" + "\n" + "if (@{$stryDropList} > 0)\n" + "{\n" + "foreach my $strDropFile (@{$stryDropList})\n" + "{\n" + "archiveAsyncStatusWrite(\n" + "WAL_STATUS_OK, $self->{strSpoolPath}, $strDropFile, 0,\n" + "\"dropped WAL file ${strDropFile} because archive queue exceeded \" .\n" + "cfgOption(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX) . ' bytes');\n" + "\n" + "$iDropTotal++;\n" + "}\n" + "}\n" + "}\n" + "\n\n" "my $stryWalFile = $self->readyList();\n" "\n\n" "foreach my $strWalFile (@{$stryWalFile})\n" @@ -1522,7 +1542,6 @@ static const EmbeddedModule embeddedModule[] = "\n\n" "my $iOkTotal = 0;\n" "my $iErrorTotal = 0;\n" - "my $iDropTotal = 0;\n" "\n" "if ($self->{oArchiveProcess}->jobTotal() > 0)\n" "{\n" @@ -1571,26 +1590,6 @@ static const EmbeddedModule embeddedModule[] = "&log(DETAIL, \"pushed WAL file ${strWalFile} to archive\", undef, undef, undef, $hJob->{iProcessId});\n" "}\n" "}\n" - "\n\n" - "if (cfgOptionTest(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX))\n" - "{\n" - "my $stryDropList = $self->dropList($self->readyList());\n" - "\n" - "if (@{$stryDropList} > 0)\n" - "{\n" - "foreach my $strDropFile (@{$stryDropList})\n" - "{\n" - "archiveAsyncStatusWrite(\n" - "WAL_STATUS_OK, $self->{strSpoolPath}, $strDropFile, 0,\n" - "\"dropped WAL file ${strDropFile} because archive queue exceeded \" .\n" - "cfgOption(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX) . ' bytes');\n" - "\n" - "$iDropTotal++;\n" - "}\n" - "\n" - "$self->{oArchiveProcess}->dequeueJobs(1, 'default');\n" - "}\n" - "}\n" "}\n" "\n" "return 1;\n" diff --git a/test/expect/mock-archive-stop-001.log b/test/expect/mock-archive-stop-001.log index a8f0a6f5c..7cd3ce639 100644 --- a/test/expect/mock-archive-stop-001.log +++ b/test/expect/mock-archive-stop-001.log @@ -52,7 +52,7 @@ P00 ERROR: [044]: raised from local-1 process: WAL segment version 9.4 does not P00 ERROR: [044]: raised from local-1 process: WAL segment version 9.4 does not match archive version 8.0 HINT: are you archiving to the correct stanza? -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/expect/mock-archive-stop-002.log b/test/expect/mock-archive-stop-002.log index b48b45399..362c6fec5 100644 --- a/test/expect/mock-archive-stop-002.log +++ b/test/expect/mock-archive-stop-002.log @@ -58,7 +58,7 @@ P00 ERROR: [044]: raised from local-1 process: WAL segment version 9.4 does not P00 ERROR: [044]: raised from local-1 process: WAL segment version 9.4 does not match archive version 8.0 HINT: are you archiving to the correct stanza? -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/expect/mock-archive-stop-003.log b/test/expect/mock-archive-stop-003.log index 9f435c03f..a20421fd3 100644 --- a/test/expect/mock-archive-stop-003.log +++ b/test/expect/mock-archive-stop-003.log @@ -52,7 +52,7 @@ P00 ERROR: [044]: raised from local-1 process: raised from remote process on 'b P00 ERROR: [044]: raised from local-1 process: raised from remote process on 'backup': WAL segment version 9.4 does not match archive version 8.0 HINT: are you archiving to the correct stanza? -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/expect/mock-archive-stop-004.log b/test/expect/mock-archive-stop-004.log index 3534fba03..5334d2f5f 100644 --- a/test/expect/mock-archive-stop-004.log +++ b/test/expect/mock-archive-stop-004.log @@ -50,7 +50,7 @@ P00 ERROR: [042]: remote process on 'bogus' terminated unexpectedly: ssh: Could ------------------------------------------------------------------------------------------------------------------------------------ P00 ERROR: [042]: remote process on 'bogus' terminated unexpectedly: ssh: Could not resolve hostname bogus: Name or service not known -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/expect/mock-archive-stop-005.log b/test/expect/mock-archive-stop-005.log index 3235211b2..36ec69504 100644 --- a/test/expect/mock-archive-stop-005.log +++ b/test/expect/mock-archive-stop-005.log @@ -58,7 +58,7 @@ P00 ERROR: [044]: raised from local-1 process: raised from remote process on 'b P00 ERROR: [044]: raised from local-1 process: raised from remote process on 'backup': WAL segment version 9.4 does not match archive version 8.0 HINT: are you archiving to the correct stanza? -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/expect/mock-archive-stop-006.log b/test/expect/mock-archive-stop-006.log index d1832d0d3..48a276516 100644 --- a/test/expect/mock-archive-stop-006.log +++ b/test/expect/mock-archive-stop-006.log @@ -56,7 +56,7 @@ P00 ERROR: [042]: remote process on 'bogus' terminated unexpectedly: ssh: Could ------------------------------------------------------------------------------------------------------------------------------------ P00 ERROR: [042]: remote process on 'bogus' terminated unexpectedly: ssh: Could not resolve hostname bogus: Name or service not known -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/expect/mock-archive-stop-007.log b/test/expect/mock-archive-stop-007.log index f7ef74512..37b8b79a6 100644 --- a/test/expect/mock-archive-stop-007.log +++ b/test/expect/mock-archive-stop-007.log @@ -50,7 +50,7 @@ P00 ERROR: [042]: remote process on 'bogus' terminated unexpectedly: ssh: Could ------------------------------------------------------------------------------------------------------------------------------------ P00 ERROR: [042]: remote process on 'bogus' terminated unexpectedly: ssh: Could not resolve hostname bogus: Name or service not known -> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=warn --archive-push-queue-max=33554432 --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000004 --repo1-host=bogus ------------------------------------------------------------------------------------------------------------------------------------ P00 WARN: dropped WAL file 000000010000000100000004 because archive queue exceeded 33554432 bytes diff --git a/test/lib/pgBackRestTest/Env/Host/HostDbCommonTest.pm b/test/lib/pgBackRestTest/Env/Host/HostDbCommonTest.pm index 6dcbb0301..6142fd943 100644 --- a/test/lib/pgBackRestTest/Env/Host/HostDbCommonTest.pm +++ b/test/lib/pgBackRestTest/Env/Host/HostDbCommonTest.pm @@ -117,6 +117,7 @@ sub archivePush $iArchiveNo, $iExpectedError, $bAsync, + $strOptionalParam, ) = logDebugParam ( @@ -126,6 +127,7 @@ sub archivePush {name => 'iArchiveNo', required => false}, {name => 'iExpectedError', required => false}, {name => 'bAsync', default => true}, + {name => 'strOptionalParam', required => false}, ); my $strSourceFile; @@ -147,7 +149,8 @@ sub archivePush ' --stanza=' . $self->stanza() . (defined($iExpectedError) && $iExpectedError == ERROR_FILE_READ ? ' --repo1-host=bogus' : '') . ($bAsync ? '' : ' --no-archive-async') . - " archive-push" . (defined($strSourceFile) ? " ${strSourceFile}" : ''), + " archive-push" . (defined($strSourceFile) ? " ${strSourceFile}" : '') . + (defined($strOptionalParam) ? " ${strOptionalParam}" : ''), {iExpectedExitStatus => $iExpectedError, oLogTest => $self->{oLogTest}, bLogOutput => $self->synthetic()}); # Return from function and log return values if any diff --git a/test/lib/pgBackRestTest/Module/Archive/ArchivePushPerlTest.pm b/test/lib/pgBackRestTest/Module/Archive/ArchivePushPerlTest.pm index 6c5640b88..ec291e93c 100644 --- a/test/lib/pgBackRestTest/Module/Archive/ArchivePushPerlTest.pm +++ b/test/lib/pgBackRestTest/Module/Archive/ArchivePushPerlTest.pm @@ -489,7 +489,7 @@ sub run } # Process and check results - $self->testResult(sub {$oPushAsync->processQueue()}, '(3, 3, 1, 0)', "process and drop files"); + $self->testResult(sub {$oPushAsync->processQueue()}, '(0, 3, 0, 0)', "process and drop files"); $self->testResult( sub {storageSpool()->list($self->{strSpoolPath})}, '(' . join('.ok, ', @strySegment) . '.ok)', @@ -499,21 +499,20 @@ sub run { $self->testResult( sub {${storageSpool()->get("$self->{strSpoolPath}/${strSegment}.ok")}}, - $strSegment eq $strySegment[0] ? undef : - "0\ndropped WAL file ${strSegment} because archive queue exceeded " . cfgOption(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX) . - ' bytes', + "0\ndropped WAL file ${strSegment} because archive queue exceeded " . cfgOption(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX) . + ' bytes', "verify ${strSegment} status"); $self->walRemove($self->{strWalPath}, $strSegment); } - $self->optionTestClear(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX); - $self->configTestLoad(CFGCMD_ARCHIVE_PUSH); - #--------------------------------------------------------------------------------------------------------------------------- $self->testResult(sub {$oPushAsync->processQueue()}, '(0, 0, 0, 0)', "final process to remove ok files"); $self->testResult(sub {storageSpool()->list($self->{strSpoolPath})}, "[undef]", "ok files removed"); + + $self->optionTestClear(CFGOPT_ARCHIVE_PUSH_QUEUE_MAX); + $self->configTestLoad(CFGCMD_ARCHIVE_PUSH); } ################################################################################################################################ diff --git a/test/lib/pgBackRestTest/Module/Mock/MockArchiveStopTest.pm b/test/lib/pgBackRestTest/Module/Mock/MockArchiveStopTest.pm index 52798d1d8..46bd8c1da 100644 --- a/test/lib/pgBackRestTest/Module/Mock/MockArchiveStopTest.pm +++ b/test/lib/pgBackRestTest/Module/Mock/MockArchiveStopTest.pm @@ -95,7 +95,7 @@ sub run $strWalPath, $strWalTestFile, 3, $iError ? ERROR_FILE_READ : ERROR_ARCHIVE_MISMATCH); # Now this segment will get dropped - $oHostDbMaster->archivePush($strWalPath, $strWalTestFile, 4); + $oHostDbMaster->archivePush($strWalPath, $strWalTestFile, 4, undef, undef, '--repo1-host=bogus'); # Fix the database version if ($iError == 0) @@ -106,18 +106,16 @@ sub run #--------------------------------------------------------------------------------------------------------------------------- $self->testResult( sub {$oStorage->list( - STORAGE_REPO_ARCHIVE . qw{/} . PG_VERSION_94 . '-1/0000000100000001', - {strExpression => '^(?!000000010000000100000002).+'})}, + STORAGE_REPO_ARCHIVE . qw{/} . PG_VERSION_94 . '-1/0000000100000001')}, "000000010000000100000001-${strWalHash}${strCompressExt}", - 'segment 2-4 not pushed (2 is pushed sometimes when remote but ignore)', {iWaitSeconds => 5}); + 'segment 2-4 not pushed', {iWaitSeconds => 5}); #--------------------------------------------------------------------------------------------------------------------------- $oHostDbMaster->archivePush($strWalPath, $strWalTestFile, 5); $self->testResult( sub {$oStorage->list( - STORAGE_REPO_ARCHIVE . qw{/} . PG_VERSION_94 . '-1/0000000100000001', - {strExpression => '^(?!000000010000000100000002).+'})}, + STORAGE_REPO_ARCHIVE . qw{/} . PG_VERSION_94 . '-1/0000000100000001')}, "(000000010000000100000001-${strWalHash}${strCompressExt}, " . "000000010000000100000005-${strWalHash}${strCompressExt})", 'segment 5 is pushed', {iWaitSeconds => 5});