diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 395e75f768f..f1112111de8 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1712,13 +1712,6 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) /* Begin a transaction for vacuuming this relation */ StartTransactionCommand(); - /* - * Need to acquire a snapshot to prevent pg_subtrans from being truncated, - * cutoff xids in local memory wrapping around, and to have updated xmin - * horizons. - */ - PushActiveSnapshot(GetTransactionSnapshot()); - if (!(params->options & VACOPT_FULL)) { /* @@ -1739,9 +1732,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) * Note: these flags remain set until CommitTransaction or * AbortTransaction. We don't want to clear them until we reset * MyProc->xid/xmin, otherwise GetOldestNonRemovableTransactionId() - * might appear to go backwards, which is probably Not Good. + * might appear to go backwards, which is probably Not Good. (We also + * set PROC_IN_VACUUM *before* taking our own snapshot, so that our + * xmin doesn't become visible ahead of setting the flag.) */ - LWLockAcquire(ProcArrayLock, LW_SHARED); + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); MyProc->statusFlags |= PROC_IN_VACUUM; if (params->is_wraparound) MyProc->statusFlags |= PROC_VACUUM_FOR_WRAPAROUND; @@ -1749,6 +1744,13 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) LWLockRelease(ProcArrayLock); } + /* + * Need to acquire a snapshot to prevent pg_subtrans from being truncated, + * cutoff xids in local memory wrapping around, and to have updated xmin + * horizons. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Check for user-requested abort. Note we want this to be inside a * transaction, so xact.c doesn't issue useless WARNING. diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 4324e326565..f1f4df7d70f 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -181,7 +181,7 @@ StartupDecodingContext(List *output_plugin_options, */ if (!IsTransactionOrTransactionBlock()) { - LWLockAcquire(ProcArrayLock, LW_SHARED); + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); MyProc->statusFlags |= PROC_IN_LOGICAL_DECODING; ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; LWLockRelease(ProcArrayLock); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 94edb24b22d..ee912b9d5e4 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -662,10 +662,8 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) /* avoid unnecessarily dirtying shared cachelines */ if (proc->statusFlags & PROC_VACUUM_STATE_MASK) { - /* Only safe to change my own flags with just share lock */ - Assert(proc == MyProc); Assert(!LWLockHeldByMe(ProcArrayLock)); - LWLockAcquire(ProcArrayLock, LW_SHARED); + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); Assert(proc->statusFlags == ProcGlobal->statusFlags[proc->pgxactoff]); proc->statusFlags &= ~PROC_VACUUM_STATE_MASK; ProcGlobal->statusFlags[proc->pgxactoff] = proc->statusFlags; @@ -685,8 +683,8 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid) size_t pgxactoff = proc->pgxactoff; /* - * Note: we need exclusive lock here because we're going to - * change other processes' PGPROC entries. + * Note: we need exclusive lock here because we're going to change other + * processes' PGPROC entries. */ Assert(LWLockHeldByMeInMode(ProcArrayLock, LW_EXCLUSIVE)); Assert(TransactionIdIsValid(ProcGlobal->xids[pgxactoff])); diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index e75f6e81782..e77f76ae8a1 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -102,9 +102,9 @@ typedef enum * but its myProcLocks[] lists are valid. * * We allow many fields of this struct to be accessed without locks, such as - * statusFlags or delayChkpt. However, keep in mind that writing mirrored ones - * (see below) requires holding ProcArrayLock or XidGenLock in at least shared - * mode, so that pgxactoff does not change concurrently. + * delayChkpt and isBackgroundWorker. However, keep in mind that writing + * mirrored ones (see below) requires holding ProcArrayLock or XidGenLock in + * at least shared mode, so that pgxactoff does not change concurrently. * * Mirrored fields: *