public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v13 6/6] snapshot scalability: cache snapshots using a xact completion counter.
3+ messages / 3 participants
[nested] [flat]
* [PATCH v13 6/6] snapshot scalability: cache snapshots using a xact completion counter.
@ 2020-07-15 22:35 Andres Freund <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Andres Freund @ 2020-07-15 22:35 UTC (permalink / raw)
Previous commits made it faster/more scalable to compute snapshots. But not
building a snapshot is still faster. Now that GetSnapshotData() does not
maintain RecentGlobal* anymore, that is actually not too hard:
This commit introduces xactCompletionCount, which tracks the number of
top-level transactions with xids (i.e. which may have modified the database)
that completed in some form since the start of the server.
We can avoid rebuilding the snapshot's contents whenever the current
xactCompletionCount is the same as it was when the snapshot was
originally built. Currently this check happens while holding
ProcArrayLock. While it's likely possible to perform the check before
acquiring ProcArrayLock, it's too complicated for now.
Author: Andres Freund
Reviewed-By: Robert Haas, Thomas Munro, David Rowley
Discussion: https://postgr.es/m/[email protected]
---
src/include/access/transam.h | 9 ++
src/include/utils/snapshot.h | 7 ++
src/backend/replication/logical/snapbuild.c | 1 +
src/backend/storage/ipc/procarray.c | 125 ++++++++++++++++----
src/backend/utils/time/snapmgr.c | 4 +
5 files changed, 126 insertions(+), 20 deletions(-)
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index b32044153b0..2f1f144db4d 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -231,6 +231,15 @@ typedef struct VariableCacheData
FullTransactionId latestCompletedXid; /* newest full XID that has
* committed or aborted */
+ /*
+ * Number of top-level transactions with xids (i.e. which may have
+ * modified the database) that completed in some form since the start of
+ * the server. This currently is solely used to check whether
+ * GetSnapshotData() needs to recompute the contents of the snapshot, or
+ * not. There are likely other users of this. Always above 1.
+ */
+ uint64 xactCompletionCount;
+
/*
* These fields are protected by XactTruncationLock
*/
diff --git a/src/include/utils/snapshot.h b/src/include/utils/snapshot.h
index 35b1f05bea6..dea072e5edf 100644
--- a/src/include/utils/snapshot.h
+++ b/src/include/utils/snapshot.h
@@ -207,6 +207,13 @@ typedef struct SnapshotData
TimestampTz whenTaken; /* timestamp when snapshot was taken */
XLogRecPtr lsn; /* position in the WAL stream when taken */
+
+ /*
+ * The transaction completion count at the time GetSnapshotData() built
+ * this snapshot. Allows to avoid re-computing static snapshots when no
+ * transactions completed since the last GetSnapshotData().
+ */
+ uint64 snapXactCompletionCount;
} SnapshotData;
#endif /* SNAPSHOT_H */
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index e9701ea7221..9d5d68f3fa7 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -524,6 +524,7 @@ SnapBuildBuildSnapshot(SnapBuild *builder)
snapshot->curcid = FirstCommandId;
snapshot->active_count = 0;
snapshot->regd_count = 0;
+ snapshot->snapXactCompletionCount = 0;
return snapshot;
}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 8e8049d9715..ac62343c1bb 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -407,6 +407,7 @@ CreateSharedProcArray(void)
procArray->lastOverflowedXid = InvalidTransactionId;
procArray->replication_slot_xmin = InvalidTransactionId;
procArray->replication_slot_catalog_xmin = InvalidTransactionId;
+ ShmemVariableCache->xactCompletionCount = 1;
}
allProcs = ProcGlobal->allProcs;
@@ -534,6 +535,9 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
/* Advance global latestCompletedXid while holding the lock */
MaintainLatestCompletedXid(latestXid);
+ /* Same with xactCompletionCount */
+ ShmemVariableCache->xactCompletionCount++;
+
ProcGlobal->xids[proc->pgxactoff] = 0;
ProcGlobal->subxidStates[proc->pgxactoff].overflowed = false;
ProcGlobal->subxidStates[proc->pgxactoff].count = 0;
@@ -668,6 +672,7 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
{
size_t pgxactoff = proc->pgxactoff;
+ Assert(LWLockHeldByMe(ProcArrayLock));
Assert(TransactionIdIsValid(ProcGlobal->xids[pgxactoff]));
Assert(ProcGlobal->xids[pgxactoff] == proc->xid);
@@ -699,6 +704,9 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
/* Also advance global latestCompletedXid while holding the lock */
MaintainLatestCompletedXid(latestXid);
+
+ /* Same with xactCompletionCount */
+ ShmemVariableCache->xactCompletionCount++;
}
/*
@@ -1913,6 +1921,93 @@ GetMaxSnapshotSubxidCount(void)
return TOTAL_MAX_CACHED_SUBXIDS;
}
+/*
+ * Initialize old_snapshot_threshold specific parts of a newly build snapshot.
+ */
+static void
+GetSnapshotDataInitOldSnapshot(Snapshot snapshot)
+{
+ if (!OldSnapshotThresholdActive())
+ {
+ /*
+ * If not using "snapshot too old" feature, fill related fields with
+ * dummy values that don't require any locking.
+ */
+ snapshot->lsn = InvalidXLogRecPtr;
+ snapshot->whenTaken = 0;
+ }
+ else
+ {
+ /*
+ * Capture the current time and WAL stream location in case this
+ * snapshot becomes old enough to need to fall back on the special
+ * "old snapshot" logic.
+ */
+ snapshot->lsn = GetXLogInsertRecPtr();
+ snapshot->whenTaken = GetSnapshotCurrentTimestamp();
+ MaintainOldSnapshotTimeMapping(snapshot->whenTaken, snapshot->xmin);
+ }
+}
+
+/*
+ * Helper function for GetSnapshotData() that check if the bulk of the
+ * visibility information in the snapshot is still valid. If so, it updates
+ * the fields that need to change and returns true. Otherwise it returns
+ * false.
+ *
+ * This very likely can be evolved to not need ProcArrayLock held (at very
+ * least in the case we already hold a snapshot), but that's for another day.
+ */
+static bool
+GetSnapshotDataReuse(Snapshot snapshot)
+{
+ uint64 curXactCompletionCount;
+
+ Assert(LWLockHeldByMe(ProcArrayLock));
+
+ if (unlikely(snapshot->snapXactCompletionCount == 0))
+ return false;
+
+ curXactCompletionCount = ShmemVariableCache->xactCompletionCount;
+ if (curXactCompletionCount != snapshot->snapXactCompletionCount)
+ return false;
+
+ /*
+ * If the current xactCompletionCount is still the same as it was at the
+ * time the snapshot was built, we can be sure that rebuilding the
+ * contents of the snapshot the hard way would result in the same snapshot
+ * contents:
+ *
+ * As explained in transam/README, the set of xids considered running by
+ * GetSnapshotData() cannot change while ProcArrayLock is held. Snapshot
+ * contents only depend on transactions with xids and xactCompletionCount
+ * is incremented whenever a transaction with an xid finishes (while
+ * holding ProcArrayLock) exclusively). Thus the xactCompletionCount check
+ * ensures we would detect if the snapshot would have changed.
+ *
+ * As the snapshot contents are the same as it was before, it is is safe
+ * to re-enter the snapshot's xmin into the PGPROC array. None of the rows
+ * visible under the snapshot could already have been removed (that'd
+ * require the set of running transactions to change) and it fulfills the
+ * requirement that concurrent GetSnapshotData() calls yield the same
+ * xmin.
+ */
+ if (!TransactionIdIsValid(MyProc->xmin))
+ MyProc->xmin = TransactionXmin = snapshot->xmin;
+
+ RecentXmin = snapshot->xmin;
+ Assert(TransactionIdPrecedesOrEquals(TransactionXmin, RecentXmin));
+
+ snapshot->curcid = GetCurrentCommandId(false);
+ snapshot->active_count = 0;
+ snapshot->regd_count = 0;
+ snapshot->copied = false;
+
+ GetSnapshotDataInitOldSnapshot(snapshot);
+
+ return true;
+}
+
/*
* GetSnapshotData -- returns information about running transactions.
*
@@ -1961,6 +2056,7 @@ GetSnapshotData(Snapshot snapshot)
TransactionId oldestxid;
int mypgxactoff;
TransactionId myxid;
+ uint64 curXactCompletionCount;
TransactionId replication_slot_xmin = InvalidTransactionId;
TransactionId replication_slot_catalog_xmin = InvalidTransactionId;
@@ -2005,12 +2101,19 @@ GetSnapshotData(Snapshot snapshot)
*/
LWLockAcquire(ProcArrayLock, LW_SHARED);
+ if (GetSnapshotDataReuse(snapshot))
+ {
+ LWLockRelease(ProcArrayLock);
+ return snapshot;
+ }
+
latest_completed = ShmemVariableCache->latestCompletedXid;
mypgxactoff = MyProc->pgxactoff;
myxid = other_xids[mypgxactoff];
Assert(myxid == MyProc->xid);
oldestxid = ShmemVariableCache->oldestXid;
+ curXactCompletionCount = ShmemVariableCache->xactCompletionCount;
/* xmax is always latestCompletedXid + 1 */
xmax = XidFromFullTransactionId(latest_completed);
@@ -2264,6 +2367,7 @@ GetSnapshotData(Snapshot snapshot)
snapshot->xcnt = count;
snapshot->subxcnt = subcount;
snapshot->suboverflowed = suboverflowed;
+ snapshot->snapXactCompletionCount = curXactCompletionCount;
snapshot->curcid = GetCurrentCommandId(false);
@@ -2275,26 +2379,7 @@ GetSnapshotData(Snapshot snapshot)
snapshot->regd_count = 0;
snapshot->copied = false;
- if (old_snapshot_threshold < 0)
- {
- /*
- * If not using "snapshot too old" feature, fill related fields with
- * dummy values that don't require any locking.
- */
- snapshot->lsn = InvalidXLogRecPtr;
- snapshot->whenTaken = 0;
- }
- else
- {
- /*
- * Capture the current time and WAL stream location in case this
- * snapshot becomes old enough to need to fall back on the special
- * "old snapshot" logic.
- */
- snapshot->lsn = GetXLogInsertRecPtr();
- snapshot->whenTaken = GetSnapshotCurrentTimestamp();
- MaintainOldSnapshotTimeMapping(snapshot->whenTaken, xmin);
- }
+ GetSnapshotDataInitOldSnapshot(snapshot);
return snapshot;
}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 689a3b6a597..09ea03c2063 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -595,6 +595,8 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid,
CurrentSnapshot->takenDuringRecovery = sourcesnap->takenDuringRecovery;
/* NB: curcid should NOT be copied, it's a local matter */
+ CurrentSnapshot->snapXactCompletionCount = 0;
+
/*
* Now we have to fix what GetSnapshotData did with MyProc->xmin and
* TransactionXmin. There is a race condition: to make sure we are not
@@ -670,6 +672,7 @@ CopySnapshot(Snapshot snapshot)
newsnap->regd_count = 0;
newsnap->active_count = 0;
newsnap->copied = true;
+ newsnap->snapXactCompletionCount = 0;
/* setup XID array */
if (snapshot->xcnt > 0)
@@ -2207,6 +2210,7 @@ RestoreSnapshot(char *start_address)
snapshot->curcid = serialized_snapshot.curcid;
snapshot->whenTaken = serialized_snapshot.whenTaken;
snapshot->lsn = serialized_snapshot.lsn;
+ snapshot->snapXactCompletionCount = 0;
/* Copy XIDs, if present. */
if (serialized_snapshot.xcnt > 0)
--
2.25.0.114.g5b0ca878e0
--4il4azyv6rmablac--
^ permalink raw reply [nested|flat] 3+ messages in thread
* Tarball builds in the new world order
@ 2024-04-23 22:05 Tom Lane <[email protected]>
2024-04-24 14:38 ` Re: Tarball builds in the new world order Greg Sabino Mullane <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2024-04-23 22:05 UTC (permalink / raw)
To: [email protected]; +Cc: Peter Eisentraut <[email protected]>; Magnus Hagander <[email protected]>
With one eye on the beta-release calendar, I thought it'd be a
good idea to test whether our tarball build script works for
the new plan where we'll use "git archive" instead of the
traditional process.
It doesn't.
It makes tarballs all right, but whatever commit ID you specify
is semi-ignored, and you get a tarball corresponding to HEAD
of master. (The PDFs come from the right version, though!)
The reason for that is that the mk-one-release script does this
(shorn of not-relevant-here details):
export BASE=/home/pgsql
export GIT_DIR=$BASE/postgresql.git
mkdir pgsql
# Export the selected git ref
git archive ${gitref} | tar xf - -C pgsql
cd pgsql
./configure
# Produce .tar.gz and .tar.bz2 files
make dist
Since 619bc23a1, what "make dist" does is
$(GIT) -C $(srcdir) -c core.autocrlf=false archive --format tar.gz -9 --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
$(GIT) -C $(srcdir) -c core.autocrlf=false -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
Since GIT_DIR is set, git consults that repo not the current working
directory, so HEAD means whatever it means in a just-fetched repo,
and mk-one-release's efforts to select the ${gitref} commit mean
nothing. (If git had tried to consult the current working directory,
it would've failed for lack of any .git subdir therein.)
I really really don't want to put version-specific coding into
mk-one-release, but fortunately I think we don't have to.
What I suggest is doing this in mk-one-release:
-make dist
+make dist PG_COMMIT_HASH=${gitref}
and changing the "make dist" rules to write $(PG_COMMIT_HASH) not
HEAD. The extra make variable will have no effect in the back
branches, while it should cause the right thing to happen with
the new implementation of "make dist".
This change seems like a good thing anyway for anyone who's tempted
to use "make dist" manually, since they wouldn't necessarily want
to package HEAD either. Now, if we just do it exactly like that
then trying to "make dist" without setting PG_COMMIT_HASH will
fail, since "git archive" has no default for its <tree-ish>
argument. I can't quite decide if that's a good thing, or if we
should hack the makefile a little further to allow PG_COMMIT_HASH
to default to HEAD.
Thoughts, better ideas?
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Tarball builds in the new world order
2024-04-23 22:05 Tarball builds in the new world order Tom Lane <[email protected]>
@ 2024-04-24 14:38 ` Greg Sabino Mullane <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Greg Sabino Mullane @ 2024-04-24 14:38 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]; Peter Eisentraut <[email protected]>; Magnus Hagander <[email protected]>
On Tue, Apr 23, 2024 at 6:06 PM Tom Lane <[email protected]> wrote:
> This change seems like a good thing anyway for anyone who's tempted
> to use "make dist" manually, since they wouldn't necessarily want
> to package HEAD either. Now, if we just do it exactly like that
> then trying to "make dist" without setting PG_COMMIT_HASH will
> fail, since "git archive" has no default for its <tree-ish>
> argument. I can't quite decide if that's a good thing, or if we
> should hack the makefile a little further to allow PG_COMMIT_HASH
> to default to HEAD.
>
Just having it fail seems harsh. What if we had plain "make dist" at least
output a friendly hint about "please specify a hash"? That seems better
than an implicit HEAD default, as they can manually set it to HEAD
themselves per the hint.
Cheers,
Greg
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-04-24 14:38 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 22:35 [PATCH v13 6/6] snapshot scalability: cache snapshots using a xact completion counter. Andres Freund <[email protected]>
2024-04-23 22:05 Tarball builds in the new world order Tom Lane <[email protected]>
2024-04-24 14:38 ` Re: Tarball builds in the new world order Greg Sabino Mullane <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox