From 2c50a3f3f48e0529533c1cce451dab6cbf792928 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Tue, 16 Nov 2021 16:12:27 +0530 Subject: [PATCH v5] Fix parallel operations that prevent oldest xmin from advancing. While determining xid horizons, we skip over backends that are running Vacuum. We also ignore Create Index Concurrently, or Reindex Concurrently for the purposes of computing Xmin for Vacuum. But we were not setting the flags corresponding to these operations when they are performed in parallel which was preventing Xid horizon from advancing. The optimization related to skipping Create Index Concurrently, or Reindex Concurrently operations was implemented in PG-14 but the fix is the same for the Parallel Vacuum as well so back-patched till PG-13. Author: Masahiko Sawada Reviewed-by: Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAD21AoCLQqgM1sXh9BrDFq0uzd3RBFKi=Vfo6cjjKODm0Onr5w@mail.gmail.com --- src/backend/access/heap/vacuumlazy.c | 6 ++++++ src/backend/access/nbtree/nbtsort.c | 7 +++++++ src/backend/storage/ipc/procarray.c | 17 +++++++++++++++-- src/include/storage/proc.h | 7 +++++++ 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4ee1f14..a00947e 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -4159,6 +4159,12 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) LVRelState vacrel; ErrorContextCallback errcallback; + /* + * A parallel vacuum worker must have only PROC_IN_VACUUM flag since we + * don't support parallel vacuum for autovacuum as of now. + */ + Assert(MyProc->statusFlags == PROC_IN_VACUUM); + lvshared = (LVShared *) shm_toc_lookup(toc, PARALLEL_VACUUM_KEY_SHARED, false); elevel = lvshared->elevel; diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index 54c8eb1..1e02be9 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -1808,6 +1808,13 @@ _bt_parallel_build_main(dsm_segment *seg, shm_toc *toc) ResetUsage(); #endif /* BTREE_BUILD_STATS */ + /* + * The only possible status flag that can be set to the parallel worker is + * PROC_IN_SAFE_IC. + */ + Assert((MyProc->statusFlags == 0) || + (MyProc->statusFlags == PROC_IN_SAFE_IC)); + /* Set debug_query_string for individual workers first */ sharedquery = shm_toc_lookup(toc, PARALLEL_KEY_QUERY_TEXT, true); debug_query_string = sharedquery; diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 892f0f6..a9945c8 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2638,6 +2638,10 @@ ProcArrayInstallImportedXmin(TransactionId xmin, * PGPROC of the transaction from which we imported the snapshot, rather than * an XID. * + * Note that this function also copies statusFlags from the source `proc` in + * order to avoid the case where MyProc's xmin needs to be skipped for + * computing xid horizon. + * * Returns true if successful, false if source xact is no longer running. */ bool @@ -2649,8 +2653,10 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc) Assert(TransactionIdIsNormal(xmin)); Assert(proc != NULL); - /* Get lock so source xact can't end while we're doing this */ - LWLockAcquire(ProcArrayLock, LW_SHARED); + /* + * Get an exclusive lock so that we can copy statusFlags from source proc. + */ + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); /* * Be certain that the referenced PGPROC has an advertised xmin which is @@ -2663,7 +2669,14 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc) TransactionIdIsNormal(xid) && TransactionIdPrecedesOrEquals(xid, xmin)) { + /* Install xmin */ MyProc->xmin = TransactionXmin = xmin; + + /* Flags being copied must be valid copy-able flags. */ + Assert((proc->statusFlags & (~PROC_COPYABLE_FLAGS)) == 0); + MyProc->statusFlags = proc->statusFlags; + ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; + result = true; } diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index be67d8a..cfabfdb 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -66,6 +66,13 @@ struct XidCache (PROC_IN_VACUUM | PROC_IN_SAFE_IC | PROC_VACUUM_FOR_WRAPAROUND) /* + * Flags that are valid to copy from another proc, the parallel leader + * process in practice. Currently, flags that are set during parallel + * vacuum and parallel index creation are allowed. + */ +#define PROC_COPYABLE_FLAGS (PROC_IN_VACUUM | PROC_IN_SAFE_IC) + +/* * We allow a small number of "weak" relation locks (AccessShareLock, * RowShareLock, RowExclusiveLock) to be recorded in the PGPROC structure * rather than the main lock table. This eases contention on the lock -- 1.8.3.1