On 2021/03/31 19:51, Maxim Orlov wrote:
On 2021-03-30 20:44, Maxim Orlov wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested
All the tests passed successfully.
The new status of this patch is: Ready for Committer
The patch is good. One note, should we put a comment about
ShutdownRecoveryTransactionEnvironment not reentrant behaviour? Or maybe rename
it to ShutdownRecoveryTransactionEnvironmentOnce?
+1 to add more comments into ShutdownRecoveryTransactionEnvironment().
I did that. What about the attached patch?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index 22135d5e07..69077bd207 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -62,6 +62,9 @@ static volatile sig_atomic_t in_restore_command = false;
static void StartupProcTriggerHandler(SIGNAL_ARGS);
static void StartupProcSigHupHandler(SIGNAL_ARGS);
+/* Callbacks */
+static void StartupProcExit(int code, Datum arg);
+
/* --------------------------------
* signal handler routines
@@ -183,6 +186,19 @@ HandleStartupProcInterrupts(void)
}
+/* --------------------------------
+ * signal handler routines
+ * --------------------------------
+ */
+static void
+StartupProcExit(int code, Datum arg)
+{
+ /* Shutdown the recovery environment */
+ if (standbyState != STANDBY_DISABLED)
+ ShutdownRecoveryTransactionEnvironment();
+}
+
+
/* ----------------------------------
* Startup Process main entry point
* ----------------------------------
@@ -190,6 +206,9 @@ HandleStartupProcInterrupts(void)
void
StartupProcessMain(void)
{
+ /* Arrange to clean up at startup process exit */
+ on_shmem_exit(StartupProcExit, 0);
+
/*
* Properly accept or ignore signals the postmaster might send us.
*/
diff --git a/src/backend/storage/ipc/standby.c
b/src/backend/storage/ipc/standby.c
index 17de5a6d0e..1465ee44a1 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -127,10 +127,25 @@ InitRecoveryTransactionEnvironment(void)
*
* Prepare to switch from hot standby mode to normal operation. Shut down
* recovery-time transaction tracking.
+ *
+ * This must be called even in shutdown of startup process if transaction
+ * tracking has been initialized. Otherwise some locks the tracked
+ * transactions were holding will not be released and and may interfere with
+ * the processes still running (but will exit soon later) at the exit of
+ * startup process.
*/
void
ShutdownRecoveryTransactionEnvironment(void)
{
+ /*
+ * Do nothing if RecoveryLockLists is NULL because which means that
+ * transaction tracking has not been yet initialized or has been already
+ * shutdowned. This prevents transaction tracking from being shutdowned
+ * unexpectedly more than once.
+ */
+ if (RecoveryLockLists == NULL)
+ return;
+
/* Mark all tracked in-progress transactions as finished. */
ExpireAllKnownAssignedTransactionIds();
