From 0ea2d9f96876c2398b0b8517bfec8d8b8f41518f Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Fri, 23 Dec 2022 16:53:38 -0800 Subject: [PATCH v6 2/3] Refactor code for restoring files via shell. Presently, restore_command uses a different code path than archive_cleanup_command and recovery_end_command. These code paths are similar and can be easily combined. --- src/backend/access/transam/shell_restore.c | 90 ++++++++++------------ 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/src/backend/access/transam/shell_restore.c b/src/backend/access/transam/shell_restore.c index f52f0b92a4..68bf6e8556 100644 --- a/src/backend/access/transam/shell_restore.c +++ b/src/backend/access/transam/shell_restore.c @@ -23,35 +23,22 @@ #include "storage/ipc.h" #include "utils/wait_event.h" -static void ExecuteRecoveryCommand(const char *command, +static bool ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal, - uint32 wait_event_info, - const char *lastRestartPointFileName); + bool exitOnSigterm, uint32 wait_event_info, + int fail_elevel); bool shell_restore(const char *file, const char *path, const char *lastRestartPointFileName) { char *cmd; - int rc; + bool ret; /* Build the restore command to execute */ cmd = BuildRestoreCommand(recoveryRestoreCommand, path, file, lastRestartPointFileName); - ereport(DEBUG3, - (errmsg_internal("executing restore command \"%s\"", cmd))); - - /* - * Copy xlog from archival storage to XLOGDIR - */ - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); - rc = system(cmd); - pgstat_report_wait_end(); - - pfree(cmd); - /* * Remember, we rollforward UNTIL the restore fails so failure here is * just part of the process... that makes it difficult to determine @@ -76,30 +63,37 @@ shell_restore(const char *file, const char *path, * * We treat hard shell errors such as "command not found" as fatal, too. */ - if (wait_result_is_signal(rc, SIGTERM)) - proc_exit(1); - - ereport(wait_result_is_any_signal(rc, true) ? FATAL : DEBUG2, - (errmsg("could not restore file \"%s\" from archive: %s", - file, wait_result_to_str(rc)))); + ret = ExecuteRecoveryCommand(cmd, "restore_command", true, true, + WAIT_EVENT_RESTORE_COMMAND, DEBUG2); + pfree(cmd); - return (rc == 0); + return ret; } void shell_archive_cleanup(const char *lastRestartPointFileName) { - ExecuteRecoveryCommand(archiveCleanupCommand, "archive_cleanup_command", - false, WAIT_EVENT_ARCHIVE_CLEANUP_COMMAND, - lastRestartPointFileName); + char *cmd; + + cmd = replace_percent_placeholders(archiveCleanupCommand, + "archive_cleanup_command", + "r", lastRestartPointFileName); + (void) ExecuteRecoveryCommand(cmd, "archive_cleanup_command", false, false, + WAIT_EVENT_ARCHIVE_CLEANUP_COMMAND, WARNING); + pfree(cmd); } void shell_recovery_end(const char *lastRestartPointFileName) { - ExecuteRecoveryCommand(recoveryEndCommand, "recovery_end_command", true, - WAIT_EVENT_RECOVERY_END_COMMAND, - lastRestartPointFileName); + char *cmd; + + cmd = replace_percent_placeholders(recoveryEndCommand, + "recovery_end_command", + "r", lastRestartPointFileName); + (void) ExecuteRecoveryCommand(cmd, "recovery_end_command", true, false, + WAIT_EVENT_RECOVERY_END_COMMAND, WARNING); + pfree(cmd); } /* @@ -107,26 +101,19 @@ shell_recovery_end(const char *lastRestartPointFileName) * * 'command' is the shell command to be executed, 'commandName' is a * human-readable name describing the command emitted in the logs. If - * 'failOnSignal' is true and the command is killed by a signal, a FATAL - * error is thrown. Otherwise a WARNING is emitted. + * 'failOnSignal' is true and the command is killed by a signal, a FATAL error + * is thrown. Otherwise, 'fail_elevel' is used for the log message. If + * 'exitOnSigterm' is true and the command is killed by SIGTERM, we exit + * immediately. * - * This is currently used for recovery_end_command and archive_cleanup_command. + * Returns whether the command succeeded. */ -static void +static bool ExecuteRecoveryCommand(const char *command, const char *commandName, - bool failOnSignal, uint32 wait_event_info, - const char *lastRestartPointFileName) + bool failOnSignal, bool exitOnSigterm, + uint32 wait_event_info, int fail_elevel) { - char *xlogRecoveryCmd; - int rc; - - Assert(command && commandName); - - /* - * construct the command to be executed - */ - xlogRecoveryCmd = replace_percent_placeholders(command, commandName, "r", - lastRestartPointFileName); + int rc; ereport(DEBUG3, (errmsg_internal("executing %s \"%s\"", commandName, command))); @@ -136,18 +123,19 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, */ fflush(NULL); pgstat_report_wait_start(wait_event_info); - rc = system(xlogRecoveryCmd); + rc = system(command); pgstat_report_wait_end(); - pfree(xlogRecoveryCmd); - if (rc != 0) { + if (exitOnSigterm && wait_result_is_signal(rc, SIGTERM)) + proc_exit(1); + /* * If the failure was due to any sort of signal, it's best to punt and * abort recovery. See comments in shell_restore(). */ - ereport((failOnSignal && wait_result_is_any_signal(rc, true)) ? FATAL : WARNING, + ereport((failOnSignal && wait_result_is_any_signal(rc, true)) ? FATAL : fail_elevel, /*------ translator: First %s represents a postgresql.conf parameter name like "recovery_end_command", the 2nd is the value of that parameter, the @@ -155,4 +143,6 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, (errmsg("%s \"%s\": %s", commandName, command, wait_result_to_str(rc)))); } + + return (rc == 0); } -- 2.25.1