postgres.git / summary / log / commit / refs
commit 7cb05dd2d1989289b64ef3098f08bdc72b5fba1e
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed Nov 12 18:59:28 2025 +0000
First released in: 14.21, 15.16, 16.12, 17.8, 18.2
Escalate ERRORs during async notify processing to FATAL
Previously, if async notify processing encountered an error, we would
report the error to the client and advance our read position past the
offending entry to prevent trying to process it over and over
again. Trying to continue after an error has a few problems however:
- We have no way of telling the client that a notification was
lost. They get an ERROR, but that doesn't tell you much. As such,
it's not clear if keeping the connection alive after losing a
notification is a good thing. Depending on the application logic,
missing a notification could cause the application to get stuck
waiting, for example.
- If the connection is idle, PqCommReadingMsg is set and any ERROR is
turned into FATAL anyway.
- We bailed out of the notification processing loop on first error
without processing any subsequent notifications. The subsequent
notifications would not be processed until another notify interrupt
arrives. For example, if there were two notifications pending, and
processing the first one caused an ERROR, the second notification
would not be processed until someone sent a new NOTIFY.
This commit changes the behavior so that any ERROR while processing
async notifications is turned into FATAL, causing the client
connection to be terminated. That makes the behavior more consistent
as that's what happened in idle state already, and terminating the
connection is a clear signal to the application that it might've
missed some notifications.
The reason to do this now is that the next commits will change the
notification processing code in a way that would make it harder to
skip over just the offending notification entry on error.
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>
Discussion: https://www.postgresql.org/message-id/fedbd908-4571-4bbe-b48e-63bfdcc38f64@iki.fi
Backpatch-through: 14
src/backend/commands/async.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index fff1aad83ed..5684901fb7a 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -461,7 +461,7 @@ static double asyncQueueUsage(void);
static void asyncQueueFillWarning(void);
static void SignalBackends(void);
static void asyncQueueReadAllNotifications(void);
-static bool asyncQueueProcessPageEntries(volatile QueuePosition *current,
+static bool asyncQueueProcessPageEntries(QueuePosition *current,
QueuePosition stop,
char *page_buffer,
Snapshot snapshot);
@@ -1913,7 +1913,7 @@ ProcessNotifyInterrupt(bool flush)
static void
asyncQueueReadAllNotifications(void)
{
- volatile QueuePosition pos;
+ QueuePosition pos;
QueuePosition head;
Snapshot snapshot;
@@ -1983,16 +1983,25 @@ asyncQueueReadAllNotifications(void)
* It is possible that we fail while trying to send a message to our
* frontend (for example, because of encoding conversion failure). If
* that happens it is critical that we not try to send the same message
- * over and over again. Therefore, we place a PG_TRY block here that will
- * forcibly advance our queue position before we lose control to an error.
- * (We could alternatively retake NotifyQueueLock and move the position
- * before handling each individual message, but that seems like too much
- * lock traffic.)
+ * over and over again. Therefore, we set ExitOnAnyError to upgrade any
+ * ERRORs to FATAL, causing the client connection to be closed on error.
+ *
+ * We used to only skip over the offending message and try to soldier on,
+ * but it was somewhat questionable to lose a notification and give the
+ * client an ERROR instead. A client application is not be prepared for
+ * that and can't tell that a notification was missed. It was also not
+ * very useful in practice because notifications are often processed while
+ * a connection is idle and reading a message from the client, and in that
+ * state, any error is upgraded to FATAL anyway. Closing the connection
+ * is a clear signal to the application that it might have missed
+ * notifications.
*/
- PG_TRY();
{
+ bool save_ExitOnAnyError = ExitOnAnyError;
bool reachedStop;
+ ExitOnAnyError = true;
+
do
{
int curpage = QUEUE_POS_PAGE(pos);
@@ -2045,15 +2054,14 @@ asyncQueueReadAllNotifications(void)
page_buffer.buf,
snapshot);
} while (!reachedStop);
- }
- PG_FINALLY();
- {
+
/* Update shared state */
LWLockAcquire(NotifyQueueLock, LW_SHARED);
QUEUE_BACKEND_POS(MyBackendId) = pos;
LWLockRelease(NotifyQueueLock);
+
+ ExitOnAnyError = save_ExitOnAnyError;
}
- PG_END_TRY();
/* Done with snapshot */
UnregisterSnapshot(snapshot);
@@ -2076,7 +2084,7 @@ asyncQueueReadAllNotifications(void)
* The QueuePosition *current is advanced past all processed messages.
*/
static bool
-asyncQueueProcessPageEntries(volatile QueuePosition *current,
+asyncQueueProcessPageEntries(QueuePosition *current,
QueuePosition stop,
char *page_buffer,
Snapshot snapshot)
[parent: 4ef048f34d2f]