From a0d5a3b8cfe1d52900027b97b8c1bf8fd9d04362 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 18 Oct 2022 08:35:23 +0000 Subject: [PATCH v2] Fixes related to archive module shutdown callback Presently, the archive module shutdown callback can get called twice as it is first registered using PG_ENSURE_ERROR_CLEANUP and then called directly in HandlePgArchInterrupts() before proc_exit(). This patch fixes it by just registering it as before_shmem_exit(). In addition to the above, this patch also does the following things: 1) Renames call_archive_module_shutdown_callback() to be more meaningful and generic as before_shmem_exit() callback. 2) Clarifies when the archive module shutdown callback gets called in documentation. 3) Defines a shutdown callback that just emits a log message in shell_archive.c and tests it. Author: Nathan Bossart Author: Bharath Rupireddy Reviewed-by: Michael Paquier, Kyotaro Horiguchi Discussion: https://www.postgresql.org/message-id/20221015221328.GB1821022%40nathanxps13 Backpatch-through: 15 --- doc/src/sgml/archive-modules.sgml | 7 ++++--- doc/src/sgml/config.sgml | 5 +++-- src/backend/postmaster/pgarch.c | 24 +++++++---------------- src/backend/postmaster/shell_archive.c | 9 +++++++++ src/test/recovery/t/020_archive_status.pl | 19 ++++++++++++++++++ 5 files changed, 42 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml index ef02051f7f..677a29182a 100644 --- a/doc/src/sgml/archive-modules.sgml +++ b/doc/src/sgml/archive-modules.sgml @@ -121,9 +121,10 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path); Shutdown Callback - The shutdown_cb callback is called when the archiver - process exits (e.g., after an error) or the value of - changes. If no + The shutdown_cb callback is called before the server + begins to shut down low-level subsystems such as shared memory upon the + archiver process exit (e.g., after an error or the value of + changes). If no shutdown_cb is defined, no special action is taken in these situations. diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 66312b53b8..22bbf05fef 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3625,8 +3625,9 @@ include_dir 'conf.d' The library to use for archiving completed WAL file segments. If set to an empty string (the default), archiving via shell is enabled, and is used. Otherwise, the specified - shared library is used for archiving. For more information, see - and + shared library is used for archiving. The WAL archiver process gets + restarted by the postmaster whenever this parameter changes. For more + information, see and . diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 3868cd7bd3..f1d1f808a8 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -144,7 +144,7 @@ static void pgarch_die(int code, Datum arg); static void HandlePgArchInterrupts(void); static int ready_file_comparator(Datum a, Datum b, void *arg); static void LoadArchiveLibrary(void); -static void call_archive_module_shutdown_callback(int code, Datum arg); +static void pgarch_before_shmem_exit(int code, Datum arg); /* Report shared memory space needed by PgArchShmemInit */ Size @@ -232,6 +232,8 @@ PgArchiverMain(void) /* We shouldn't be launched unnecessarily. */ Assert(XLogArchivingActive()); + before_shmem_exit(pgarch_before_shmem_exit, 0); + /* Arrange to clean up at archiver exit */ on_shmem_exit(pgarch_die, 0); @@ -252,13 +254,7 @@ PgArchiverMain(void) /* Load the archive_library. */ LoadArchiveLibrary(); - PG_ENSURE_ERROR_CLEANUP(call_archive_module_shutdown_callback, 0); - { - pgarch_MainLoop(); - } - PG_END_ENSURE_ERROR_CLEANUP(call_archive_module_shutdown_callback, 0); - - call_archive_module_shutdown_callback(0, 0); + pgarch_MainLoop(); proc_exit(0); } @@ -803,12 +799,6 @@ HandlePgArchInterrupts(void) if (archiveLibChanged) { - /* - * Call the currently loaded archive module's shutdown callback, - * if one is defined. - */ - call_archive_module_shutdown_callback(0, 0); - /* * Ideally, we would simply unload the previous archive module and * load the new one, but there is presently no mechanism for @@ -861,12 +851,12 @@ LoadArchiveLibrary(void) } /* - * call_archive_module_shutdown_callback + * Before shared memory exit callback for WAL archiver process. * - * Calls the loaded archive module's shutdown callback, if one is defined. + * It calls the loaded archive module's shutdown callback, if one is defined. */ static void -call_archive_module_shutdown_callback(int code, Datum arg) +pgarch_before_shmem_exit(int code, Datum arg) { if (ArchiveContext.shutdown_cb != NULL) ArchiveContext.shutdown_cb(); diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c index 8a54d02e7b..0a0ea0e6b5 100644 --- a/src/backend/postmaster/shell_archive.c +++ b/src/backend/postmaster/shell_archive.c @@ -23,6 +23,7 @@ static bool shell_archive_configured(void); static bool shell_archive_file(const char *file, const char *path); +static void shell_archive_shutdown(void); void shell_archive_init(ArchiveModuleCallbacks *cb) @@ -31,6 +32,7 @@ shell_archive_init(ArchiveModuleCallbacks *cb) cb->check_configured_cb = shell_archive_configured; cb->archive_file_cb = shell_archive_file; + cb->shutdown_cb = shell_archive_shutdown; } static bool @@ -156,3 +158,10 @@ shell_archive_file(const char *file, const char *path) elog(DEBUG1, "archived write-ahead log file \"%s\"", file); return true; } + +static void +shell_archive_shutdown(void) +{ + ereport(DEBUG1, + (errmsg_internal("archiver process shutting down"))); +} diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/t/020_archive_status.pl index 550fb37cb6..b2c6a127f2 100644 --- a/src/test/recovery/t/020_archive_status.pl +++ b/src/test/recovery/t/020_archive_status.pl @@ -9,6 +9,7 @@ use warnings; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +use Time::HiRes qw(usleep); my $primary = PostgreSQL::Test::Cluster->new('primary'); $primary->init( @@ -234,4 +235,22 @@ ok( -f "$standby2_data/$segment_path_1_done" ".done files created after archive success with archive_mode=always on standby" ); +# Check that the archiver process calls the shell archive module's shutdown +# callback. +$standby2->append_conf('postgresql.conf', "log_min_messages = debug1"); +$standby2->reload; + +$standby2->stop(); + +# wait for postgres to terminate +foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default) +{ + last if !-f $standby2->data_dir . '/postmaster.pid'; + usleep(100_000); +} + +my $logfile = slurp_file($standby2->logfile()); +ok($logfile =~ qr/archiver process shutting down/, + 'check shutdown callback of shell archive module'); + done_testing(); -- 2.34.1