Set the process latch when processing recovery conflict interrupts.

anarazel · anarazel · commit 05d22d06ae0f · 2014-06-03T14:19:29.000+02:00
Because RecoveryConflictInterrupt() didn't set the process latch
anything using the latter to wait for events didn't get notified about
recovery conflicts. Most latch users are never the target of recovery
conflicts, which explains the lack of reports about this until
now.
Since 9.3 two possible affected users exist though: The sql callable
pg_sleep() now uses latches to wait and background workers are
expected to use latches in their main loop. Both would currently wait
until the end of WaitLatch's timeout.

Fix by adding a SetLatch() to RecoveryConflictInterrupt(). It'd also
be possible to fix the issue by having each latch user set
set_latch_on_sigusr1. That seems failure prone and though, as most of
these callsites won't often receive recovery conflicts and thus will
likely only be tested against normal query cancels et al. It'd also be
unnecessarily verbose.

Backpatch to 9.1 where latches were introduced. Arguably 9.3 would be
sufficient, because that's where pg_sleep() was converted to waiting
on the latch and background workers got introduced; but there could be
user level code making use of the latch pre 9.3.
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
@@ -2848,6 +2848,16 @@ RecoveryConflictInterrupt(ProcSignalReason reason)
 		}
 	}
 
+	/*
+	 * Set the process latch. This function essentially emulates signal
+	 * handlers like die() and StatementCancelHandler() and it seems prudent
+	 * to behave similarly as they do. Alternatively all plain backend code
+	 * waiting on that latch, expecting to get interrupted by query cancels et
+	 * al., would also need to set set_latch_on_sigusr1.
+	 */
+	if (MyProc)
+		SetLatch(&MyProc->procLatch);
+
 	errno = save_errno;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -2848,6 +2848,16 @@ RecoveryConflictInterrupt(ProcSignalReason reason)`
`2848`	`2848`	`}`
`2849`	`2849`	`}`
`2850`	`2850`
	`2851`	`+ /*`
	`2852`	`+ * Set the process latch. This function essentially emulates signal`
	`2853`	`+ * handlers like die() and StatementCancelHandler() and it seems prudent`
	`2854`	`+ * to behave similarly as they do. Alternatively all plain backend code`
	`2855`	`+ * waiting on that latch, expecting to get interrupted by query cancels et`
	`2856`	`+ * al., would also need to set set_latch_on_sigusr1.`
	`2857`	`+ */`
	`2858`	`+ if (MyProc)`
	`2859`	`+ SetLatch(&MyProc->procLatch);`
	`2860`	`+`
`2851`	`2861`	`errno = save_errno;`
`2852`	`2862`	`}`
`2853`	`2863`