From ec72e05e87c73dee3de73f9a6586e8e8db2d919e Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 23 Feb 2025 16:52:29 +0100 Subject: [PATCH v5 1/3] Adds a helper for places that shell out to system() We need to call system() in a few places, and to do so safely we need some pre/post-fork logic. This encapsulates that logic into a single System helper function. The main reason to do this is that in a follow up commit we want to add even more logic pre and post-fork. --- src/backend/access/transam/xlogarchive.c | 28 +----------- src/backend/archive/shell_archive.c | 6 +-- src/backend/postmaster/startup.c | 54 +++++++++++++++++------- src/include/postmaster/startup.h | 3 +- 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 1ef1713c91a..59a0f191339 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -158,27 +158,7 @@ RestoreArchivedFile(char *path, const char *xlogfname, (errmsg_internal("executing restore command \"%s\"", xlogRestoreCmd))); - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND); - - /* - * PreRestoreCommand() informs the SIGTERM handler for the startup process - * that it should proc_exit() right away. This is done for the duration - * of the system() call because there isn't a good way to break out while - * it is executing. Since we might call proc_exit() in a signal handler, - * it is best to put any additional logic before or after the - * PreRestoreCommand()/PostRestoreCommand() section. - */ - PreRestoreCommand(); - - /* - * Copy xlog from archival storage to XLOGDIR - */ - rc = system(xlogRestoreCmd); - - PostRestoreCommand(); - - pgstat_report_wait_end(); + rc = System(xlogRestoreCmd, WAIT_EVENT_RESTORE_COMMAND); pfree(xlogRestoreCmd); if (rc == 0) @@ -325,11 +305,7 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, /* * execute the constructed command */ - fflush(NULL); - pgstat_report_wait_start(wait_event_info); - rc = system(xlogRecoveryCmd); - pgstat_report_wait_end(); - + rc = System(xlogRecoveryCmd, wait_event_info); pfree(xlogRecoveryCmd); if (rc != 0) diff --git a/src/backend/archive/shell_archive.c b/src/backend/archive/shell_archive.c index 828723afe47..7977e5a5092 100644 --- a/src/backend/archive/shell_archive.c +++ b/src/backend/archive/shell_archive.c @@ -22,6 +22,7 @@ #include "archive/shell_archive.h" #include "common/percentrepl.h" #include "pgstat.h" +#include "postmaster/startup.h" static bool shell_archive_configured(ArchiveModuleState *state); static bool shell_archive_file(ArchiveModuleState *state, @@ -75,10 +76,7 @@ shell_archive_file(ArchiveModuleState *state, const char *file, (errmsg_internal("executing archive command \"%s\"", xlogarchcmd))); - fflush(NULL); - pgstat_report_wait_start(WAIT_EVENT_ARCHIVE_COMMAND); - rc = system(xlogarchcmd); - pgstat_report_wait_end(); + rc = System(xlogarchcmd, WAIT_EVENT_ARCHIVE_COMMAND); if (rc != 0) { diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index 88eab3d0baf..b0cd40c081e 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -33,6 +33,7 @@ #include "utils/guc.h" #include "utils/memutils.h" #include "utils/timeout.h" +#include "utils/wait_event_types.h" #ifndef USE_POSTMASTER_DEATH_SIGNAL @@ -264,24 +265,45 @@ StartupProcessMain(const void *startup_data, size_t startup_data_len) proc_exit(0); } -void -PreRestoreCommand(void) +/* + * A custom wrapper around the system() function that calls the necessary + * functions pre/post-fork. + */ +int +System(const char *command, uint32 wait_event_info) { - /* - * Set in_restore_command to tell the signal handler that we should exit - * right away on SIGTERM. We know that we're at a safe point to do that. - * Check if we had already received the signal, so that we don't miss a - * shutdown request received just before this. - */ - in_restore_command = true; - if (shutdown_requested) - proc_exit(1); -} + int rc; -void -PostRestoreCommand(void) -{ - in_restore_command = false; + fflush(NULL); + pgstat_report_wait_start(wait_event_info); + + if (wait_event_info == WAIT_EVENT_RESTORE_COMMAND) + { + /* + * Set in_restore_command to tell the signal handler that we should + * exit right away on SIGTERM. This is done for the duration of the + * system() call because there isn't a good way to break out while it + * is executing. Since we might call proc_exit() in a signal handler, + * it is best to put any additional logic outside of this section + * where in_restore_command is set to true. + */ + in_restore_command = true; + + /* + * Also check if we had already received the signal, so that we don't + * miss a shutdown request received just before this. + */ + if (shutdown_requested) + proc_exit(1); + } + + rc = system(command); + + if (wait_event_info == WAIT_EVENT_RESTORE_COMMAND) + in_restore_command = false; + + pgstat_report_wait_end(); + return rc; } bool diff --git a/src/include/postmaster/startup.h b/src/include/postmaster/startup.h index ae0f6347fc0..09f5dcd3d03 100644 --- a/src/include/postmaster/startup.h +++ b/src/include/postmaster/startup.h @@ -27,8 +27,7 @@ extern PGDLLIMPORT int log_startup_progress_interval; extern void HandleStartupProcInterrupts(void); extern void StartupProcessMain(const void *startup_data, size_t startup_data_len) pg_attribute_noreturn(); -extern void PreRestoreCommand(void); -extern void PostRestoreCommand(void); +extern int System(const char *command, uint32 wait_event_info); extern bool IsPromoteSignaled(void); extern void ResetPromoteSignaled(void); base-commit: 454c182f8542890d0e2eac85f70d9a254a34fce3 -- 2.43.0