Hi hackers,
Presently, when an archive module sets up a shutdown callback, it will be
called upon ERROR/FATAL (via PG_ENSURE_ERROR_CLEANUP), when the archive
library changes (via HandlePgArchInterrupts()), and upon normal shutdown.
There are a couple of problems with this:
* HandlePgArchInterrupts() calls the shutdown callback directly before
proc_exit(). However, the PG_ENSURE_ERROR_CLEANUP surrounding the call to
pgarch_MainLoop() sets up a before_shmem_exit callback that also calls the
shutdown callback. This means that the shutdown callback will be called
twice whenever archive_library is changed via SIGHUP.
* PG_ENSURE_ERROR_CLEANUP is intended for both ERROR and FATAL. However,
the archiver operates at the bottom of the exception stack, so ERRORs are
treated as FATALs. This means that PG_ENSURE_ERROR_CLEANUP is excessive.
We only need to set up the before_shmem_exit callback.
To fix, the attached patch removes the use of PG_ENSURE_ERROR_CLEANUP and
the call to the shutdown callback in HandlePgArchInterrupts() in favor of
just setting up a before_shmem_exit callback in LoadArchiveLibrary().
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 3868cd7bd3..7b6d48f7c5 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -252,13 +252,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 +797,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
@@ -858,6 +846,8 @@ LoadArchiveLibrary(void)
if (ArchiveContext.archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+
+ before_shmem_exit(call_archive_module_shutdown_callback, 0);
}
/*
