From 2a06df93f379bdc28a6c5084d204cb1e5391dbb3 Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
Date: Thu, 6 Feb 2020 20:59:04 -0800
Subject: [PATCH] Remove async archive error file when not throwing an error.

This ensures that the error will not be thrown before the async process has a chance to retry.
---
 src/command/archive/common.c                | 20 ++++++++++++++------
 test/src/module/command/archiveCommonTest.c |  6 ++++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/command/archive/common.c b/src/command/archive/common.c
index cbeb9b84d..17351f8db 100644
--- a/src/command/archive/common.c
+++ b/src/command/archive/common.c
@@ -137,14 +137,22 @@ archiveAsyncStatus(ArchiveMode archiveMode, const String *walSegment, bool confe
 
                 result = true;
             }
-            else if (confessOnError)
+            else
             {
-                // Error status files must have content
-                if (strSize(content) == 0)
-                    THROW_FMT(AssertError, "status file '%s' has no content", strPtr(statusFile));
+                if (confessOnError)
+                {
+                    // Error status files must have content
+                    if (strSize(content) == 0)
+                        THROW_FMT(AssertError, "status file '%s' has no content", strPtr(statusFile));
 
-                // Throw error using the code passed in the file
-                THROW_CODE(code, strPtr(message));
+                    // Throw error using the code passed in the file
+                    THROW_CODE(code, strPtr(message));
+                }
+
+                // Remove the error if it will not be thrown to give the async process a chance to retry.  Otherwise, it could be a
+                // race between the foreground process retrying and the async process removing error files. Ignore missing since the
+                // async process may have already removed the file.
+                storageRemoveP(storageSpoolWrite(), strNewFmt("%s/%s", strPtr(spoolQueue), strPtr(errorFile)));
             }
         }
     }
diff --git a/test/src/module/command/archiveCommonTest.c b/test/src/module/command/archiveCommonTest.c
index 8faa254f2..753e4659a 100644
--- a/test/src/module/command/archiveCommonTest.c
+++ b/test/src/module/command/archiveCommonTest.c
@@ -95,8 +95,14 @@ testRun(void)
             storageNewWriteP(storageSpoolWrite(), strNewFmt(STORAGE_SPOOL_ARCHIVE_OUT "/%s.error", strPtr(segment))),
             BUFSTRDEF("25\nmessage"));
         TEST_ERROR(archiveAsyncStatus(archiveModePush, segment, true), AssertError, "message");
+        TEST_RESULT_BOOL(
+            storageExistsP(storageSpool(), strNewFmt(STORAGE_SPOOL_ARCHIVE_OUT "/%s.error", strPtr(segment))), true,
+             "error file was not removed");
 
         TEST_RESULT_BOOL(archiveAsyncStatus(archiveModePush, segment, false), false, "suppress error");
+        TEST_RESULT_BOOL(
+            storageExistsP(storageSpool(), strNewFmt(STORAGE_SPOOL_ARCHIVE_OUT "/%s.error", strPtr(segment))), false,
+             "error file was removed");
 
         // -------------------------------------------------------------------------------------------------------------------------
         storagePutP(