Fixed an issue where the async archiver would not be started if archi…

…ve-push did not have enough space to queue a new WAL segment. This meant that the queue would never be cleared without manual intervention (such as calling archive-push directly). PostgreSQL now receives errors when there is not enough space to store new WAL segments but the async process will still be started so that space is eventually freed. Reported by Jens Wilke.
pgbackrest · Oct 5, 2016 · 5dc801d · 5dc801d
1 parent 2a18674
commit 5dc801d
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 3 deletions.
diff --git a/doc/xml/release.xml b/doc/xml/release.xml
@@ -141,6 +141,14 @@
 
                         <p>Fixed error message to properly display the archive command when an invalid archive command is detected.</p>
                     </release-item>
+
+                    <release-item>
+                        <release-item-contributor-list>
+                            <release-item-ideator id="wilke.jens"/>
+                        </release-item-contributor-list>
+
+                        <p>Fixed an issue where the async archiver would not be started if <cmd>archive-push</cmd> did not have enough space to queue a new WAL segment.  This meant that the queue would never be cleared without manual intervention (such as calling <cmd>archive-push</cmd> directly).  <postgres/> now receives errors when there is not enough space to store new WAL segments but the async process will still be started so that space is eventually freed.</p>
+                    </release-item>
                 </release-bug-list>
 
                 <release-feature-list>

diff --git a/lib/pgBackRest/Archive.pm b/lib/pgBackRest/Archive.pm
@@ -589,6 +589,8 @@ sub pushProcess
     }
 
     # If an archive file is defined, then push it
+    my $oException = undef;
+
     if (defined($ARGV[1]))
     {
         # If the stop file exists then discard the archive log
@@ -606,13 +608,27 @@ sub pushProcess
 
         &log(INFO, 'push WAL segment ' . $ARGV[1] . ($bArchiveAsync ? ' asynchronously' : ''));
 
-        $self->push($ARGV[1], $bArchiveAsync);
+        # Push WAL segment - when async, any error will be deferred until after the async process has been started
+        eval
+        {
+            $self->push($ARGV[1], $bArchiveAsync);
+                return true;
+        }
+        or do
+        {
+            $oException = $EVAL_ERROR;
+
+            if (!$bArchiveAsync || !optionTest(OPTION_TEST_NO_FORK))
+            {
+                confess $oException;
+            }
+        };
 
         # Fork if async archiving is enabled
         if ($bArchiveAsync)
         {
             # Fork and disable the async archive flag if this is the parent process
-            if (!optionTest(OPTION_TEST_NO_FORK) || !optionGet(OPTION_TEST_NO_FORK))
+            if (!optionTest(OPTION_TEST_NO_FORK))
             {
                 $bArchiveAsync = fork() == 0 ? true : false;
             }
@@ -661,6 +677,10 @@ sub pushProcess
             lockRelease();
         }
     }
+    elsif (defined($oException))
+    {
+        confess $oException;
+    }
 
     # Return from function and log return values if any
     return logDebugReturn

diff --git a/lib/pgBackRest/Protocol/IO.pm b/lib/pgBackRest/Protocol/IO.pm
@@ -490,7 +490,7 @@ sub bufferWrite
         my $strError = $!;
 
         $self->waitPid();
-        confess "unable to write ${iWriteSize} bytes" . (defined($strError) ? ': ' . $strError : '');
+        confess &log(ERROR, "unable to write ${iWriteSize} bytes" . (defined($strError) ? ': ' . $strError : ''), ERROR_FILE_WRITE);
     }
 }
 

diff --git a/test/expect/backup-archive-push-002.log b/test/expect/backup-archive-push-002.log
@@ -175,6 +175,10 @@ P00  DEBUG:     Protocol::Protocol::protocolGet(): iRemoteIdx = <1>, oParam = [u
 P00  DEBUG:     Protocol::Protocol::protocolGet: create local protocol
 P00  DEBUG:     File->new(): oProtocol = [object], strBackupPath = [TEST_PATH]/db-master/repo, strDefaultFileMode = <0640>, strDefaultPathMode = <0750>, strStanza = db
 P00  ERROR: [137]: stop file exists for all stanzas
+P00  DEBUG:     Archive->pushProcess: no fork on archive local for TESTING
+P00  DEBUG:     Archive->pushProcess: start async archive-push
+P00  DEBUG:     Common::Lock::lockAcquire(): bFailOnNoLock = false, bRemote = <false>, iProcessIdx = [undef], strLockType = archive-push
+P00  ERROR: [137]: stop file exists for all stanzas
 P00  DEBUG:     Common::Exit::exitSafe(): iExitCode = [undef], oException = [object], strSignal = [undef]
 P00   INFO: archive-push stop
 P00  DEBUG:     Protocol::Protocol::protocolDestroy(): bComplete = false, iRemoteIdx = [undef], strRemoteType = [undef]

diff --git a/test/expect/backup-archive-push-006.log b/test/expect/backup-archive-push-006.log
@@ -181,6 +181,10 @@ P00  DEBUG:     Protocol::Protocol::protocolGet(): iRemoteIdx = <1>, oParam = [u
 P00  DEBUG:     Protocol::Protocol::protocolGet: create local protocol
 P00  DEBUG:     File->new(): oProtocol = [object], strBackupPath = [TEST_PATH]/db-master/spool, strDefaultFileMode = <0640>, strDefaultPathMode = <0750>, strStanza = db
 P00  ERROR: [137]: stop file exists for all stanzas
+P00  DEBUG:     Archive->pushProcess: no fork on archive local for TESTING
+P00  DEBUG:     Archive->pushProcess: start async archive-push
+P00  DEBUG:     Common::Lock::lockAcquire(): bFailOnNoLock = false, bRemote = <false>, iProcessIdx = [undef], strLockType = archive-push
+P00  ERROR: [137]: stop file exists for all stanzas
 P00  DEBUG:     Common::Exit::exitSafe(): iExitCode = [undef], oException = [object], strSignal = [undef]
 P00   INFO: archive-push stop
 P00  DEBUG:     Protocol::Protocol::protocolDestroy(): bComplete = false, iRemoteIdx = [undef], strRemoteType = [undef]