aboutsummaryrefslogtreecommitdiff
path: root/src/backend/access/transam/xlogarchive.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2018-12-16 14:32:14 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2018-12-16 14:32:14 -0500
commitb1894a6076f36a20338e0c3b0eb73a250ea2e47e (patch)
treead4d51ae01d31d62d0a7c2a501282c1c26e0e120 /src/backend/access/transam/xlogarchive.c
parent171cae1579124a400e234f89c1fc80f3caed5f68 (diff)
downloadpostgresql-b1894a6076f36a20338e0c3b0eb73a250ea2e47e.tar.gz
postgresql-b1894a6076f36a20338e0c3b0eb73a250ea2e47e.zip
Improve detection of child-process SIGPIPE failures.
Commit ffa4cbd62 added logic to detect SIGPIPE failure of a COPY child process, but it only worked correctly if the SIGPIPE occurred in the immediate child process. Depending on the shell in use and the complexity of the shell command string, we might instead get back an exit code of 128 + SIGPIPE, representing a shell error exit reporting SIGPIPE in the child process. We could just hack up ClosePipeToProgram() to add the extra case, but it seems like this is a fairly general issue deserving a more general and better-documented solution. I chose to add a couple of functions in src/common/wait_error.c, which is a natural place to know about wait-result encodings, that will test for either a specific child-process signal type or any child-process signal failure. Then, adjust other places that were doing ad-hoc tests of this type to use the common functions. In RestoreArchivedFile, this fixes a race condition affecting whether the process will report an error or just silently proc_exit(1): before, that depended on whether the intermediate shell got SIGTERM'd itself or reported a child process failing on SIGTERM. Like the previous patch, back-patch to v10; we could go further but there seems no real need to. Per report from Erik Rijkers. Discussion: https://postgr.es/m/f3683f87ab1701bea5d86a7742b22432@xs4all.nl
Diffstat (limited to 'src/backend/access/transam/xlogarchive.c')
-rw-r--r--src/backend/access/transam/xlogarchive.c20
1 files changed, 5 insertions, 15 deletions
diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c
index b3f0602ffd4..2d2812b59c8 100644
--- a/src/backend/access/transam/xlogarchive.c
+++ b/src/backend/access/transam/xlogarchive.c
@@ -59,7 +59,6 @@ RestoreArchivedFile(char *path, const char *xlogfname,
char *endp;
const char *sp;
int rc;
- bool signaled;
struct stat stat_buf;
XLogSegNo restartSegNo;
XLogRecPtr restartRedoPtr;
@@ -289,17 +288,12 @@ RestoreArchivedFile(char *path, const char *xlogfname,
* will perform an immediate shutdown when it sees us exiting
* unexpectedly.
*
- * Per the Single Unix Spec, shells report exit status > 128 when a called
- * command died on a signal. Also, 126 and 127 are used to report
- * problems such as an unfindable command; treat those as fatal errors
- * too.
+ * We treat hard shell errors such as "command not found" as fatal, too.
*/
- if (WIFSIGNALED(rc) && WTERMSIG(rc) == SIGTERM)
+ if (wait_result_is_signal(rc, SIGTERM))
proc_exit(1);
- signaled = WIFSIGNALED(rc) || WEXITSTATUS(rc) > 125;
-
- ereport(signaled ? FATAL : DEBUG2,
+ ereport(wait_result_is_any_signal(rc, true) ? FATAL : DEBUG2,
(errmsg("could not restore file \"%s\" from archive: %s",
xlogfname, wait_result_to_str(rc))));
@@ -335,7 +329,6 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOn
char *endp;
const char *sp;
int rc;
- bool signaled;
XLogSegNo restartSegNo;
XLogRecPtr restartRedoPtr;
TimeLineID restartTli;
@@ -403,12 +396,9 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOn
{
/*
* If the failure was due to any sort of signal, it's best to punt and
- * abort recovery. See also detailed comments on signals in
- * RestoreArchivedFile().
+ * abort recovery. See comments in RestoreArchivedFile().
*/
- signaled = WIFSIGNALED(rc) || WEXITSTATUS(rc) > 125;
-
- ereport((signaled && failOnSignal) ? FATAL : WARNING,
+ ereport((failOnSignal && wait_result_is_any_signal(rc, true)) ? FATAL : WARNING,
/*------
translator: First %s represents a recovery.conf parameter name like
"recovery_end_command", the 2nd is the value of that parameter, the