diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 3a89149016fe6..b63be24de4794 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2430,6 +2430,26 @@ CommitTransaction(void) */ ProcArrayEndTransaction(MyProc, latestXid); + /* + * PROTOTYPE ("Alt B"): insert this transaction's notifications into the + * queue now. This is deliberately *after* ProcArrayEndTransaction(): + * + * - The serializing lock is acquired after XLogFlush()/SyncRepWaitForLSN(), + * so notifying transactions no longer serialize across their own commit + * round trip and can group-commit with one another. + * + * - We have already left the ProcArray, so a backend queued on the insert + * lock no longer pins its xid/xmin, and so does not hold back the global + * horizon or delay the visibility of an already-durable commit. + * + * - Every entry is therefore inserted by a transaction that is already + * ProcArray-complete. A listener reads the queue head before taking its + * snapshot, so it can never observe an entry whose xid is still + * in-progress: insertion-order == visibility-order holds trivially + * rather than being derived from how long the lock is held. + */ + PostCommitInsert_Notify(); + /* * This is all post-commit cleanup. Note that if an error is raised here, * it's too late to abort the transaction. This should be just diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 2799f989b96dc..53340593330f7 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -1226,9 +1226,6 @@ PreCommit_Notify(void) /* Queue any pending notifies (must happen after the above) */ if (pendingNotifies) { - ListCell *nextNotify; - bool firstIteration = true; - /* * Build list of unique channel names being notified for use by * SignalBackends(). @@ -1291,75 +1288,93 @@ PreCommit_Notify(void) (void) GetCurrentTransactionId(); /* - * Serialize writers by acquiring a special lock that we hold till - * after commit. This ensures that queue entries appear in commit - * order, and in particular that there are never uncommitted queue - * entries ahead of committed ones, so an uncommitted transaction - * can't block delivery of deliverable notifications. - * - * We use a heavyweight lock so that it'll automatically be released - * after either commit or abort. This also allows deadlocks to be - * detected, though really a deadlock shouldn't be possible here. - * - * The lock is on "database 0", which is pretty ugly but it doesn't - * seem worth inventing a special locktag category just for this. - * (Historical note: before PG 9.0, a similar lock on "database 0" was - * used by the flatfiles mechanism.) + * PROTOTYPE ("Alt B"): the queue insertion, and the serializing lock + * that used to be taken here, have moved to PostCommitInsert_Notify(), + * which CommitTransaction() calls after RecordTransactionCommit(). + * Nothing further to do at pre-commit time. */ - LockSharedObject(DatabaseRelationId, InvalidOid, 0, - AccessExclusiveLock); + } +} - /* - * For the direct advancement optimization in SignalBackends(), we - * need to ensure that no other backend can insert queue entries - * between queueHeadBeforeWrite and queueHeadAfterWrite. The - * heavyweight lock above provides this guarantee, since it serializes - * all writers. - * - * Note: if the heavyweight lock were ever removed for scalability - * reasons, we could achieve the same guarantee by holding - * NotifyQueueLock in EXCLUSIVE mode across all our insertions, rather - * than releasing and reacquiring it for each page as we do below. - */ +/* + * PostCommitInsert_Notify + * + * PROTOTYPE -- NOT A PATCH PROPOSAL. See notify-bench/README.md. + * + * Insert this transaction's notifications into the queue, after its + * commit record is durable but before it leaves the ProcArray. + * + * The point of moving the insertion to here is that the serializing lock + * is then acquired *after* XLogFlush() and SyncRepWaitForLSN(), so + * notifying transactions no longer serialize across their own commit + * round trip and can group-commit with one another again. + * + * The lock is still held from here until end of transaction, so it still + * spans ProcArrayEndTransaction(). That is deliberate and load-bearing: + * the listener's gate is XidInMVCCSnapshot() (see + * asyncQueueProcessPageEntries), so the invariant that must be preserved + * is *queue insertion order == snapshot visibility order*, not commit-LSN + * order. Holding across the ProcArray removal preserves it exactly, and + * also preserves the precondition that SignalBackends()'s direct-advance + * optimization depends on. + * + * KNOWN DEFECT: asyncQueueIsFull() can still ereport(ERROR) here, and we + * are now past the commit point, so that error would be raised for an + * already-committed transaction. A real patch must reserve quota during + * pre-commit (where failing is safe) and make this path infallible. The + * benchmark never fills the queue, so this does not affect the numbers. + */ +void +PostCommitInsert_Notify(void) +{ + ListCell *nextNotify; + bool firstIteration = true; - /* Initialize values to a safe default in case list is empty */ - SET_QUEUE_POS(queueHeadBeforeWrite, 0, 0); - SET_QUEUE_POS(queueHeadAfterWrite, 0, 0); + if (!pendingNotifies) + return; - /* Now push the notifications into the queue */ - nextNotify = list_head(pendingNotifies->events); - while (nextNotify != NULL) + /* + * Serialize insertions with a dedicated LWLock rather than the old + * heavyweight lock. At this point we are past the commit record, so any + * ereport(ERROR) would reach RecordTransactionAbort()'s "cannot abort + * transaction %u, it was already committed" PANIC. LWLockAcquire() + * allocates nothing, consults no lock table, runs no deadlock detector and + * does not call AcceptInvalidationMessages(), so unlike LockSharedObject() + * it cannot throw. It is also released by LWLockReleaseAll() during error + * recovery, and is not subject to the uninterruptible-wait problem that a + * heavyweight lock wait has here (we are inside HOLD_INTERRUPTS()). + * + * Lock ordering: NotifyQueueInsertLock is taken before NotifyQueueLock, + * which is taken before the SLRU bank locks. + */ + LWLockAcquire(NotifyQueueInsertLock, LW_EXCLUSIVE); + + /* Initialize values to a safe default in case list is empty */ + SET_QUEUE_POS(queueHeadBeforeWrite, 0, 0); + SET_QUEUE_POS(queueHeadAfterWrite, 0, 0); + + nextNotify = list_head(pendingNotifies->events); + while (nextNotify != NULL) + { + LWLockAcquire(NotifyQueueLock, LW_EXCLUSIVE); + if (firstIteration) { - /* - * Add the pending notifications to the queue. We acquire and - * release NotifyQueueLock once per page, which might be overkill - * but it does allow readers to get in while we're doing this. - * - * A full queue is very uncommon and should really not happen, - * given that we have so much space available in the SLRU pages. - * Nevertheless we need to deal with this possibility. Note that - * when we get here we are in the process of committing our - * transaction, but we have not yet committed to clog, so at this - * point in time we can still roll the transaction back. - */ - LWLockAcquire(NotifyQueueLock, LW_EXCLUSIVE); - if (firstIteration) - { - queueHeadBeforeWrite = QUEUE_HEAD; - firstIteration = false; - } - asyncQueueFillWarning(); - if (asyncQueueIsFull()) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("too many notifications in the NOTIFY queue"))); - nextNotify = asyncQueueAddEntries(nextNotify); - queueHeadAfterWrite = QUEUE_HEAD; - LWLockRelease(NotifyQueueLock); + queueHeadBeforeWrite = QUEUE_HEAD; + firstIteration = false; } - - /* Note that we don't clear pendingNotifies; AtCommit_Notify will. */ + asyncQueueFillWarning(); + if (asyncQueueIsFull()) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("too many notifications in the NOTIFY queue"))); + nextNotify = asyncQueueAddEntries(nextNotify); + queueHeadAfterWrite = QUEUE_HEAD; + LWLockRelease(NotifyQueueLock); } + + LWLockRelease(NotifyQueueInsertLock); + + /* Note that we don't clear pendingNotifies; AtCommit_Notify will. */ } /* diff --git a/src/backend/storage/lmgr/lwlocknames.h.tmp23721 b/src/backend/storage/lmgr/lwlocknames.h.tmp23721 new file mode 100644 index 0000000000000..ff1dfafffef76 --- /dev/null +++ b/src/backend/storage/lmgr/lwlocknames.h.tmp23721 @@ -0,0 +1,22 @@ +/* autogenerated from src/include/storage/lwlocklist.h, do not edit */ +/* there is deliberately not an #ifndef LWLOCKNAMES_H here */ + +#define OidGenLock (&MainLWLockArray[2].lock) +#define XidGenLock (&MainLWLockArray[3].lock) +#define ProcArrayLock (&MainLWLockArray[4].lock) +#define SInvalReadLock (&MainLWLockArray[5].lock) +#define SInvalWriteLock (&MainLWLockArray[6].lock) +#define WALBufMappingLock (&MainLWLockArray[7].lock) +#define WALWriteLock (&MainLWLockArray[8].lock) +#define ControlFileLock (&MainLWLockArray[9].lock) +#define MultiXactGenLock (&MainLWLockArray[13].lock) +#define RelCacheInitLock (&MainLWLockArray[16].lock) +#define CheckpointerCommLock (&MainLWLockArray[17].lock) +#define TwoPhaseStateLock (&MainLWLockArray[18].lock) +#define TablespaceCreateLock (&MainLWLockArray[19].lock) +#define BtreeVacuumLock (&MainLWLockArray[20].lock) +#define AddinShmemInitLock (&MainLWLockArray[21].lock) +#define AutovacuumLock (&MainLWLockArray[22].lock) +#define AutovacuumScheduleLock (&MainLWLockArray[23].lock) +#define SyncScanLock (&MainLWLockArray[24].lock) +#define RelationMappingLock (&MainLWLockArray[25].lock) diff --git a/src/backend/storage/lmgr/lwlocknames.h.tmp23748 b/src/backend/storage/lmgr/lwlocknames.h.tmp23748 new file mode 100644 index 0000000000000..ff1dfafffef76 --- /dev/null +++ b/src/backend/storage/lmgr/lwlocknames.h.tmp23748 @@ -0,0 +1,22 @@ +/* autogenerated from src/include/storage/lwlocklist.h, do not edit */ +/* there is deliberately not an #ifndef LWLOCKNAMES_H here */ + +#define OidGenLock (&MainLWLockArray[2].lock) +#define XidGenLock (&MainLWLockArray[3].lock) +#define ProcArrayLock (&MainLWLockArray[4].lock) +#define SInvalReadLock (&MainLWLockArray[5].lock) +#define SInvalWriteLock (&MainLWLockArray[6].lock) +#define WALBufMappingLock (&MainLWLockArray[7].lock) +#define WALWriteLock (&MainLWLockArray[8].lock) +#define ControlFileLock (&MainLWLockArray[9].lock) +#define MultiXactGenLock (&MainLWLockArray[13].lock) +#define RelCacheInitLock (&MainLWLockArray[16].lock) +#define CheckpointerCommLock (&MainLWLockArray[17].lock) +#define TwoPhaseStateLock (&MainLWLockArray[18].lock) +#define TablespaceCreateLock (&MainLWLockArray[19].lock) +#define BtreeVacuumLock (&MainLWLockArray[20].lock) +#define AddinShmemInitLock (&MainLWLockArray[21].lock) +#define AutovacuumLock (&MainLWLockArray[22].lock) +#define AutovacuumScheduleLock (&MainLWLockArray[23].lock) +#define SyncScanLock (&MainLWLockArray[24].lock) +#define RelationMappingLock (&MainLWLockArray[25].lock) diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index 1016502d042eb..03c0b2424412c 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -343,6 +343,7 @@ Autovacuum "Waiting to read or update the current state of autovacuum workers." AutovacuumSchedule "Waiting to ensure that a table selected for autovacuum still needs vacuuming." SyncScan "Waiting to select the starting location of a synchronized table scan." RelationMapping "Waiting to read or update a pg_filenode.map file (used to track the filenode assignments of certain system catalogs)." +NotifyQueueInsert "Waiting to insert NOTIFY messages into the queue at commit." NotifyQueue "Waiting to read or update NOTIFY messages." SerializableXactHash "Waiting to read or update information about serializable transactions." SerializableFinishedList "Waiting to access the list of finished serializable transactions." diff --git a/src/include/commands/async.h b/src/include/commands/async.h index 202e4aa5e7452..cdd768c30f077 100644 --- a/src/include/commands/async.h +++ b/src/include/commands/async.h @@ -31,6 +31,7 @@ extern void Async_UnlistenAll(void); /* perform (or cancel) outbound notify processing at transaction commit */ extern void PreCommit_Notify(void); +extern void PostCommitInsert_Notify(void); extern void AtCommit_Notify(void); extern void AtAbort_Notify(void); extern void AtSubCommit_Notify(void); diff --git a/src/include/storage/lwlocklist.h b/src/include/storage/lwlocklist.h index d7eb648bd2758..4dadda0a770c5 100644 --- a/src/include/storage/lwlocklist.h +++ b/src/include/storage/lwlocklist.h @@ -57,7 +57,7 @@ PG_LWLOCK(22, Autovacuum) PG_LWLOCK(23, AutovacuumSchedule) PG_LWLOCK(24, SyncScan) PG_LWLOCK(25, RelationMapping) -/* 26 was NotifySLRULock */ +PG_LWLOCK(26, NotifyQueueInsert) PG_LWLOCK(27, NotifyQueue) PG_LWLOCK(28, SerializableXactHash) PG_LWLOCK(29, SerializableFinishedList)