Avoid SnapshotResetXmin() during AtEOXact_Snapshot()

For normal commits and aborts we already reset PgXact->xmin
Avoiding touching highly contented shmem improves concurrent
performance.

Simon Riggs

Discussion: CANP8+jJdXE9b+b9F8CQT-LuxxO0PBCB-SZFfMVAdp+akqo4zfg@mail.gmail.com

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index c8751c697d4baa82d7e0b3f3eddbe3696ab631e6..cc11177bce03ede5b8c6132fe6d411d586fcc911 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2137,7 +2137,7 @@ CommitTransaction(void)
    AtEOXact_ComboCid();
    AtEOXact_HashTables(true);
    AtEOXact_PgStat(true);
-   AtEOXact_Snapshot(true);
+   AtEOXact_Snapshot(true, false);
    AtCommit_ApplyLauncher();
    pgstat_report_xact_timestamp(0);
 
@@ -2409,7 +2409,7 @@ PrepareTransaction(void)
    AtEOXact_ComboCid();
    AtEOXact_HashTables(true);
    /* don't call AtEOXact_PgStat here; we fixed pgstat state above */
-   AtEOXact_Snapshot(true);
+   AtEOXact_Snapshot(true, true);
    pgstat_report_xact_timestamp(0);
 
    CurrentResourceOwner = NULL;
@@ -2640,7 +2640,7 @@ CleanupTransaction(void)
     * do abort cleanup processing
     */
    AtCleanup_Portals();        /* now safe to release portal memory */
-   AtEOXact_Snapshot(false);   /* and release the transaction's snapshots */
+   AtEOXact_Snapshot(false, false); /* and release the transaction's snapshots */
 
    CurrentResourceOwner = NULL;    /* and resource owner */
    if (TopTransactionResourceOwner)

diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index ecc32cb3fe76f31ddce840adc665eb535950b0f8..67f3e2d667d2c8fe3b722ecf942a1e98c7fe068f 100644 (file)
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -954,7 +954,12 @@ xmin_cmp(const pairingheap_node *a, const pairingheap_node *b, void *arg)
  *
  * If there are no more snapshots, we can reset our PGXACT->xmin to InvalidXid.
  * Note we can do this without locking because we assume that storing an Xid
- * is atomic.
+ * is atomic. We do this because it will allow multi-statement transactions to
+ * reset their xmin and prevent us from holding back removal of dead rows;
+ * this has little purpose when we are dealing with very fast statements in
+ * read committed mode since the xmin will advance quickly anyway. It has no
+ * use at all when we are running single statement transactions since the xmin
+ * is reset as part of end of transaction anyway.
  *
  * Even if there are some remaining snapshots, we may be able to advance our
  * PGXACT->xmin to some degree.  This typically happens when a portal is
@@ -1051,7 +1056,7 @@ AtSubAbort_Snapshot(int level)
  *     Snapshot manager's cleanup function for end of transaction
  */
 void
-AtEOXact_Snapshot(bool isCommit)
+AtEOXact_Snapshot(bool isCommit, bool isPrepare)
 {
    /*
     * In transaction-snapshot mode we must release our privately-managed
@@ -1136,7 +1141,17 @@ AtEOXact_Snapshot(bool isCommit)
 
    FirstSnapshotSet = false;
 
-   SnapshotResetXmin();
+   /*
+    * During normal commit and abort processing, we call
+    * ProcArrayEndTransaction() or ProcArrayClearTransaction() to
+    * reset the PgXact->xmin. That call happens prior to the call to
+    * AtEOXact_Snapshot(), so we need not touch xmin here at all,
+    * accept when we are preparing a transaction.
+    */
+   if (isPrepare)
+       SnapshotResetXmin();
+
+   Assert(MyPgXact->xmin == 0);
 }
 
 

diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 86c48d9c8eff6ff90bb9635a57d60c2bd9aa4597..2f8d4fd4a0ca531c85d504888e080cb9b4ec0c4e 100644 (file)
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -85,7 +85,7 @@ extern void UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner);
 
 extern void AtSubCommit_Snapshot(int level);
 extern void AtSubAbort_Snapshot(int level);
-extern void AtEOXact_Snapshot(bool isCommit);
+extern void AtEOXact_Snapshot(bool isCommit, bool isPrepare);
 
 extern void ImportSnapshot(const char *idstr);
 extern bool XactHasExportedSnapshots(void);