There is a race condition that could lead to problems

for the CLOG and sub transactions.

In Postgres-XC, multiple processes may decide to extend the CLOG
at the same time. One will wait for the other, then afterwards
re-zero out the page.

Instead, once the lock is obtained, we re-check to make sure
that another process did not already extend and create the page.
If so, we just exit.

2 files changed, 35 insertions, 2 deletions

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 8dc23f7039..b975ac1d27 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -590,6 +590,9 @@ ExtendCLOG(TransactionId newestXact)
 	/* 
 	 * The first condition makes sure we did not wrap around 
 	 * The second checks if we are still using the same page
+	 * Note that this value can change and we are not holding a lock, 
+	 * so we repeat the check below. We do it this way instead of 
+	 * grabbing the lock to avoid lock contention.
 	 */
 	if (ClogCtl->shared->latest_page_number - pageno <= CLOG_WRAP_CHECK_DELTA 
 			&& pageno <= ClogCtl->shared->latest_page_number)
@@ -604,6 +607,20 @@ ExtendCLOG(TransactionId newestXact)
 
 	LWLockAcquire(CLogControlLock, LW_EXCLUSIVE);
 
+#ifdef PGXC
+	/*
+	 * We repeat the check.  Another process may have written 
+	 * out the page already and advanced the latest_page_number
+	 * while we were waiting for the lock.
+	 */
+	if (ClogCtl->shared->latest_page_number - pageno <= CLOG_WRAP_CHECK_DELTA 
+			&& pageno <= ClogCtl->shared->latest_page_number)
+	{
+		LWLockRelease(CLogControlLock);
+		return;
+	}
+#endif
+
 	/* Zero the page and make an XLOG entry about it */
 	ZeroCLOGPage(pageno, true);
 
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 2695085be3..c468c631f3 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -294,7 +294,6 @@ CheckPointSUBTRANS(void)
 	TRACE_POSTGRESQL_SUBTRANS_CHECKPOINT_DONE(true);
 }
 
-
 /*
  * Make sure that SUBTRANS has room for a newly-allocated XID.
  *
@@ -325,7 +324,10 @@ ExtendSUBTRANS(TransactionId newestXact)
 
 	/* 
 	 * The first condition makes sure we did not wrap around 
-	 * The second checks if we are still using the same page
+	 * The second checks if we are still using the same page.
+	 * Note that this value can change and we are not holding a lock, 
+	 * so we repeat the check below. We do it this way instead of 
+	 * grabbing the lock to avoid lock contention.
 	 */
 	if (SubTransCtl->shared->latest_page_number - pageno <= SUBTRANS_WRAP_CHECK_DELTA 
 			&& pageno <= SubTransCtl->shared->latest_page_number)
@@ -340,6 +342,20 @@ ExtendSUBTRANS(TransactionId newestXact)
 
 	LWLockAcquire(SubtransControlLock, LW_EXCLUSIVE);
 
+#ifdef PGXC
+	/*
+	 * We repeat the check.  Another process may have written 
+	 * out the page already and advanced the latest_page_number
+	 * while we were waiting for the lock.
+	 */
+	if (SubTransCtl->shared->latest_page_number - pageno <= SUBTRANS_WRAP_CHECK_DELTA 
+			&& pageno <= SubTransCtl->shared->latest_page_number)
+	{
+		LWLockRelease(SubtransControlLock);
+		return;
+	}
+#endif
+
 	/* Zero the page */
 	ZeroSUBTRANSPage(pageno);