Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit dcfff74

Browse filesBrowse files
alvherrealvherre
committed
Restore lock level to update statusFlags
Reverts 2783898 (some comments are kept). Per discussion, it does not seem safe to relax the lock level used for this; in order for it to be safe, there would have to be memory barriers between the point we set the flag and the point we set the trasaction Xid, which perhaps would not be so bad; but there would also have to be barriers at the readers' side, which from a performance perspective might be bad. Now maybe this analysis is wrong and it *is* safe for some reason, but proof of that is not trivial. Discussion: https://postgr.es/m/20201118190928.vnztes7c2sldu43a@alap3.anarazel.de
1 parent 9fbc3f3 commit dcfff74
Copy full SHA for dcfff74

File tree

Expand file treeCollapse file tree

4 files changed

+18
-18
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+18
-18
lines changed

‎src/backend/commands/vacuum.c

Copy file name to clipboardExpand all lines: src/backend/commands/vacuum.c
+11-9Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,13 +1712,6 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
17121712
/* Begin a transaction for vacuuming this relation */
17131713
StartTransactionCommand();
17141714

1715-
/*
1716-
* Need to acquire a snapshot to prevent pg_subtrans from being truncated,
1717-
* cutoff xids in local memory wrapping around, and to have updated xmin
1718-
* horizons.
1719-
*/
1720-
PushActiveSnapshot(GetTransactionSnapshot());
1721-
17221715
if (!(params->options & VACOPT_FULL))
17231716
{
17241717
/*
@@ -1739,16 +1732,25 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
17391732
* Note: these flags remain set until CommitTransaction or
17401733
* AbortTransaction. We don't want to clear them until we reset
17411734
* MyProc->xid/xmin, otherwise GetOldestNonRemovableTransactionId()
1742-
* might appear to go backwards, which is probably Not Good.
1735+
* might appear to go backwards, which is probably Not Good. (We also
1736+
* set PROC_IN_VACUUM *before* taking our own snapshot, so that our
1737+
* xmin doesn't become visible ahead of setting the flag.)
17431738
*/
1744-
LWLockAcquire(ProcArrayLock, LW_SHARED);
1739+
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
17451740
MyProc->statusFlags |= PROC_IN_VACUUM;
17461741
if (params->is_wraparound)
17471742
MyProc->statusFlags |= PROC_VACUUM_FOR_WRAPAROUND;
17481743
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
17491744
LWLockRelease(ProcArrayLock);
17501745
}
17511746

1747+
/*
1748+
* Need to acquire a snapshot to prevent pg_subtrans from being truncated,
1749+
* cutoff xids in local memory wrapping around, and to have updated xmin
1750+
* horizons.
1751+
*/
1752+
PushActiveSnapshot(GetTransactionSnapshot());
1753+
17521754
/*
17531755
* Check for user-requested abort. Note we want this to be inside a
17541756
* transaction, so xact.c doesn't issue useless WARNING.

‎src/backend/replication/logical/logical.c

Copy file name to clipboardExpand all lines: src/backend/replication/logical/logical.c
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ StartupDecodingContext(List *output_plugin_options,
181181
*/
182182
if (!IsTransactionOrTransactionBlock())
183183
{
184-
LWLockAcquire(ProcArrayLock, LW_SHARED);
184+
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
185185
MyProc->statusFlags |= PROC_IN_LOGICAL_DECODING;
186186
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
187187
LWLockRelease(ProcArrayLock);

‎src/backend/storage/ipc/procarray.c

Copy file name to clipboardExpand all lines: src/backend/storage/ipc/procarray.c
+3-5Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -662,10 +662,8 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
662662
/* avoid unnecessarily dirtying shared cachelines */
663663
if (proc->statusFlags & PROC_VACUUM_STATE_MASK)
664664
{
665-
/* Only safe to change my own flags with just share lock */
666-
Assert(proc == MyProc);
667665
Assert(!LWLockHeldByMe(ProcArrayLock));
668-
LWLockAcquire(ProcArrayLock, LW_SHARED);
666+
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
669667
Assert(proc->statusFlags == ProcGlobal->statusFlags[proc->pgxactoff]);
670668
proc->statusFlags &= ~PROC_VACUUM_STATE_MASK;
671669
ProcGlobal->statusFlags[proc->pgxactoff] = proc->statusFlags;
@@ -685,8 +683,8 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
685683
size_t pgxactoff = proc->pgxactoff;
686684

687685
/*
688-
* Note: we need exclusive lock here because we're going to
689-
* change other processes' PGPROC entries.
686+
* Note: we need exclusive lock here because we're going to change other
687+
* processes' PGPROC entries.
690688
*/
691689
Assert(LWLockHeldByMeInMode(ProcArrayLock, LW_EXCLUSIVE));
692690
Assert(TransactionIdIsValid(ProcGlobal->xids[pgxactoff]));

‎src/include/storage/proc.h

Copy file name to clipboardExpand all lines: src/include/storage/proc.h
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ typedef enum
102102
* but its myProcLocks[] lists are valid.
103103
*
104104
* We allow many fields of this struct to be accessed without locks, such as
105-
* statusFlags or delayChkpt. However, keep in mind that writing mirrored ones
106-
* (see below) requires holding ProcArrayLock or XidGenLock in at least shared
107-
* mode, so that pgxactoff does not change concurrently.
105+
* delayChkpt and isBackgroundWorker. However, keep in mind that writing
106+
* mirrored ones (see below) requires holding ProcArrayLock or XidGenLock in
107+
* at least shared mode, so that pgxactoff does not change concurrently.
108108
*
109109
* Mirrored fields:
110110
*

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.