shutdown_without_completion.v1.patch

text/x-patch

Filename: shutdown_without_completion.v1.patch
Type: text/x-patch
Part: 0
Message: Re: Sync Rep v19

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
Series: patch v1
File+
src/backend/replication/syncrep.c 39 13
src/backend/tcop/dest.c 35 0
src/backend/tcop/postgres.c 6 30
src/include/miscadmin.h 1 1
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index ac82ebb..c6e3093 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -67,7 +67,7 @@ bool	sync_rep_mode = false;			/* Only set in user backends */
 int		sync_rep_timeout = 120;			/* Only set in user backends */
 char 	*SyncRepStandbyNames;
 
-bool	WaitingForSyncRep = false;	/* Global state for some exit methods */
+bool	ExitSilentlyAtEndCommand = false;	/* Global state for some exit methods */
 
 static bool announce_next_takeover = true;
 
@@ -105,7 +105,7 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
 void
 SyncRepCleanupAtProcExit(int code, Datum arg)
 {
-	if (WaitingForSyncRep && !SHMQueueIsDetached(&(MyProc->syncrep_links)))
+	if (!SHMQueueIsDetached(&(MyProc->syncrep_links)))
 	{
 		LWLockAcquire(SyncRepLock, LW_EXCLUSIVE);
 		SHMQueueDelete(&(MyProc->syncrep_links));
@@ -194,7 +194,6 @@ SyncRepWaitOnQueue(XLogRecPtr XactCommitLSN)
 			MyProc->waitLSN = XactCommitLSN;
 			SHMQueueInsertBefore(&(WalSndCtl->SyncRepQueue), &(MyProc->syncrep_links));
 			LWLockRelease(SyncRepLock);
-			WaitingForSyncRep = true;
 
 			/*
 			 * Alter ps display to show waiting for sync rep.
@@ -222,7 +221,7 @@ SyncRepWaitOnQueue(XLogRecPtr XactCommitLSN)
 			 * unlikely to be far enough, yet is possible. Next time we are
 			 * woken we should be more lucky.
 			 */
-			if (XLByteLE(XactCommitLSN, walsndctl->lsn))
+			if (XLByteLE(XactCommitLSN, walsndctl->lsn) || ProcDiePending)
 				release = true;
 			else if (timeout > 0 &&
 				TimestampDifferenceExceeds(wait_start, now, timeout))
@@ -235,7 +234,6 @@ SyncRepWaitOnQueue(XLogRecPtr XactCommitLSN)
 			{
 				SHMQueueDelete(&(MyProc->syncrep_links));
 				LWLockRelease(SyncRepLock);
-				WaitingForSyncRep = false;
 
 				/*
 				 * Reset our waitLSN.
@@ -251,17 +249,39 @@ SyncRepWaitOnQueue(XLogRecPtr XactCommitLSN)
 				}
 
 				/*
-				 * Our response to the timeout is to simply post a NOTICE and
-				 * then return to the user. The commit has happened, we just
-				 * haven't been able to verify it has been replicated in the
-				 * way requested.
+				 * Our response to the timeout is to simply post a NOTICE
+				 * and then return to the user. The commit has happened, we
+				 * just haven't been able to verify it has been replicated
+				 * in the way requested.
 				 */
 				if (timed_out)
+				{
 					ereport(NOTICE,
-							(errmsg("synchronous replication wait for %X/%X timeout at %s",
+						(errmsg("timeout of synchronous replication wait for %X/%X",
 										XactCommitLSN.xlogid,
-										XactCommitLSN.xrecoff,
-										timestamptz_to_str(now))));
+										XactCommitLSN.xrecoff),
+						errdetail("Synchronous replication was requested"
+							" yet confirmation of replication has not been"
+							" received from the specified standby server."
+							" The transaction has committed locally and might"
+							" also be committed on some recently disconnected"
+							" standby servers.")));
+				}
+				else if (ProcDiePending)
+				{
+					/*
+					 * Cancel the interrupt state and return to normal
+					 * processing until we hit EndCommand(), where we
+					 * exit without returning anything to client.
+					 * This ensures that we don't break our guarantee that
+					 * a commit message means the data is safe. Note that
+					 * the guarantee doesn't work the other way around,
+					 * the absence of a commit message doesn't mean it
+					 * didn't commit.
+					 */
+					ProcDiePending = false;
+					ExitSilentlyAtEndCommand = true;
+				}
 				else
 					ereport(DEBUG3,
 							(errmsg("synchronous replication wait for %X/%X complete at %s",
@@ -274,7 +294,13 @@ SyncRepWaitOnQueue(XLogRecPtr XactCommitLSN)
 			LWLockRelease(SyncRepLock);
 		}
 
-		WaitLatch(&MyProc->waitLatch, timeout);
+		/*
+		 * If we've received a signal to shutdown or a specific signal to
+		 * terminate this backend, don't wait, just loop straight back
+		 * round to remove ourselves from the queue and go.
+		 */
+		if (!ProcDiePending)
+			WaitLatch(&MyProc->waitLatch, timeout);
 	}
 }
 
diff --git a/src/backend/tcop/dest.c b/src/backend/tcop/dest.c
index 24af3fb..2e8e17f 100644
--- a/src/backend/tcop/dest.c
+++ b/src/backend/tcop/dest.c
@@ -36,6 +36,8 @@
 #include "executor/tstoreReceiver.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
+#include "miscadmin.h"
+#include "storage/ipc.h"
 #include "utils/portal.h"
 
 
@@ -143,6 +145,39 @@ EndCommand(const char *commandTag, CommandDest dest)
 		case DestRemote:
 		case DestRemoteExecute:
 
+			if (ExitSilentlyAtEndCommand)
+			{
+				/*
+				 * This must NOT be a FATAL message. We want the state of the
+				 * transaction being aborted to be indeterminate to ensure that
+				 * the transaction completion guarantee is never broken.
+				 */
+				ereport(WARNING,
+					(errcode(ERRCODE_ADMIN_SHUTDOWN),
+				 	errmsg("terminating connection because of fast shutdown"
+						" while waiting for synchronous replication"),
+					errdetail("Synchronous replication was requested"
+						" yet confirmation of replication has not been"
+						" received from the specified standby server."
+						" The transaction has committed locally and might"
+						" also be committed on some recently disconnected"
+						" standby servers.")));
+
+				/*
+				 * We DO NOT want to run proc_exit() callbacks -- we're here because
+				 * we are shutting down and don't want any code to stall or
+				 * prevent that.
+				 */
+				on_exit_reset();
+
+				/*
+				 * Note we do exit(0) not exit(>0). This is to avoid forcing
+				 * postmaster into a system reset cycle if we are the only
+				 * backend that received a SIGTERM.
+				 */
+				exit(0);
+			}
+
 			/*
 			 * We assume the commandTag is plain ASCII and therefore requires
 			 * no encoding conversion.
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 5d86deb..7d19b26 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2628,6 +2628,11 @@ die(SIGNAL_ARGS)
 		ProcDiePending = true;
 
 		/*
+		 * Set this proc's wait latch to stop waiting
+		 */
+		SetLatch(&(MyProc->waitLatch));
+
+		/*
 		 * If it's safe to interrupt, and we're waiting for input or a lock,
 		 * service the interrupt immediately
 		 */
@@ -2852,8 +2857,7 @@ ProcessInterrupts(void)
 	 * For SyncRep, we want to accept SIGTERM signals while other interrupts
 	 * are held, so we have a special case solely when WaitingForSyncRep.
 	 */
-	if ((InterruptHoldoffCount != 0 || CritSectionCount != 0) &&
-		!(WaitingForSyncRep && ProcDiePending))
+	if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
 		return;
 	InterruptPending = false;
 	if (ProcDiePending)
@@ -2870,34 +2874,6 @@ ProcessInterrupts(void)
 			ereport(FATAL,
 					(errcode(ERRCODE_ADMIN_SHUTDOWN),
 					 errmsg("terminating autovacuum process due to administrator command")));
-		else if (WaitingForSyncRep)
-		{
-			/*
-			 * This must NOT be a FATAL message. We want the state of the
-			 * transaction being aborted to be indeterminate to ensure that
-			 * the transaction completion guarantee is never broken.
-			 */
-			ereport(WARNING,
-					(errcode(ERRCODE_ADMIN_SHUTDOWN),
-					 errmsg("terminating connection because fast shutdown is requested"),
-			errdetail("This connection requested synchronous replication at commit"
-					  " yet confirmation of replication has not been received."
-					  " The transaction has committed locally and might be committed"
-					  " on recently disconnected standby servers also.")));
-
-			/*
-			 * We DO NOT want to run proc_exit() callbacks -- we're here because
-			 * we are shutting down and don't want any code to stall or
-			 * prevent that.
-			 */
-			on_exit_reset();
-
-			/*
-			 * Note we do exit(0) not exit(>0). This is to avoid forcing
-			 * postmaster into a system reset cycle.
-			 */
-			exit(0);
-		}
 		else if (RecoveryConflictPending && RecoveryConflictRetryable)
 		{
 			pgstat_report_recovery_conflict(RecoveryConflictReason);
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index c2552e7..16067a5 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -79,7 +79,7 @@ extern PGDLLIMPORT volatile uint32 CritSectionCount;
 extern void ProcessInterrupts(void);
 
 /* in replication/syncrep.c */
-extern bool WaitingForSyncRep;
+extern bool ExitSilentlyAtEndCommand;
 
 #ifndef WIN32