On 10/12/16 23:10, Petr Jelinek wrote:
>
> The attached 0002-Skip-unnecessary-snapshot-builds.patch changes this
> behavior so that we don't make snapshots for transactions that we seen
> wholly and know that they didn't make catalog changes even if we didn't
> reach consistency yet.
>
Eh, attached wrong patch. This one is correct.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 2add068ed38c2887e6652396c280695a7e384fe7 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Sat, 10 Dec 2016 22:22:13 +0100
Subject: [PATCH 2/2] Skip unnecessary snapshot builds
---
src/backend/replication/logical/snapbuild.c | 82 +++++++++++++++++++----------
1 file changed, 53 insertions(+), 29 deletions(-)
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 5836d52..ea3f40f 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -955,6 +955,7 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
bool forced_timetravel = false;
bool sub_needs_timetravel = false;
bool top_needs_timetravel = false;
+ bool skip_forced_snapshot = false;
TransactionId xmax = xid;
@@ -976,10 +977,19 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
/*
* We could avoid treating !SnapBuildTxnIsRunning transactions as
* timetravel ones, but we want to be able to export a snapshot when
- * we reached consistency.
+ * we reached consistency so we need to keep track of them.
*/
forced_timetravel = true;
elog(DEBUG1, "forced to assume catalog changes for xid %u because it was running too early", xid);
+
+ /*
+ * It is however desirable to skip building new snapshot for
+ * !SnapBuildTxnIsRunning transactions as otherwise we might end up
+ * building thousands of unused snapshots on busy servers which can
+ * be very expensive.
+ */
+ if (!SnapBuildTxnIsRunning(builder, xid))
+ skip_forced_snapshot = true;
}
for (nxact = 0; nxact < nsubxacts; nxact++)
@@ -992,21 +1002,10 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
SnapBuildEndTxn(builder, lsn, subxid);
/*
- * If we're forcing timetravel we also need visibility information
- * about subtransaction, so keep track of subtransaction's state.
- */
- if (forced_timetravel)
- {
- SnapBuildAddCommittedTxn(builder, subxid);
- if (NormalTransactionIdFollows(subxid, xmax))
- xmax = subxid;
- }
-
- /*
* Add subtransaction to base snapshot if it DDL, we don't distinguish
* to toplevel transactions there.
*/
- else if (ReorderBufferXidHasCatalogChanges(builder->reorder, subxid))
+ if (ReorderBufferXidHasCatalogChanges(builder->reorder, subxid))
{
sub_needs_timetravel = true;
@@ -1018,6 +1017,16 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
if (NormalTransactionIdFollows(subxid, xmax))
xmax = subxid;
}
+ /*
+ * If we're forcing timetravel we also need visibility information
+ * about subtransaction, so keep track of subtransaction's state.
+ */
+ else if (forced_timetravel)
+ {
+ SnapBuildAddCommittedTxn(builder, subxid);
+ if (NormalTransactionIdFollows(subxid, xmax))
+ xmax = subxid;
+ }
}
/*
@@ -1026,14 +1035,8 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
*/
SnapBuildEndTxn(builder, lsn, xid);
- if (forced_timetravel)
- {
- elog(DEBUG2, "forced transaction %u to do timetravel.", xid);
-
- SnapBuildAddCommittedTxn(builder, xid);
- }
/* add toplevel transaction to base snapshot */
- else if (ReorderBufferXidHasCatalogChanges(builder->reorder, xid))
+ if (ReorderBufferXidHasCatalogChanges(builder->reorder, xid))
{
elog(DEBUG2, "found top level transaction %u, with catalog changes!",
xid);
@@ -1046,10 +1049,18 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
/* mark toplevel txn as timetravel as well */
SnapBuildAddCommittedTxn(builder, xid);
}
+ else if (forced_timetravel)
+ {
+ elog(DEBUG2, "forced transaction %u to do timetravel.", xid);
+
+ SnapBuildAddCommittedTxn(builder, xid);
+ }
/* if there's any reason to build a historic snapshot, do so now */
if (forced_timetravel || top_needs_timetravel || sub_needs_timetravel)
{
+ bool build_snapshot;
+
/*
* Adjust xmax of the snapshot builder, we only do that for committed,
* catalog modifying, transactions, everything else isn't interesting
@@ -1070,14 +1081,29 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
return;
/*
- * Decrease the snapshot builder's refcount of the old snapshot, note
- * that it still will be used if it has been handed out to the
- * reorderbuffer earlier.
+ * Build snapshot if needed. We need to build it if there isn't one
+ * already built, or if the transaction has made catalog changes or
+ * when we can't know if transaction made catalog changes.
*/
- if (builder->snapshot)
+ build_snapshot = !builder->snapshot || top_needs_timetravel ||
+ sub_needs_timetravel || !skip_forced_snapshot;
+
+ /*
+ * Decrease the snapshot builder's refcount of the old snapshot if we
+ * plan to build new one, note that it still will be used if it has
+ * been handed out to the reorderbuffer earlier.
+ */
+ if (builder->snapshot && build_snapshot)
SnapBuildSnapDecRefcount(builder->snapshot);
- builder->snapshot = SnapBuildBuildSnapshot(builder, xid);
+ /* Build new snapshot unless asked not to. */
+ if (build_snapshot)
+ {
+ builder->snapshot = SnapBuildBuildSnapshot(builder, xid);
+
+ /* refcount of the snapshot builder for the new snapshot */
+ SnapBuildSnapIncRefcount(builder->snapshot);
+ }
/* we might need to execute invalidations, add snapshot */
if (!ReorderBufferXidHasBaseSnapshot(builder->reorder, xid))
@@ -1087,11 +1113,9 @@ SnapBuildCommitTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid,
builder->snapshot);
}
- /* refcount of the snapshot builder for the new snapshot */
- SnapBuildSnapIncRefcount(builder->snapshot);
-
/* add a new Snapshot to all currently running transactions */
- SnapBuildDistributeNewCatalogSnapshot(builder, lsn);
+ if (build_snapshot)
+ SnapBuildDistributeNewCatalogSnapshot(builder, lsn);
}
else
{
--
2.7.4
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
