Thanks for the review!
On 23/01/2026 04:41, Chao Li wrote:
On Jan 23, 2026, at 07:20, Heikki Linnakangas <[email protected]> wrote:
Patch 0001: Remove useless errdetail_abort()
--------------------------------------------
...
Has anyone seen the "DETAIL: Abort reason: recovery conflict" in recent years,
or ever? If not, let's rip it out.
I did a Google search and couldn't find any post reporting the message, which
seems to prove that message can be removed.
0002: Don't hint that you can reconnect when the database is dropped
--------------------------------------------------------------------
If you're connected to a database is being dropped, during recovery, you get an
error like this:
FATAL: terminating connection due to conflict with recovery
DETAIL: User was connected to a database that must be dropped.
HINT: In a moment you should be able to reconnect to the database and repeat
your command.
The hint seems misleading. The database is being dropped, you most likely can
*not* reconnect to it. Let's remove it.
I like this change. Not only removing the misleading error message, the code is
also clearer now.
Pushed patches 0001 and 0002.
1 - 0003
```
ReplicationSlotAcquire(const char *name, bool nowait, bool error_if_invalid)
{
ReplicationSlot *s;
- int active_pid;
+ ProcNumber active_proc;
+ pid_t active_pid;
```
Active_pid is only used inside the "if (active_proc != MyProcNumber)” clause,
so it can be only defined within the “if” clause.
It needs to fetched while still holding the lock. We could arrange the
code to avoid fetching it when active_proc == MyProcNumber, but it seems
it would be less clear, and this isn't performance critical.
2 - 0003
```
if (MyBackendType == B_STARTUP)
- (void) SendProcSignal(active_pid,
-
PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT,
-
INVALID_PROC_NUMBER);
+ SendProcSignal(active_pid,
+
PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT,
+ active_proc);
```
Here active_proc!=INVALID_PROC_NUMBER, so this changes the original logic
without an explanation. Is the change intentional?
Yes, it's intentional. SendProcSignal() is more efficient when you pass
procNumber argument. We didn't pass it here before because we didn't
have it, but now we do.
3 - 0004
```
+ * This is a bitmask of RecoveryConflictReasons
+ */
+ pg_atomic_uint32 pendingRecoveryConflicts;
```
I just feel this comment is a little bit confusing because
RecoveryConflictReasons is an enum. Maybe we can say something like: This is a
bitmask; each bit corresponds to one RecoveryConflictReason enum value.
Ok, adopted that wording.
4 - 0004
```
- (void) SendProcSignal(pid, sigmode,
vxid.procNumber);
+ (void) SendProcSignal(pid,
PROCSIG_RECOVERY_CONFLICT, vxid.procNumber);
```
Nit: Here (void) is retained for SendProcSignal, but in the place of commit 2
for 0003, (void) is deleted when calling SendProcSignal, is there any reason
for retaining this one and deleting that one?
I added the (void) back to the SendProcSignal call in 0003. We're not
very consistent about that, there are SendProcSignal calls with and
without the (void) in the codebase. But there was no particular reason
to change it in this patch.
7 - 0005
```
+static void
+report_recovery_conflict(RecoveryConflictReason reason)
+{
+ bool fatal;
+ if (RECOVERY_CONFLICT_DATABASE)
+ {
```
I believe this should be if (reason == RECOVERY_CONFLICT_DATABASE).
Oops yes, fixed!
Attached are new versions of the remaining patches, with the above and
the typo and whitespace fixes that you pointed out.
- Heikki
From 86d719df88f1681822ef63fa1532f96a044a792d Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 3 Feb 2026 15:26:14 +0200
Subject: [PATCH v2 1/3] Use ProcNumber rather than pid in ReplicationSlot
This helps the next commit.
Reviewed-by: Chao Li <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
---
src/backend/replication/logical/slotsync.c | 2 +-
src/backend/replication/slot.c | 67 ++++++++++++----------
src/backend/replication/slotfuncs.c | 13 +++--
src/include/replication/slot.h | 7 ++-
4 files changed, 51 insertions(+), 38 deletions(-)
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index 1c343d03d21..60fc756b509 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1759,7 +1759,7 @@ update_synced_slots_inactive_since(void)
Assert(SlotIsLogical(s));
/* The slot must not be acquired by any process */
- Assert(s->active_pid == 0);
+ Assert(s->active_proc == INVALID_PROC_NUMBER);
/* Use the same inactive_since time for all the slots. */
if (now == 0)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 4c47261c7f9..224978095d6 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -226,6 +226,7 @@ ReplicationSlotsShmemInit(void)
ReplicationSlot *slot = &ReplicationSlotCtl->replication_slots[i];
/* everything else is zeroed by the memset above */
+ slot->active_proc = INVALID_PROC_NUMBER;
SpinLockInit(&slot->mutex);
LWLockInitialize(&slot->io_in_progress_lock,
LWTRANCHE_REPLICATION_SLOT_IO);
@@ -461,7 +462,7 @@ ReplicationSlotCreate(const char *name, bool db_specific,
* be doing that. So it's safe to initialize the slot.
*/
Assert(!slot->in_use);
- Assert(slot->active_pid == 0);
+ Assert(slot->active_proc == INVALID_PROC_NUMBER);
/* first initialize persistent data */
memset(&slot->data, 0, sizeof(ReplicationSlotPersistentData));
@@ -505,8 +506,8 @@ ReplicationSlotCreate(const char *name, bool db_specific,
/* We can now mark the slot active, and that makes it our slot. */
SpinLockAcquire(&slot->mutex);
- Assert(slot->active_pid == 0);
- slot->active_pid = MyProcPid;
+ Assert(slot->active_proc == INVALID_PROC_NUMBER);
+ slot->active_proc = MyProcNumber;
SpinLockRelease(&slot->mutex);
MyReplicationSlot = slot;
@@ -620,7 +621,8 @@ void
ReplicationSlotAcquire(const char *name, bool nowait, bool error_if_invalid)
{
ReplicationSlot *s;
- int active_pid;
+ ProcNumber active_proc;
+ pid_t active_pid;
Assert(name != NULL);
@@ -672,17 +674,18 @@ retry:
* to inactive_since in InvalidatePossiblyObsoleteSlot.
*/
SpinLockAcquire(&s->mutex);
- if (s->active_pid == 0)
- s->active_pid = MyProcPid;
- active_pid = s->active_pid;
+ if (s->active_proc == INVALID_PROC_NUMBER)
+ s->active_proc = MyProcNumber;
+ active_proc = s->active_proc;
ReplicationSlotSetInactiveSince(s, 0, false);
SpinLockRelease(&s->mutex);
}
else
{
- s->active_pid = active_pid = MyProcPid;
+ s->active_proc = active_proc = MyProcNumber;
ReplicationSlotSetInactiveSince(s, 0, true);
}
+ active_pid = GetPGProcByNumber(active_proc)->pid;
LWLockRelease(ReplicationSlotControlLock);
/*
@@ -690,7 +693,7 @@ retry:
* wait until the owning process signals us that it's been released, or
* error out.
*/
- if (active_pid != MyProcPid)
+ if (active_proc != MyProcNumber)
{
if (!nowait)
{
@@ -762,7 +765,7 @@ ReplicationSlotRelease(void)
bool is_logical;
TimestampTz now = 0;
- Assert(slot != NULL && slot->active_pid != 0);
+ Assert(slot != NULL && slot->active_proc != INVALID_PROC_NUMBER);
is_logical = SlotIsLogical(slot);
@@ -815,7 +818,7 @@ ReplicationSlotRelease(void)
* disconnecting, but wake up others that may be waiting for it.
*/
SpinLockAcquire(&slot->mutex);
- slot->active_pid = 0;
+ slot->active_proc = INVALID_PROC_NUMBER;
ReplicationSlotSetInactiveSince(slot, now, false);
SpinLockRelease(&slot->mutex);
ConditionVariableBroadcast(&slot->active_cv);
@@ -877,7 +880,7 @@ restart:
found_valid_logicalslot |=
(SlotIsLogical(s) && s->data.invalidated == RS_INVAL_NONE);
- if ((s->active_pid == MyProcPid &&
+ if ((s->active_proc == MyProcNumber &&
(!synced_only || s->data.synced)))
{
Assert(s->data.persistency == RS_TEMPORARY);
@@ -1088,7 +1091,7 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
bool fail_softly = slot->data.persistency != RS_PERSISTENT;
SpinLockAcquire(&slot->mutex);
- slot->active_pid = 0;
+ slot->active_proc = INVALID_PROC_NUMBER;
SpinLockRelease(&slot->mutex);
/* wake up anyone waiting on this slot */
@@ -1110,7 +1113,7 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
* Also wake up processes waiting for it.
*/
LWLockAcquire(ReplicationSlotControlLock, LW_EXCLUSIVE);
- slot->active_pid = 0;
+ slot->active_proc = INVALID_PROC_NUMBER;
slot->in_use = false;
LWLockRelease(ReplicationSlotControlLock);
ConditionVariableBroadcast(&slot->active_cv);
@@ -1476,7 +1479,7 @@ ReplicationSlotsCountDBSlots(Oid dboid, int *nslots, int *nactive)
/* count slots with spinlock held */
SpinLockAcquire(&s->mutex);
(*nslots)++;
- if (s->active_pid != 0)
+ if (s->active_proc != INVALID_PROC_NUMBER)
(*nactive)++;
SpinLockRelease(&s->mutex);
}
@@ -1520,7 +1523,7 @@ restart:
{
ReplicationSlot *s;
char *slotname;
- int active_pid;
+ ProcNumber active_proc;
s = &ReplicationSlotCtl->replication_slots[i];
@@ -1550,11 +1553,11 @@ restart:
SpinLockAcquire(&s->mutex);
/* can't change while ReplicationSlotControlLock is held */
slotname = NameStr(s->data.name);
- active_pid = s->active_pid;
- if (active_pid == 0)
+ active_proc = s->active_proc;
+ if (active_proc == INVALID_PROC_NUMBER)
{
MyReplicationSlot = s;
- s->active_pid = MyProcPid;
+ s->active_proc = MyProcNumber;
}
SpinLockRelease(&s->mutex);
@@ -1579,11 +1582,11 @@ restart:
* XXX: We can consider shutting down the slot sync worker before
* trying to drop synced temporary slots here.
*/
- if (active_pid)
+ if (active_proc != INVALID_PROC_NUMBER)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_IN_USE),
errmsg("replication slot \"%s\" is active for PID %d",
- slotname, active_pid)));
+ slotname, GetPGProcByNumber(active_proc)->pid)));
/*
* To avoid duplicating ReplicationSlotDropAcquired() and to avoid
@@ -1974,7 +1977,8 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
{
XLogRecPtr restart_lsn;
NameData slotname;
- int active_pid = 0;
+ ProcNumber active_proc;
+ pid_t active_pid = 0;
ReplicationSlotInvalidationCause invalidation_cause = RS_INVAL_NONE;
TimestampTz now = 0;
long slot_idle_secs = 0;
@@ -2027,7 +2031,7 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
}
slotname = s->data.name;
- active_pid = s->active_pid;
+ active_proc = s->active_proc;
/*
* If the slot can be acquired, do so and mark it invalidated
@@ -2039,10 +2043,10 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
* is terminated. So, the inactive slot can only be invalidated
* immediately without being terminated.
*/
- if (active_pid == 0)
+ if (active_proc == INVALID_PROC_NUMBER)
{
MyReplicationSlot = s;
- s->active_pid = MyProcPid;
+ s->active_proc = MyProcNumber;
s->data.invalidated = invalidation_cause;
/*
@@ -2058,6 +2062,11 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
/* Let caller know */
invalidated = true;
}
+ else
+ {
+ active_pid = GetPGProcByNumber(active_proc)->pid;
+ Assert(active_pid != 0);
+ }
SpinLockRelease(&s->mutex);
@@ -2073,7 +2082,7 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
&slot_idle_usecs);
}
- if (active_pid != 0)
+ if (active_proc != INVALID_PROC_NUMBER)
{
/*
* Prepare the sleep on the slot's condition variable before
@@ -2107,7 +2116,7 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
if (MyBackendType == B_STARTUP)
(void) SendProcSignal(active_pid,
PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT,
- INVALID_PROC_NUMBER);
+ active_proc);
else
(void) kill(active_pid, SIGTERM);
@@ -2875,7 +2884,7 @@ RestoreSlotFromDisk(const char *name)
slot->candidate_restart_valid = InvalidXLogRecPtr;
slot->in_use = true;
- slot->active_pid = 0;
+ slot->active_proc = INVALID_PROC_NUMBER;
/*
* Set the time since the slot has become inactive after loading the
@@ -3158,7 +3167,7 @@ StandbySlotsHaveCaughtup(XLogRecPtr wait_for_lsn, int elevel)
SpinLockAcquire(&slot->mutex);
restart_lsn = slot->data.restart_lsn;
invalidated = slot->data.invalidated != RS_INVAL_NONE;
- inactive = slot->active_pid == 0;
+ inactive = slot->active_proc == INVALID_PROC_NUMBER;
SpinLockRelease(&slot->mutex);
if (invalidated)
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 1ed2d80c2d2..9f5e4f998fe 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -20,6 +20,7 @@
#include "replication/logical.h"
#include "replication/slot.h"
#include "replication/slotsync.h"
+#include "storage/proc.h"
#include "utils/builtins.h"
#include "utils/guc.h"
#include "utils/pg_lsn.h"
@@ -309,10 +310,10 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
values[i++] = ObjectIdGetDatum(slot_contents.data.database);
values[i++] = BoolGetDatum(slot_contents.data.persistency == RS_TEMPORARY);
- values[i++] = BoolGetDatum(slot_contents.active_pid != 0);
+ values[i++] = BoolGetDatum(slot_contents.active_proc != INVALID_PROC_NUMBER);
- if (slot_contents.active_pid != 0)
- values[i++] = Int32GetDatum(slot_contents.active_pid);
+ if (slot_contents.active_proc != INVALID_PROC_NUMBER)
+ values[i++] = Int32GetDatum(GetPGProcByNumber(slot_contents.active_proc)->pid);
else
nulls[i++] = true;
@@ -377,13 +378,13 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
*/
if (XLogRecPtrIsValid(slot_contents.data.restart_lsn))
{
- int pid;
+ ProcNumber procno;
SpinLockAcquire(&slot->mutex);
- pid = slot->active_pid;
+ procno = slot->active_proc;
slot_contents.data.restart_lsn = slot->data.restart_lsn;
SpinLockRelease(&slot->mutex);
- if (pid != 0)
+ if (procno != INVALID_PROC_NUMBER)
{
values[i++] = CStringGetTextDatum("unreserved");
walstate = WALAVAIL_UNRESERVED;
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index f465e430cc6..72f8be629f3 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -185,8 +185,11 @@ typedef struct ReplicationSlot
/* is this slot defined */
bool in_use;
- /* Who is streaming out changes for this slot? 0 in unused slots. */
- pid_t active_pid;
+ /*
+ * Who is streaming out changes for this slot? INVALID_PROC_NUMBER in
+ * unused slots.
+ */
+ ProcNumber active_proc;
/* any outstanding modifications? */
bool just_dirtied;
--
2.47.3
From 7e6dc88aa756b81f6153bb7bf55b81ade2b88c27 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 3 Feb 2026 15:26:17 +0200
Subject: [PATCH v2 2/3] Separate RecoveryConflictReasons from procsignals
Share the same PROCSIG_RECOVERY_CONFLICT flag for all recovery
conflict reasons. To distinguish, have a bitmask in PGPROC to indicate
the reason(s).
Reviewed-by: Chao Li <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
---
src/backend/commands/dbcommands.c | 1 +
src/backend/commands/tablespace.c | 1 +
src/backend/replication/logical/logicalctl.c | 1 +
src/backend/replication/slot.c | 6 +-
src/backend/storage/buffer/bufmgr.c | 5 +-
src/backend/storage/ipc/procarray.c | 136 +++++++++++++------
src/backend/storage/ipc/procsignal.c | 22 +--
src/backend/storage/ipc/standby.c | 61 ++++-----
src/backend/storage/lmgr/proc.c | 5 +-
src/backend/tcop/postgres.c | 109 ++++++++-------
src/backend/utils/activity/pgstat_database.c | 18 +--
src/backend/utils/adt/mcxtfuncs.c | 1 +
src/include/storage/proc.h | 10 ++
src/include/storage/procarray.h | 7 +-
src/include/storage/procsignal.h | 16 +--
src/include/storage/standby.h | 34 ++++-
src/include/tcop/tcopprot.h | 2 +-
src/tools/pgindent/typedefs.list | 1 +
18 files changed, 251 insertions(+), 185 deletions(-)
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 87949054f26..33311760df7 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -60,6 +60,7 @@
#include "storage/lmgr.h"
#include "storage/md.h"
#include "storage/procarray.h"
+#include "storage/procsignal.h"
#include "storage/smgr.h"
#include "utils/acl.h"
#include "utils/builtins.h"
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 0b064891932..3511a4ec0fd 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -70,6 +70,7 @@
#include "miscadmin.h"
#include "postmaster/bgwriter.h"
#include "storage/fd.h"
+#include "storage/procsignal.h"
#include "storage/standby.h"
#include "utils/acl.h"
#include "utils/builtins.h"
diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c
index 9f787f3dc51..4e292951201 100644
--- a/src/backend/replication/logical/logicalctl.c
+++ b/src/backend/replication/logical/logicalctl.c
@@ -71,6 +71,7 @@
#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "storage/procsignal.h"
#include "utils/injection_point.h"
/*
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 224978095d6..143569dffe3 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2114,9 +2114,9 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
slot_idle_secs);
if (MyBackendType == B_STARTUP)
- (void) SendProcSignal(active_pid,
- PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT,
- active_proc);
+ (void) SignalRecoveryConflict(GetPGProcByNumber(active_proc),
+ active_pid,
+ RECOVERY_CONFLICT_LOGICALSLOT);
else
(void) kill(active_pid, SIGTERM);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 7241477cac0..d1babaff023 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -59,6 +59,7 @@
#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/proclist.h"
+#include "storage/procsignal.h"
#include "storage/read_stream.h"
#include "storage/smgr.h"
#include "storage/standby.h"
@@ -6570,7 +6571,7 @@ LockBufferForCleanup(Buffer buffer)
* deadlock_timeout for it.
*/
if (logged_recovery_conflict)
- LogRecoveryConflict(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN,
+ LogRecoveryConflict(RECOVERY_CONFLICT_BUFFERPIN,
waitStart, GetCurrentTimestamp(),
NULL, false);
@@ -6621,7 +6622,7 @@ LockBufferForCleanup(Buffer buffer)
if (TimestampDifferenceExceeds(waitStart, now,
DeadlockTimeout))
{
- LogRecoveryConflict(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN,
+ LogRecoveryConflict(RECOVERY_CONFLICT_BUFFERPIN,
waitStart, now, NULL, true);
logged_recovery_conflict = true;
}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 748c06b51cb..423ca3aeaa1 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -60,6 +60,7 @@
#include "port/pg_lfind.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "storage/procsignal.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/injection_point.h"
@@ -708,6 +709,8 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
/* be sure this is cleared in abort */
proc->delayChkptFlags = 0;
+ pg_atomic_write_u32(&proc->pendingRecoveryConflicts, 0);
+
/* must be cleared with xid/xmin: */
/* avoid unnecessarily dirtying shared cachelines */
if (proc->statusFlags & PROC_VACUUM_STATE_MASK)
@@ -748,6 +751,8 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
/* be sure this is cleared in abort */
proc->delayChkptFlags = 0;
+ pg_atomic_write_u32(&proc->pendingRecoveryConflicts, 0);
+
/* must be cleared with xid/xmin: */
/* avoid unnecessarily dirtying shared cachelines */
if (proc->statusFlags & PROC_VACUUM_STATE_MASK)
@@ -929,6 +934,7 @@ ProcArrayClearTransaction(PGPROC *proc)
proc->vxid.lxid = InvalidLocalTransactionId;
proc->xmin = InvalidTransactionId;
+ pg_atomic_write_u32(&proc->pendingRecoveryConflicts, 0);
Assert(!(proc->statusFlags & PROC_VACUUM_STATE_MASK));
Assert(!proc->delayChkptFlags);
@@ -3440,12 +3446,46 @@ GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid)
}
/*
- * SignalVirtualTransaction - used in recovery conflict processing
+ * SignalRecoveryConflict -- signal that a process is blocking recovery
*
- * Returns pid of the process signaled, or 0 if not found.
+ * The 'pid' is redundant with 'proc', but it acts as a cross-check to
+ * detect process had exited and the PGPROC entry was reused for a different
+ * process.
+ *
+ * Returns true if the process was signaled, or false if not found.
*/
-pid_t
-SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
+bool
+SignalRecoveryConflict(PGPROC *proc, pid_t pid, RecoveryConflictReason reason)
+{
+ bool found = false;
+
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
+
+ /*
+ * Kill the pid if it's still here. If not, that's what we wanted so
+ * ignore any errors.
+ */
+ if (proc->pid == pid)
+ {
+ (void) pg_atomic_fetch_or_u32(&proc->pendingRecoveryConflicts, (1 << reason));
+
+ /* wake up the process */
+ (void) SendProcSignal(pid, PROCSIG_RECOVERY_CONFLICT, GetNumberFromPGProc(proc));
+ found = true;
+ }
+
+ LWLockRelease(ProcArrayLock);
+
+ return found;
+}
+
+/*
+ * SignalRecoveryConflictWithVirtualXID -- signal that a VXID is blocking recovery
+ *
+ * Like SignalRecoveryConflict, but the target is identified by VXID
+ */
+bool
+SignalRecoveryConflictWithVirtualXID(VirtualTransactionId vxid, RecoveryConflictReason reason)
{
ProcArrayStruct *arrayP = procArray;
int index;
@@ -3467,11 +3507,13 @@ SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
pid = proc->pid;
if (pid != 0)
{
+ (void) pg_atomic_fetch_or_u32(&proc->pendingRecoveryConflicts, (1 << reason));
+
/*
* Kill the pid if it's still here. If not, that's what we
* wanted so ignore any errors.
*/
- (void) SendProcSignal(pid, sigmode, vxid.procNumber);
+ (void) SendProcSignal(pid, PROCSIG_RECOVERY_CONFLICT, vxid.procNumber);
}
break;
}
@@ -3479,7 +3521,50 @@ SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode)
LWLockRelease(ProcArrayLock);
- return pid;
+ return pid != 0;
+}
+
+/*
+ * SignalRecoveryConflictWithDatabase --- signal all backends specified database
+ *
+ * Like SignalRecoveryConflict, but signals all backends using the database.
+ */
+void
+SignalRecoveryConflictWithDatabase(Oid databaseid, RecoveryConflictReason reason)
+{
+ ProcArrayStruct *arrayP = procArray;
+ int index;
+
+ /* tell all backends to die */
+ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+
+ for (index = 0; index < arrayP->numProcs; index++)
+ {
+ int pgprocno = arrayP->pgprocnos[index];
+ PGPROC *proc = &allProcs[pgprocno];
+
+ if (databaseid == InvalidOid || proc->databaseId == databaseid)
+ {
+ VirtualTransactionId procvxid;
+ pid_t pid;
+
+ GET_VXID_FROM_PGPROC(procvxid, *proc);
+
+ pid = proc->pid;
+ if (pid != 0)
+ {
+ (void) pg_atomic_fetch_or_u32(&proc->pendingRecoveryConflicts, (1 << reason));
+
+ /*
+ * Kill the pid if it's still here. If not, that's what we
+ * wanted so ignore any errors.
+ */
+ (void) SendProcSignal(pid, PROCSIG_RECOVERY_CONFLICT, procvxid.procNumber);
+ }
+ }
+ }
+
+ LWLockRelease(ProcArrayLock);
}
/*
@@ -3601,45 +3686,6 @@ CountDBConnections(Oid databaseid)
return count;
}
-/*
- * CancelDBBackends --- cancel backends that are using specified database
- */
-void
-CancelDBBackends(Oid databaseid, ProcSignalReason sigmode)
-{
- ProcArrayStruct *arrayP = procArray;
- int index;
-
- /* tell all backends to die */
- LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-
- for (index = 0; index < arrayP->numProcs; index++)
- {
- int pgprocno = arrayP->pgprocnos[index];
- PGPROC *proc = &allProcs[pgprocno];
-
- if (databaseid == InvalidOid || proc->databaseId == databaseid)
- {
- VirtualTransactionId procvxid;
- pid_t pid;
-
- GET_VXID_FROM_PGPROC(procvxid, *proc);
-
- pid = proc->pid;
- if (pid != 0)
- {
- /*
- * Kill the pid if it's still here. If not, that's what we
- * wanted so ignore any errors.
- */
- (void) SendProcSignal(pid, sigmode, procvxid.procNumber);
- }
- }
- }
-
- LWLockRelease(ProcArrayLock);
-}
-
/*
* CountUserBackends --- count backends that are used by specified user
* (only regular backends, not any type of background worker)
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 8e56922dcea..5d33559926a 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -697,26 +697,8 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
if (CheckProcSignal(PROCSIG_PARALLEL_APPLY_MESSAGE))
HandleParallelApplyMessageInterrupt();
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_DATABASE))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_DATABASE);
-
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_TABLESPACE))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_TABLESPACE);
-
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_LOCK))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_LOCK);
-
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT);
-
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT);
-
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
-
- if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN))
- HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
+ if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT))
+ HandleRecoveryConflictInterrupt();
SetLatch(MyLatch);
}
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 6db803476c4..0851789e8b6 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -71,13 +71,13 @@ static volatile sig_atomic_t got_standby_delay_timeout = false;
static volatile sig_atomic_t got_standby_lock_timeout = false;
static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
- ProcSignalReason reason,
+ RecoveryConflictReason reason,
uint32 wait_event_info,
bool report_waiting);
-static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
+static void SendRecoveryConflictWithBufferPin(RecoveryConflictReason reason);
static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
-static const char *get_recovery_conflict_desc(ProcSignalReason reason);
+static const char *get_recovery_conflict_desc(RecoveryConflictReason reason);
/*
* InitRecoveryTransactionEnvironment
@@ -271,7 +271,7 @@ WaitExceedsMaxStandbyDelay(uint32 wait_event_info)
* to be resolved or not.
*/
void
-LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start,
+LogRecoveryConflict(RecoveryConflictReason reason, TimestampTz wait_start,
TimestampTz now, VirtualTransactionId *wait_list,
bool still_waiting)
{
@@ -358,7 +358,8 @@ LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start,
*/
static void
ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
- ProcSignalReason reason, uint32 wait_event_info,
+ RecoveryConflictReason reason,
+ uint32 wait_event_info,
bool report_waiting)
{
TimestampTz waitStart = 0;
@@ -384,19 +385,19 @@ ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
/* Is it time to kill it? */
if (WaitExceedsMaxStandbyDelay(wait_event_info))
{
- pid_t pid;
+ bool signaled;
/*
* Now find out who to throw out of the balloon.
*/
Assert(VirtualTransactionIdIsValid(*waitlist));
- pid = SignalVirtualTransaction(*waitlist, reason);
+ signaled = SignalRecoveryConflictWithVirtualXID(*waitlist, reason);
/*
* Wait a little bit for it to die so that we avoid flooding
* an unresponsive backend when system is heavily loaded.
*/
- if (pid != 0)
+ if (signaled)
pg_usleep(5000L);
}
@@ -489,7 +490,7 @@ ResolveRecoveryConflictWithSnapshot(TransactionId snapshotConflictHorizon,
backends = GetConflictingVirtualXIDs(snapshotConflictHorizon,
locator.dbOid);
ResolveRecoveryConflictWithVirtualXIDs(backends,
- PROCSIG_RECOVERY_CONFLICT_SNAPSHOT,
+ RECOVERY_CONFLICT_SNAPSHOT,
WAIT_EVENT_RECOVERY_CONFLICT_SNAPSHOT,
true);
@@ -560,7 +561,7 @@ ResolveRecoveryConflictWithTablespace(Oid tsid)
temp_file_users = GetConflictingVirtualXIDs(InvalidTransactionId,
InvalidOid);
ResolveRecoveryConflictWithVirtualXIDs(temp_file_users,
- PROCSIG_RECOVERY_CONFLICT_TABLESPACE,
+ RECOVERY_CONFLICT_TABLESPACE,
WAIT_EVENT_RECOVERY_CONFLICT_TABLESPACE,
true);
}
@@ -581,7 +582,7 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
*/
while (CountDBBackends(dbid) > 0)
{
- CancelDBBackends(dbid, PROCSIG_RECOVERY_CONFLICT_DATABASE);
+ SignalRecoveryConflictWithDatabase(dbid, RECOVERY_CONFLICT_DATABASE);
/*
* Wait awhile for them to die so that we avoid flooding an
@@ -665,7 +666,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag, bool logging_conflict)
* because the caller, WaitOnLock(), has already reported that.
*/
ResolveRecoveryConflictWithVirtualXIDs(backends,
- PROCSIG_RECOVERY_CONFLICT_LOCK,
+ RECOVERY_CONFLICT_LOCK,
PG_WAIT_LOCK | locktag.locktag_type,
false);
}
@@ -723,8 +724,8 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag, bool logging_conflict)
*/
while (VirtualTransactionIdIsValid(*backends))
{
- SignalVirtualTransaction(*backends,
- PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+ (void) SignalRecoveryConflictWithVirtualXID(*backends,
+ RECOVERY_CONFLICT_STARTUP_DEADLOCK);
backends++;
}
@@ -802,7 +803,7 @@ ResolveRecoveryConflictWithBufferPin(void)
/*
* We're already behind, so clear a path as quickly as possible.
*/
- SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
+ SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_BUFFERPIN);
}
else
{
@@ -842,7 +843,7 @@ ResolveRecoveryConflictWithBufferPin(void)
ProcWaitForSignal(WAIT_EVENT_BUFFER_CLEANUP);
if (got_standby_delay_timeout)
- SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
+ SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_BUFFERPIN);
else if (got_standby_deadlock_timeout)
{
/*
@@ -858,7 +859,7 @@ ResolveRecoveryConflictWithBufferPin(void)
* not be so harmful because the period that the buffer is kept pinned
* is basically no so long. But we should fix this?
*/
- SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+ SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_STARTUP_DEADLOCK);
}
/*
@@ -873,10 +874,10 @@ ResolveRecoveryConflictWithBufferPin(void)
}
static void
-SendRecoveryConflictWithBufferPin(ProcSignalReason reason)
+SendRecoveryConflictWithBufferPin(RecoveryConflictReason reason)
{
- Assert(reason == PROCSIG_RECOVERY_CONFLICT_BUFFERPIN ||
- reason == PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+ Assert(reason == RECOVERY_CONFLICT_BUFFERPIN ||
+ reason == RECOVERY_CONFLICT_STARTUP_DEADLOCK);
/*
* We send signal to all backends to ask them if they are holding the
@@ -884,7 +885,7 @@ SendRecoveryConflictWithBufferPin(ProcSignalReason reason)
* innocent, but we let the SIGUSR1 handling in each backend decide their
* own fate.
*/
- CancelDBBackends(InvalidOid, reason);
+ SignalRecoveryConflictWithDatabase(InvalidOid, reason);
}
/*
@@ -1489,35 +1490,33 @@ LogStandbyInvalidations(int nmsgs, SharedInvalidationMessage *msgs,
/* Return the description of recovery conflict */
static const char *
-get_recovery_conflict_desc(ProcSignalReason reason)
+get_recovery_conflict_desc(RecoveryConflictReason reason)
{
const char *reasonDesc = _("unknown reason");
switch (reason)
{
- case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
+ case RECOVERY_CONFLICT_BUFFERPIN:
reasonDesc = _("recovery conflict on buffer pin");
break;
- case PROCSIG_RECOVERY_CONFLICT_LOCK:
+ case RECOVERY_CONFLICT_LOCK:
reasonDesc = _("recovery conflict on lock");
break;
- case PROCSIG_RECOVERY_CONFLICT_TABLESPACE:
+ case RECOVERY_CONFLICT_TABLESPACE:
reasonDesc = _("recovery conflict on tablespace");
break;
- case PROCSIG_RECOVERY_CONFLICT_SNAPSHOT:
+ case RECOVERY_CONFLICT_SNAPSHOT:
reasonDesc = _("recovery conflict on snapshot");
break;
- case PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT:
+ case RECOVERY_CONFLICT_LOGICALSLOT:
reasonDesc = _("recovery conflict on replication slot");
break;
- case PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK:
+ case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
reasonDesc = _("recovery conflict on buffer deadlock");
break;
- case PROCSIG_RECOVERY_CONFLICT_DATABASE:
+ case RECOVERY_CONFLICT_DATABASE:
reasonDesc = _("recovery conflict on database");
break;
- default:
- break;
}
return reasonDesc;
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index fdeed0f3953..99cf1783dca 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -506,6 +506,7 @@ InitProcess(void)
Assert(dlist_is_empty(&(MyProc->myProcLocks[i])));
}
#endif
+ pg_atomic_write_u32(&MyProc->pendingRecoveryConflicts, 0);
/* Initialize fields for sync rep */
MyProc->waitLSN = InvalidXLogRecPtr;
@@ -1446,7 +1447,7 @@ ProcSleep(LOCALLOCK *locallock)
* because the startup process here has already waited
* longer than deadlock_timeout.
*/
- LogRecoveryConflict(PROCSIG_RECOVERY_CONFLICT_LOCK,
+ LogRecoveryConflict(RECOVERY_CONFLICT_LOCK,
standbyWaitStart, now,
cnt > 0 ? vxids : NULL, true);
logged_recovery_conflict = true;
@@ -1687,7 +1688,7 @@ ProcSleep(LOCALLOCK *locallock)
* startup process waited longer than deadlock_timeout for it.
*/
if (InHotStandby && logged_recovery_conflict)
- LogRecoveryConflict(PROCSIG_RECOVERY_CONFLICT_LOCK,
+ LogRecoveryConflict(RECOVERY_CONFLICT_LOCK,
standbyWaitStart, GetCurrentTimestamp(),
NULL, false);
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 02e9aaa6bca..a6ab9f9860c 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -67,6 +67,7 @@
#include "storage/proc.h"
#include "storage/procsignal.h"
#include "storage/sinval.h"
+#include "storage/standby.h"
#include "tcop/backend_startup.h"
#include "tcop/fastpath.h"
#include "tcop/pquery.h"
@@ -155,10 +156,6 @@ static const char *userDoption = NULL; /* -D switch */
static bool EchoQuery = false; /* -E switch */
static bool UseSemiNewlineNewline = false; /* -j switch */
-/* whether or not, and why, we were canceled by conflict with recovery */
-static volatile sig_atomic_t RecoveryConflictPending = false;
-static volatile sig_atomic_t RecoveryConflictPendingReasons[NUM_PROCSIGNALS];
-
/* reused buffer to pass to SendRowDescriptionMessage() */
static MemoryContext row_description_context = NULL;
static StringInfoData row_description_buf;
@@ -2537,34 +2534,31 @@ errdetail_params(ParamListInfo params)
* Add an errdetail() line showing conflict source.
*/
static int
-errdetail_recovery_conflict(ProcSignalReason reason)
+errdetail_recovery_conflict(RecoveryConflictReason reason)
{
switch (reason)
{
- case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
+ case RECOVERY_CONFLICT_BUFFERPIN:
errdetail("User was holding shared buffer pin for too long.");
break;
- case PROCSIG_RECOVERY_CONFLICT_LOCK:
+ case RECOVERY_CONFLICT_LOCK:
errdetail("User was holding a relation lock for too long.");
break;
- case PROCSIG_RECOVERY_CONFLICT_TABLESPACE:
+ case RECOVERY_CONFLICT_TABLESPACE:
errdetail("User was or might have been using tablespace that must be dropped.");
break;
- case PROCSIG_RECOVERY_CONFLICT_SNAPSHOT:
+ case RECOVERY_CONFLICT_SNAPSHOT:
errdetail("User query might have needed to see row versions that must be removed.");
break;
- case PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT:
+ case RECOVERY_CONFLICT_LOGICALSLOT:
errdetail("User was using a logical replication slot that must be invalidated.");
break;
- case PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK:
+ case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
errdetail("User transaction caused buffer deadlock with recovery.");
break;
- case PROCSIG_RECOVERY_CONFLICT_DATABASE:
+ case RECOVERY_CONFLICT_DATABASE:
errdetail("User was connected to a database that must be dropped.");
break;
- default:
- break;
- /* no errdetail */
}
return 0;
@@ -3067,15 +3061,14 @@ FloatExceptionHandler(SIGNAL_ARGS)
}
/*
- * Tell the next CHECK_FOR_INTERRUPTS() to check for a particular type of
+ * Tell the next CHECK_FOR_INTERRUPTS() to check for a particular type of XXX
* recovery conflict. Runs in a SIGUSR1 handler.
*/
void
-HandleRecoveryConflictInterrupt(ProcSignalReason reason)
+HandleRecoveryConflictInterrupt(void)
{
- RecoveryConflictPendingReasons[reason] = true;
- RecoveryConflictPending = true;
- InterruptPending = true;
+ if (pg_atomic_read_u32(&MyProc->pendingRecoveryConflicts) != 0)
+ InterruptPending = true;
/* latch will be set by procsignal_sigusr1_handler */
}
@@ -3083,11 +3076,11 @@ HandleRecoveryConflictInterrupt(ProcSignalReason reason)
* Check one individual conflict reason.
*/
static void
-ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
+ProcessRecoveryConflictInterrupt(RecoveryConflictReason reason)
{
switch (reason)
{
- case PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK:
+ case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
/*
* If we aren't waiting for a lock we can never deadlock.
@@ -3098,21 +3091,20 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
/* Intentional fall through to check wait for pin */
/* FALLTHROUGH */
- case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
+ case RECOVERY_CONFLICT_BUFFERPIN:
/*
- * If PROCSIG_RECOVERY_CONFLICT_BUFFERPIN is requested but we
- * aren't blocking the Startup process there is nothing more to
- * do.
+ * If RECOVERY_CONFLICT_BUFFERPIN is requested but we aren't
+ * blocking the Startup process there is nothing more to do.
*
- * When PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK is requested,
- * if we're waiting for locks and the startup process is not
- * waiting for buffer pin (i.e., also waiting for locks), we set
- * the flag so that ProcSleep() will check for deadlocks.
+ * When RECOVERY_CONFLICT_STARTUP_DEADLOCK is requested, if we're
+ * waiting for locks and the startup process is not waiting for
+ * buffer pin (i.e., also waiting for locks), we set the flag so
+ * that ProcSleep() will check for deadlocks.
*/
if (!HoldingBufferPinThatDelaysRecovery())
{
- if (reason == PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK &&
+ if (reason == RECOVERY_CONFLICT_STARTUP_DEADLOCK &&
GetStartupBufferPinWaitBufId() < 0)
CheckDeadLockAlert();
return;
@@ -3121,9 +3113,9 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
/* Intentional fall through to error handling */
/* FALLTHROUGH */
- case PROCSIG_RECOVERY_CONFLICT_LOCK:
- case PROCSIG_RECOVERY_CONFLICT_TABLESPACE:
- case PROCSIG_RECOVERY_CONFLICT_SNAPSHOT:
+ case RECOVERY_CONFLICT_LOCK:
+ case RECOVERY_CONFLICT_TABLESPACE:
+ case RECOVERY_CONFLICT_SNAPSHOT:
/*
* If we aren't in a transaction any longer then ignore.
@@ -3133,34 +3125,34 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
/* FALLTHROUGH */
- case PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT:
+ case RECOVERY_CONFLICT_LOGICALSLOT:
/*
* If we're not in a subtransaction then we are OK to throw an
* ERROR to resolve the conflict. Otherwise drop through to the
* FATAL case.
*
- * PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT is a special case that
- * always throws an ERROR (ie never promotes to FATAL), though it
- * still has to respect QueryCancelHoldoffCount, so it shares this
- * code path. Logical decoding slots are only acquired while
+ * RECOVERY_CONFLICT_LOGICALSLOT is a special case that always
+ * throws an ERROR (ie never promotes to FATAL), though it still
+ * has to respect QueryCancelHoldoffCount, so it shares this code
+ * path. Logical decoding slots are only acquired while
* performing logical decoding. During logical decoding no user
* controlled code is run. During [sub]transaction abort, the
* slot is released. Therefore user controlled code cannot
* intercept an error before the replication slot is released.
*
* XXX other times that we can throw just an ERROR *may* be
- * PROCSIG_RECOVERY_CONFLICT_LOCK if no locks are held in parent
+ * RECOVERY_CONFLICT_LOCK if no locks are held in parent
* transactions
*
- * PROCSIG_RECOVERY_CONFLICT_SNAPSHOT if no snapshots are held by
- * parent transactions and the transaction is not
- * transaction-snapshot mode
+ * RECOVERY_CONFLICT_SNAPSHOT if no snapshots are held by parent
+ * transactions and the transaction is not transaction-snapshot
+ * mode
*
- * PROCSIG_RECOVERY_CONFLICT_TABLESPACE if no temp files or
- * cursors open in parent transactions
+ * RECOVERY_CONFLICT_TABLESPACE if no temp files or cursors open
+ * in parent transactions
*/
- if (reason == PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT ||
+ if (reason == RECOVERY_CONFLICT_LOGICALSLOT ||
!IsSubTransaction())
{
/*
@@ -3187,8 +3179,7 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
* Re-arm and defer this interrupt until later. See
* similar code in ProcessInterrupts().
*/
- RecoveryConflictPendingReasons[reason] = true;
- RecoveryConflictPending = true;
+ (void) pg_atomic_fetch_or_u32(&MyProc->pendingRecoveryConflicts, (1 << reason));
InterruptPending = true;
return;
}
@@ -3222,7 +3213,7 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
" database and repeat your command.")));
break;
- case PROCSIG_RECOVERY_CONFLICT_DATABASE:
+ case RECOVERY_CONFLICT_DATABASE:
/* The database is being dropped; terminate the session */
pgstat_report_recovery_conflict(reason);
@@ -3243,6 +3234,8 @@ ProcessRecoveryConflictInterrupt(ProcSignalReason reason)
static void
ProcessRecoveryConflictInterrupts(void)
{
+ uint32 pending;
+
/*
* We don't need to worry about joggling the elbow of proc_exit, because
* proc_exit_prepare() holds interrupts, so ProcessInterrupts() won't call
@@ -3250,17 +3243,21 @@ ProcessRecoveryConflictInterrupts(void)
*/
Assert(!proc_exit_inprogress);
Assert(InterruptHoldoffCount == 0);
- Assert(RecoveryConflictPending);
- RecoveryConflictPending = false;
+ /* Are any recovery conflict pending? */
+ pending = pg_atomic_read_membarrier_u32(&MyProc->pendingRecoveryConflicts);
+ if (pending == 0)
+ return;
- for (ProcSignalReason reason = PROCSIG_RECOVERY_CONFLICT_FIRST;
- reason <= PROCSIG_RECOVERY_CONFLICT_LAST;
+ for (RecoveryConflictReason reason = 0;
+ reason < NUM_RECOVERY_CONFLICT_REASONS;
reason++)
{
- if (RecoveryConflictPendingReasons[reason])
+ if ((pending & (1 << reason)) != 0)
{
- RecoveryConflictPendingReasons[reason] = false;
+ /* clear the flag */
+ (void) pg_atomic_fetch_and_u32(&MyProc->pendingRecoveryConflicts, ~(1 << reason));
+
ProcessRecoveryConflictInterrupt(reason);
}
}
@@ -3451,7 +3448,7 @@ ProcessInterrupts(void)
}
}
- if (RecoveryConflictPending)
+ if (pg_atomic_read_u32(&MyProc->pendingRecoveryConflicts) != 0)
ProcessRecoveryConflictInterrupts();
if (IdleInTransactionSessionTimeoutPending)
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index d7f6d4c5ee6..e6759ccaa3d 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -17,7 +17,7 @@
#include "postgres.h"
-#include "storage/procsignal.h"
+#include "storage/standby.h"
#include "utils/pgstat_internal.h"
#include "utils/timestamp.h"
@@ -88,31 +88,31 @@ pgstat_report_recovery_conflict(int reason)
dbentry = pgstat_prep_database_pending(MyDatabaseId);
- switch (reason)
+ switch ((RecoveryConflictReason) reason)
{
- case PROCSIG_RECOVERY_CONFLICT_DATABASE:
+ case RECOVERY_CONFLICT_DATABASE:
/*
* Since we drop the information about the database as soon as it
* replicates, there is no point in counting these conflicts.
*/
break;
- case PROCSIG_RECOVERY_CONFLICT_TABLESPACE:
+ case RECOVERY_CONFLICT_TABLESPACE:
dbentry->conflict_tablespace++;
break;
- case PROCSIG_RECOVERY_CONFLICT_LOCK:
+ case RECOVERY_CONFLICT_LOCK:
dbentry->conflict_lock++;
break;
- case PROCSIG_RECOVERY_CONFLICT_SNAPSHOT:
+ case RECOVERY_CONFLICT_SNAPSHOT:
dbentry->conflict_snapshot++;
break;
- case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
+ case RECOVERY_CONFLICT_BUFFERPIN:
dbentry->conflict_bufferpin++;
break;
- case PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT:
+ case RECOVERY_CONFLICT_LOGICALSLOT:
dbentry->conflict_logicalslot++;
break;
- case PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK:
+ case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
dbentry->conflict_startup_deadlock++;
break;
}
diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c
index 12b8d4cefaf..c7f7b8bc2dd 100644
--- a/src/backend/utils/adt/mcxtfuncs.c
+++ b/src/backend/utils/adt/mcxtfuncs.c
@@ -19,6 +19,7 @@
#include "mb/pg_wchar.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "storage/procsignal.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/hsearch.h"
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 81f1960a635..dd4a4033c3c 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -235,6 +235,16 @@ struct PGPROC
bool isRegularBackend; /* true if it's a regular backend. */
+ /*
+ * While in hot standby mode, shows that a conflict signal has been sent
+ * for the current transaction. Set/cleared while holding ProcArrayLock,
+ * though not required. Accessed without lock, if needed.
+ *
+ * This is a bitmask; each bit corresponds to a RecoveryConflictReason
+ * enum value.
+ */
+ pg_atomic_uint32 pendingRecoveryConflicts;
+
/*
* Info about LWLock the process is currently waiting for, if any.
*
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 3a8593f87ba..c5ab1574fe3 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -77,12 +77,15 @@ extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin,
bool excludeXmin0, bool allDbs, int excludeVacuum,
int *nvxids);
extern VirtualTransactionId *GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid);
-extern pid_t SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode);
+
+extern bool SignalRecoveryConflict(PGPROC *proc, pid_t pid, RecoveryConflictReason reason);
+extern bool SignalRecoveryConflictWithVirtualXID(VirtualTransactionId vxid, RecoveryConflictReason reason);
+extern void SignalRecoveryConflictWithDatabase(Oid databaseid, RecoveryConflictReason reason);
+
extern bool MinimumActiveBackends(int min);
extern int CountDBBackends(Oid databaseid);
extern int CountDBConnections(Oid databaseid);
-extern void CancelDBBackends(Oid databaseid, ProcSignalReason sigmode);
extern int CountUserBackends(Oid roleid);
extern bool CountOtherDBBackends(Oid databaseId,
int *nbackends, int *nprepared);
diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h
index e52b8eb7697..348fba53a93 100644
--- a/src/include/storage/procsignal.h
+++ b/src/include/storage/procsignal.h
@@ -36,20 +36,12 @@ typedef enum
PROCSIG_BARRIER, /* global barrier interrupt */
PROCSIG_LOG_MEMORY_CONTEXT, /* ask backend to log the memory contexts */
PROCSIG_PARALLEL_APPLY_MESSAGE, /* Message from parallel apply workers */
-
- /* Recovery conflict reasons */
- PROCSIG_RECOVERY_CONFLICT_FIRST,
- PROCSIG_RECOVERY_CONFLICT_DATABASE = PROCSIG_RECOVERY_CONFLICT_FIRST,
- PROCSIG_RECOVERY_CONFLICT_TABLESPACE,
- PROCSIG_RECOVERY_CONFLICT_LOCK,
- PROCSIG_RECOVERY_CONFLICT_SNAPSHOT,
- PROCSIG_RECOVERY_CONFLICT_LOGICALSLOT,
- PROCSIG_RECOVERY_CONFLICT_BUFFERPIN,
- PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK,
- PROCSIG_RECOVERY_CONFLICT_LAST = PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK,
+ PROCSIG_RECOVERY_CONFLICT, /* backend is blocking recovery, check
+ * PGPROC->pendingRecoveryConflicts for the
+ * reason */
} ProcSignalReason;
-#define NUM_PROCSIGNALS (PROCSIG_RECOVERY_CONFLICT_LAST + 1)
+#define NUM_PROCSIGNALS (PROCSIG_RECOVERY_CONFLICT + 1)
typedef enum
{
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index 7b10932635a..65a8176785e 100644
--- a/src/include/storage/standby.h
+++ b/src/include/storage/standby.h
@@ -16,7 +16,6 @@
#include "datatype/timestamp.h"
#include "storage/lock.h"
-#include "storage/procsignal.h"
#include "storage/relfilelocator.h"
#include "storage/standbydefs.h"
@@ -25,6 +24,37 @@ extern PGDLLIMPORT int max_standby_archive_delay;
extern PGDLLIMPORT int max_standby_streaming_delay;
extern PGDLLIMPORT bool log_recovery_conflict_waits;
+/* Recovery conflict reasons */
+typedef enum
+{
+ /* Backend is connected to a database that is being dropped */
+ RECOVERY_CONFLICT_DATABASE,
+
+ /* Backend is using a tablespace that is being dropped */
+ RECOVERY_CONFLICT_TABLESPACE,
+
+ /* Backend is holding a lock that is blocking recovery */
+ RECOVERY_CONFLICT_LOCK,
+
+ /* Backend is holding a snapshot that is blocking recovery */
+ RECOVERY_CONFLICT_SNAPSHOT,
+
+ /* Backend is using a logical replication slot that must be invalidated */
+ RECOVERY_CONFLICT_LOGICALSLOT,
+
+ /* Backend is holding a pin on a buffer that is blocking recovery */
+ RECOVERY_CONFLICT_BUFFERPIN,
+
+ /*
+ * The backend is requested to check for deadlocks. The startup process
+ * doesn't check for deadlock directly, because we want to kill one of the
+ * other backends instead of the startup process.
+ */
+ RECOVERY_CONFLICT_STARTUP_DEADLOCK,
+} RecoveryConflictReason;
+
+#define NUM_RECOVERY_CONFLICT_REASONS (RECOVERY_CONFLICT_STARTUP_DEADLOCK + 1)
+
extern void InitRecoveryTransactionEnvironment(void);
extern void ShutdownRecoveryTransactionEnvironment(void);
@@ -43,7 +73,7 @@ extern void CheckRecoveryConflictDeadlock(void);
extern void StandbyDeadLockHandler(void);
extern void StandbyTimeoutHandler(void);
extern void StandbyLockTimeoutHandler(void);
-extern void LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start,
+extern void LogRecoveryConflict(RecoveryConflictReason reason, TimestampTz wait_start,
TimestampTz now, VirtualTransactionId *wait_list,
bool still_waiting);
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 54ddee875ed..5bc5bcfb20d 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -74,7 +74,7 @@ extern void die(SIGNAL_ARGS);
pg_noreturn extern void quickdie(SIGNAL_ARGS);
extern void StatementCancelHandler(SIGNAL_ARGS);
pg_noreturn extern void FloatExceptionHandler(SIGNAL_ARGS);
-extern void HandleRecoveryConflictInterrupt(ProcSignalReason reason);
+extern void HandleRecoveryConflictInterrupt(void);
extern void ProcessClientReadInterrupt(bool blocked);
extern void ProcessClientWriteInterrupt(bool blocked);
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 9f5ee8fd482..32c3b8be9da 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2488,6 +2488,7 @@ RecordCacheArrayEntry
RecordCacheEntry
RecordCompareData
RecordIOData
+RecoveryConflictReason
RecoveryLockEntry
RecoveryLockXidEntry
RecoveryPauseState
--
2.47.3
From e0363fd95e3d5ccac3b3965fb6f46512c37fa77c Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 3 Feb 2026 15:27:31 +0200
Subject: [PATCH v2 3/3] Refactor ProcessRecoveryConflictInterrupt for
readability
Two changes here:
1. Introduce a separate RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK flag to
indicate a suspected deadlock that involves a buffer pin. Previously
the startup process used the same flag for a deadlock involving just
regular locks, and to check for deadlocks involving the buffer
pin. The cases are handled separately in the startup process, but the
receiving backend had to deduce which one it was based on
HoldingBufferPinThatDelaysRecovery(). With a separate flag, the
receiver doesn't need to guess.
2. Rewrite the ProcessRecoveryConflictInterrupt() function to not rely
on fallthrough through the switch-statement. That was difficult to
read.
Reviewed-by: Chao Li <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
---
src/backend/storage/ipc/standby.c | 7 +-
src/backend/tcop/postgres.c | 262 +++++++++++--------
src/backend/utils/activity/pgstat_database.c | 10 +
src/include/storage/standby.h | 10 +-
4 files changed, 181 insertions(+), 108 deletions(-)
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 0851789e8b6..d83afbfb9d6 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -859,7 +859,7 @@ ResolveRecoveryConflictWithBufferPin(void)
* not be so harmful because the period that the buffer is kept pinned
* is basically no so long. But we should fix this?
*/
- SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+ SendRecoveryConflictWithBufferPin(RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK);
}
/*
@@ -877,7 +877,7 @@ static void
SendRecoveryConflictWithBufferPin(RecoveryConflictReason reason)
{
Assert(reason == RECOVERY_CONFLICT_BUFFERPIN ||
- reason == RECOVERY_CONFLICT_STARTUP_DEADLOCK);
+ reason == RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK);
/*
* We send signal to all backends to ask them if they are holding the
@@ -1512,6 +1512,9 @@ get_recovery_conflict_desc(RecoveryConflictReason reason)
reasonDesc = _("recovery conflict on replication slot");
break;
case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
+ reasonDesc = _("recovery conflict on deadlock");
+ break;
+ case RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK:
reasonDesc = _("recovery conflict on buffer deadlock");
break;
case RECOVERY_CONFLICT_DATABASE:
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index a6ab9f9860c..8ac2ee7dd23 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -179,6 +179,9 @@ static bool IsTransactionExitStmt(Node *parsetree);
static bool IsTransactionExitStmtList(List *pstmts);
static bool IsTransactionStmtList(List *pstmts);
static void drop_unnamed_stmt(void);
+static void ProcessRecoveryConflictInterrupts(void);
+static void ProcessRecoveryConflictInterrupt(RecoveryConflictReason reason);
+static void report_recovery_conflict(RecoveryConflictReason reason);
static void log_disconnections(int code, Datum arg);
static void enable_statement_timeout(void);
static void disable_statement_timeout(void);
@@ -2554,6 +2557,9 @@ errdetail_recovery_conflict(RecoveryConflictReason reason)
errdetail("User was using a logical replication slot that must be invalidated.");
break;
case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
+ errdetail("User transaction caused deadlock with recovery.");
+ break;
+ case RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK:
errdetail("User transaction caused buffer deadlock with recovery.");
break;
case RECOVERY_CONFLICT_DATABASE:
@@ -3083,35 +3089,62 @@ ProcessRecoveryConflictInterrupt(RecoveryConflictReason reason)
case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
/*
+ * The startup process is waiting on a lock held by us, and has
+ * requested us to check if it is a deadlock (i.e. the deadlock
+ * timeout expired).
+ *
* If we aren't waiting for a lock we can never deadlock.
*/
if (GetAwaitedLock() == NULL)
return;
- /* Intentional fall through to check wait for pin */
- /* FALLTHROUGH */
+ /* Set the flag so that ProcSleep() will check for deadlocks. */
+ CheckDeadLockAlert();
+ return;
- case RECOVERY_CONFLICT_BUFFERPIN:
+ case RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK:
/*
- * If RECOVERY_CONFLICT_BUFFERPIN is requested but we aren't
- * blocking the Startup process there is nothing more to do.
+ * The startup process is waiting on a buffer pin, and has
+ * requested us to check if there is a deadlock involving the pin.
*
- * When RECOVERY_CONFLICT_STARTUP_DEADLOCK is requested, if we're
- * waiting for locks and the startup process is not waiting for
- * buffer pin (i.e., also waiting for locks), we set the flag so
- * that ProcSleep() will check for deadlocks.
+ * If we're not waiting on a lock, there can be no deadlock.
+ */
+ if (GetAwaitedLock() == NULL)
+ return;
+
+ /*
+ * If we're not holding the buffer pin, also no deadlock. (The
+ * startup process doesn't know who's holding the pin, and sends
+ * this signal to *all* backends, so this is the common case.)
*/
if (!HoldingBufferPinThatDelaysRecovery())
- {
- if (reason == RECOVERY_CONFLICT_STARTUP_DEADLOCK &&
- GetStartupBufferPinWaitBufId() < 0)
- CheckDeadLockAlert();
return;
- }
- /* Intentional fall through to error handling */
- /* FALLTHROUGH */
+ /*
+ * Otherwise, we probably have a deadlock. Unfortunately the
+ * normal deadlock detector doesn't know about buffer pins, so we
+ * cannot perform comprehensively deadlock check. Instead, we
+ * just assume that it is a deadlock if the above two conditions
+ * are met. In principle this can lead to false positives, but
+ * it's rare in practice because sessions in a hot standby server
+ * rarely hold locks that can block other backends.
+ */
+ report_recovery_conflict(reason);
+ return;
+
+ case RECOVERY_CONFLICT_BUFFERPIN:
+
+ /*
+ * Someone is holding a buffer pin that the startup process is
+ * waiting for, and it got tired of waiting. If that's us, error
+ * out to release the pin.
+ */
+ if (!HoldingBufferPinThatDelaysRecovery())
+ return;
+
+ report_recovery_conflict(reason);
+ return;
case RECOVERY_CONFLICT_LOCK:
case RECOVERY_CONFLICT_TABLESPACE:
@@ -3123,109 +3156,128 @@ ProcessRecoveryConflictInterrupt(RecoveryConflictReason reason)
if (!IsTransactionOrTransactionBlock())
return;
- /* FALLTHROUGH */
+ report_recovery_conflict(reason);
+ return;
case RECOVERY_CONFLICT_LOGICALSLOT:
+ report_recovery_conflict(reason);
+ return;
- /*
- * If we're not in a subtransaction then we are OK to throw an
- * ERROR to resolve the conflict. Otherwise drop through to the
- * FATAL case.
- *
- * RECOVERY_CONFLICT_LOGICALSLOT is a special case that always
- * throws an ERROR (ie never promotes to FATAL), though it still
- * has to respect QueryCancelHoldoffCount, so it shares this code
- * path. Logical decoding slots are only acquired while
- * performing logical decoding. During logical decoding no user
- * controlled code is run. During [sub]transaction abort, the
- * slot is released. Therefore user controlled code cannot
- * intercept an error before the replication slot is released.
- *
- * XXX other times that we can throw just an ERROR *may* be
- * RECOVERY_CONFLICT_LOCK if no locks are held in parent
- * transactions
- *
- * RECOVERY_CONFLICT_SNAPSHOT if no snapshots are held by parent
- * transactions and the transaction is not transaction-snapshot
- * mode
- *
- * RECOVERY_CONFLICT_TABLESPACE if no temp files or cursors open
- * in parent transactions
- */
- if (reason == RECOVERY_CONFLICT_LOGICALSLOT ||
- !IsSubTransaction())
- {
- /*
- * If we already aborted then we no longer need to cancel. We
- * do this here since we do not wish to ignore aborted
- * subtransactions, which must cause FATAL, currently.
- */
- if (IsAbortedTransactionBlockState())
- return;
+ case RECOVERY_CONFLICT_DATABASE:
+
+ /* The database is being dropped; terminate the session */
+ report_recovery_conflict(reason);
+ return;
+ }
+ elog(FATAL, "unrecognized conflict mode: %d", (int) reason);
+}
+/*
+ * This transaction or session is conflicting with recovery and needs to be
+ * killed. Roll back the transaction, if that's sufficient, or terminate the
+ * connection, or do nothing if we're already in aborted state.
+ */
+static void
+report_recovery_conflict(RecoveryConflictReason reason)
+{
+ bool fatal;
+
+ if (reason == RECOVERY_CONFLICT_DATABASE)
+ {
+ /* note: no hint about reconnecting, and different errcode */
+ pgstat_report_recovery_conflict(reason);
+ ereport(FATAL,
+ (errcode(ERRCODE_DATABASE_DROPPED),
+ errmsg("terminating connection due to conflict with recovery"),
+ errdetail_recovery_conflict(reason)));
+ }
+ if (reason == RECOVERY_CONFLICT_LOGICALSLOT)
+ {
+ /*
+ * RECOVERY_CONFLICT_LOGICALSLOT is a special case that always throws
+ * an ERROR (ie never promotes to FATAL), though it still has to
+ * respect QueryCancelHoldoffCount, so it shares this code path.
+ * Logical decoding slots are only acquired while performing logical
+ * decoding. During logical decoding no user controlled code is run.
+ * During [sub]transaction abort, the slot is released. Therefore
+ * user controlled code cannot intercept an error before the
+ * replication slot is released.
+ */
+ fatal = false;
+ }
+ else
+ {
+ fatal = IsSubTransaction();
+ }
+
+ /*
+ * If we're not in a subtransaction then we are OK to throw an ERROR to
+ * resolve the conflict.
+ *
+ * XXX other times that we can throw just an ERROR *may* be
+ * RECOVERY_CONFLICT_LOCK if no locks are held in parent transactions
+ *
+ * RECOVERY_CONFLICT_SNAPSHOT if no snapshots are held by parent
+ * transactions and the transaction is not transaction-snapshot mode
+ *
+ * RECOVERY_CONFLICT_TABLESPACE if no temp files or cursors open in parent
+ * transactions
+ */
+ if (!fatal)
+ {
+ /*
+ * If we already aborted then we no longer need to cancel. We do this
+ * here since we do not wish to ignore aborted subtransactions, which
+ * must cause FATAL, currently.
+ */
+ if (IsAbortedTransactionBlockState())
+ return;
+
+ /*
+ * If a recovery conflict happens while we are waiting for input from
+ * the client, the client is presumably just sitting idle in a
+ * transaction, preventing recovery from making progress. We'll drop
+ * through to the FATAL case below to dislodge it, in that case.
+ */
+ if (!DoingCommandRead)
+ {
+ /* Avoid losing sync in the FE/BE protocol. */
+ if (QueryCancelHoldoffCount != 0)
+ {
/*
- * If a recovery conflict happens while we are waiting for
- * input from the client, the client is presumably just
- * sitting idle in a transaction, preventing recovery from
- * making progress. We'll drop through to the FATAL case
- * below to dislodge it, in that case.
+ * Re-arm and defer this interrupt until later. See similar
+ * code in ProcessInterrupts().
*/
- if (!DoingCommandRead)
- {
- /* Avoid losing sync in the FE/BE protocol. */
- if (QueryCancelHoldoffCount != 0)
- {
- /*
- * Re-arm and defer this interrupt until later. See
- * similar code in ProcessInterrupts().
- */
- (void) pg_atomic_fetch_or_u32(&MyProc->pendingRecoveryConflicts, (1 << reason));
- InterruptPending = true;
- return;
- }
-
- /*
- * We are cleared to throw an ERROR. Either it's the
- * logical slot case, or we have a top-level transaction
- * that we can abort and a conflict that isn't inherently
- * non-retryable.
- */
- LockErrorCleanup();
- pgstat_report_recovery_conflict(reason);
- ereport(ERROR,
- (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
- errmsg("canceling statement due to conflict with recovery"),
- errdetail_recovery_conflict(reason)));
- break;
- }
+ (void) pg_atomic_fetch_or_u32(&MyProc->pendingRecoveryConflicts, (1 << reason));
+ InterruptPending = true;
+ return;
}
/*
- * We couldn't resolve the conflict with ERROR, so terminate the
- * whole session.
+ * We are cleared to throw an ERROR. Either it's the logical slot
+ * case, or we have a top-level transaction that we can abort and
+ * a conflict that isn't inherently non-retryable.
*/
+ LockErrorCleanup();
pgstat_report_recovery_conflict(reason);
- ereport(FATAL,
+ ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
- errmsg("terminating connection due to conflict with recovery"),
- errdetail_recovery_conflict(reason),
- errhint("In a moment you should be able to reconnect to the"
- " database and repeat your command.")));
- break;
-
- case RECOVERY_CONFLICT_DATABASE:
-
- /* The database is being dropped; terminate the session */
- pgstat_report_recovery_conflict(reason);
- ereport(FATAL,
- (errcode(ERRCODE_DATABASE_DROPPED),
- errmsg("terminating connection due to conflict with recovery"),
+ errmsg("canceling statement due to conflict with recovery"),
errdetail_recovery_conflict(reason)));
- break;
-
- default:
- elog(FATAL, "unrecognized conflict mode: %d", (int) reason);
+ }
}
+
+ /*
+ * We couldn't resolve the conflict with ERROR, so terminate the whole
+ * session.
+ */
+ pgstat_report_recovery_conflict(reason);
+ ereport(FATAL,
+ (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+ errmsg("terminating connection due to conflict with recovery"),
+ errdetail_recovery_conflict(reason),
+ errhint("In a moment you should be able to reconnect to the"
+ " database and repeat your command.")));
}
/*
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index e6759ccaa3d..6309909bcd0 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -115,6 +115,16 @@ pgstat_report_recovery_conflict(int reason)
case RECOVERY_CONFLICT_STARTUP_DEADLOCK:
dbentry->conflict_startup_deadlock++;
break;
+ case RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK:
+
+ /*
+ * The difference between RECOVERY_CONFLICT_STARTUP_DEADLOCK and
+ * RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK is merely whether a buffer
+ * pin was part of the deadlock. We use the same counter for both
+ * reasons.
+ */
+ dbentry->conflict_startup_deadlock++;
+ break;
}
}
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index 65a8176785e..c63a4f2cc6a 100644
--- a/src/include/storage/standby.h
+++ b/src/include/storage/standby.h
@@ -51,9 +51,17 @@ typedef enum
* other backends instead of the startup process.
*/
RECOVERY_CONFLICT_STARTUP_DEADLOCK,
+
+ /*
+ * Like RECOVERY_CONFLICT_STARTUP_DEADLOCK is, but the suspected deadlock
+ * involves a buffer pin that some other backend is holding. That needs
+ * special checking because the normal deadlock detector doesn't track the
+ * buffer pins.
+ */
+ RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK,
} RecoveryConflictReason;
-#define NUM_RECOVERY_CONFLICT_REASONS (RECOVERY_CONFLICT_STARTUP_DEADLOCK + 1)
+#define NUM_RECOVERY_CONFLICT_REASONS (RECOVERY_CONFLICT_BUFFERPIN_DEADLOCK + 1)
extern void InitRecoveryTransactionEnvironment(void);
extern void ShutdownRecoveryTransactionEnvironment(void);
--
2.47.3
