diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index 3a58f1e..cbbd857 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -262,9 +262,26 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids, status == TRANSACTION_STATUS_ABORTED || (status == TRANSACTION_STATUS_SUB_COMMITTED && !TransactionIdIsValid(xid))); - LWLockAcquire(CLogControlLock, LW_EXCLUSIVE); - /* + * Acquire a Shared lock while we set commit bits, if possible. + * This is safe because all callers of these routines always check + * TransactionIdIsInProgress() first, which will always return true + * for the transaction we are currently committing. As a result, + * anybody checking visibility of our xids will never get as far as + * checking the status here at the same time we are setting it. + * Access to the ProcArrayLock gives us a memory barrier between + * writes and reads for this. + * + * We might have problems with concurrent writes, but we solve + * that by acquiring a specific lock to serialize commits and ensure + * there is a memory barrier between commit writes. Note that we + * acquire and release the lock for each page, to ensure that + * transactions with huge numbers of subxacts don't hold up everyone else. + * + * We might also try to partition the commit lock by slotno, though that + * would mean we queue for the lock while holding ClogCtl. Partitioning + * by pageno would be easily possible; lets wait and see if its needed. + * * If we're doing an async commit (ie, lsn is valid), then we must wait * for any active write on the page slot to complete. Otherwise our * update could reach disk in that write, which will not do since we @@ -273,7 +290,9 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids, * write-busy, since we don't care if the update reaches disk sooner than * we think. */ - slotno = SimpleLruReadPage(ClogCtl, pageno, XLogRecPtrIsInvalid(lsn), xid); + if (!InRecovery) + LWLockAcquire(CommitLock, LW_EXCLUSIVE); + slotno = SimpleLruReadPage_ReadOnly(ClogCtl, pageno, XLogRecPtrIsInvalid(lsn), xid); /* * Set the main transaction id, if any. @@ -312,6 +331,9 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids, ClogCtl->shared->page_dirty[slotno] = true; LWLockRelease(CLogControlLock); + + if (!InRecovery) + LWLockRelease(CommitLock); } /* @@ -401,7 +423,7 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn) /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(ClogCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(ClogCtl, pageno, true, xid); byteptr = ClogCtl->shared->page_buffer[slotno] + byteno; status = (*byteptr >> bshift) & CLOG_XACT_BITMASK; diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c index 5ad35c0..5a5465e 100644 --- a/src/backend/access/transam/commit_ts.c +++ b/src/backend/access/transam/commit_ts.c @@ -324,7 +324,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts, } /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, true, xid); memcpy(&entry, CommitTsCtl->shared->page_buffer[slotno] + SizeOfCommitTimestampEntry * entryno, diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 377d084..0db879e 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -2781,7 +2781,7 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result) return false; /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, multi); + slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, true, multi); offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno]; offptr += entryno; offset = *offptr; diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index 5fcea11..2cc54a8 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -447,7 +447,8 @@ SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok, * It is unspecified whether the lock will be shared or exclusive. */ int -SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid) +SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, bool write_ok, + TransactionId xid) { SlruShared shared = ctl->shared; int slotno; @@ -460,7 +461,9 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid) { if (shared->page_number[slotno] == pageno && shared->page_status[slotno] != SLRU_PAGE_EMPTY && - shared->page_status[slotno] != SLRU_PAGE_READ_IN_PROGRESS) + shared->page_status[slotno] != SLRU_PAGE_READ_IN_PROGRESS && + (write_ok || + shared->page_status[slotno] != SLRU_PAGE_WRITE_IN_PROGRESS)) { /* See comments for SlruRecentlyUsed macro */ SlruRecentlyUsed(shared, slotno); @@ -472,7 +475,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid) LWLockRelease(shared->ControlLock); LWLockAcquire(shared->ControlLock, LW_EXCLUSIVE); - return SimpleLruReadPage(ctl, pageno, true, xid); + return SimpleLruReadPage(ctl, pageno, write_ok, xid); } /* diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index 4bc24d9..195d77e 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -119,7 +119,7 @@ SubTransGetParent(TransactionId xid) /* lock is acquired by SimpleLruReadPage_ReadOnly */ - slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid); + slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, true, xid); ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno]; ptr += entryno; diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 2826b7e..93b2f57 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -1749,7 +1749,7 @@ asyncQueueReadAllNotifications(void) * possibly transmitting them to our frontend. Copy only the part * of the page we will actually inspect. */ - slotno = SimpleLruReadPage_ReadOnly(AsyncCtl, curpage, + slotno = SimpleLruReadPage_ReadOnly(AsyncCtl, curpage, true, InvalidTransactionId); if (curpage == QUEUE_POS_PAGE(head)) { diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index bad5618..4aa37b6 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -963,7 +963,7 @@ OldSerXidGetMinConflictCommitSeqNo(TransactionId xid) * but will return with that lock held, which must then be released. */ slotno = SimpleLruReadPage_ReadOnly(OldSerXidSlruCtl, - OldSerXidPage(xid), xid); + OldSerXidPage(xid), true, xid); val = OldSerXidValue(slotno, xid); LWLockRelease(OldSerXidLock); return val; diff --git a/src/include/access/slru.h b/src/include/access/slru.h index 9c7f019..1d8e3ee 100644 --- a/src/include/access/slru.h +++ b/src/include/access/slru.h @@ -141,7 +141,7 @@ extern int SimpleLruZeroPage(SlruCtl ctl, int pageno); extern int SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok, TransactionId xid); extern int SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, - TransactionId xid); + bool write_ok, TransactionId xid); extern void SimpleLruWritePage(SlruCtl ctl, int slotno); extern void SimpleLruFlush(SlruCtl ctl, bool checkpoint); extern void SimpleLruTruncate(SlruCtl ctl, int cutoffPage); diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index cff3b99..9a05ab8 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -135,8 +135,9 @@ extern PGDLLIMPORT LWLockPadded *MainLWLockArray; #define CommitTsControlLock (&MainLWLockArray[38].lock) #define CommitTsLock (&MainLWLockArray[39].lock) #define ReplicationOriginLock (&MainLWLockArray[40].lock) +#define CommitLock (&MainLWLockArray[41].lock) -#define NUM_INDIVIDUAL_LWLOCKS 41 +#define NUM_INDIVIDUAL_LWLOCKS 42 /* * It's a bit odd to declare NUM_BUFFER_PARTITIONS and NUM_LOCK_PARTITIONS