From: Pavan Deolasee <pavan.deolasee@gmail.com>
Date: Thu, 15 Jun 2017 06:04:06 +0000 (+0530)
Subject: Use thread-specific storage for computing a snapshot.
X-Git-Tag: XL_10_R1BETA1~265^2~7
X-Git-Url: http://git.postgresql.org/gitweb/-?a=commitdiff_plain;h=fb56418d660437d1cbaa0aa7a4260ca826792ec9;p=postgres-xl.git

Use thread-specific storage for computing a snapshot.

We mustn't use a global variable since concurrent GTM threads might try to
compute a snapshot at the same time and may overwrite the information before a
thread can send the complete snapshot to the client. Chi Gao
<chi.gao@microfun.com> reported that this can cause infinite wait on the client
side because the client expects N bytes of data, but only receives (N - x)
bytes and it keeps waiting for remaining x bytes which the GTM never sends.
While we don't have a report, it's obvious that it can also go wrong in the
other direction.

We fix this by using a thread-specific storage which ensures that the snapshot
cannot be changed while it's being sent to the client.

Report, analysis and fix by Chi Gao <chi.gao@microfun.com>. Some minor
editorialisation by me.

Backpatched to XL9_5_STABLE
---

diff --git a/src/gtm/main/gtm_snap.c b/src/gtm/main/gtm_snap.c
index f4ab3a127b..5a538985df 100644
--- a/src/gtm/main/gtm_snap.c
+++ b/src/gtm/main/gtm_snap.c
@@ -23,7 +23,6 @@
 #include "gtm/libpq-int.h"
 #include "gtm/pqformat.h"
 
-static GTM_SnapshotData localSnapshot;
 /*
  * Get snapshot for the given transactions. If this is the first call in the
  * transaction, a fresh snapshot is taken and returned back. For a serializable
@@ -91,10 +90,16 @@ GTM_GetTransactionSnapshot(GTM_TransactionHandle handle[], int txn_count, int *s
 
 	/*
 	 * If no valid transaction exists in the array, we record the snapshot in a
-	 * local strucure and still send it out to the caller
+	 * thread-specific structure. This allows us to avoid repeated
+	 * allocation/freeing of the structure.
+	 *
+	 * Note that we must use a thread-specific variable and not a global
+	 * variable because a concurrent thread might compute a new snapshot and
+	 * overwrite the snapshot information while we are still sending this copy
+	 * to the client. Using a thread-specific storage avoids that problem.
 	 */
 	if (snapshot == NULL)
-		snapshot = &localSnapshot;
+		snapshot = &GetMyThreadInfo->thr_snapshot;
 
 	Assert(snapshot != NULL);
 
diff --git a/src/include/gtm/gtm.h b/src/include/gtm/gtm.h
index 8dfa77b139..37d31f6ba4 100644
--- a/src/include/gtm/gtm.h
+++ b/src/include/gtm/gtm.h
@@ -57,7 +57,8 @@ typedef struct GTM_ThreadInfo
 	uint32				thr_client_id;		/* unique client identifier */
 
 	GTM_RWLock			thr_lock;
-	gtm_List				*thr_cached_txninfo;
+	gtm_List			*thr_cached_txninfo;
+	GTM_SnapshotData    thr_snapshot;
 } GTM_ThreadInfo;
 
 typedef struct GTM_Threads