Allocate thread-local snapshot array statically
authorTomas Vondra <[email protected]>
Sat, 14 Oct 2017 10:38:06 +0000 (12:38 +0200)
committerTomas Vondra <[email protected]>
Sat, 14 Oct 2017 10:52:04 +0000 (12:52 +0200)
Since commit fb56418d66 the snapshots are computed in thread-local
storage, but we haven't been freeing the memory (on thread exit).
As the memory is allocated in the global (TopMostMemoryContext),
this presented a memory leak of 64kB on each GTM connection.

One way to fix this would be to track when the thread-local storage
is used in GTM_GetTransactionSnapshot(), and allocate the memory
in TopMemoryContext (which is per-thread and released on exit).

But there's a simpler way - allocate the thread-specific storage as
part of GTM_ThreadInfo, and just redirect sn_xip from the snapshot.
This way we don't have to worry about palloc/pfree at all, and we
mostly assume that every connection will need to compute at least
one snapshot anyway.

Reported by Rob Canavan <[email protected]>, investigation and fix
by me. For more discussion see
<CAFTg0q6VC_11+c=Q=gsAcDsBrDjvuGKjzNwH4Lr8vERRDn4Ycw@mail.gmail.com>

Backpatch to Postgres-XL 9.5.

src/gtm/main/gtm_snap.c
src/gtm/main/gtm_thread.c
src/include/gtm/gtm.h

index 5a538985dfb481917ce9dcc98ca1587b95087e19..f8e3d3197624158786fc88110ac533454df9335f 100644 (file)
@@ -103,6 +103,10 @@ GTM_GetTransactionSnapshot(GTM_TransactionHandle handle[], int txn_count, int *s
 
        Assert(snapshot != NULL);
 
+       /*
+        * This can only happen when using a snapshot from GTMTransactions, as the
+        * thread-specific sn_xip array is allocated statically as part of GTM_ThreadInfo.
+        */
        if (snapshot->sn_xip == NULL)
        {
                /*
index c513f6a71f3ab05d4d4b017ae6c2fec394dacb83..9d166f4a0eb2ce1e17dd5dc135d98177fd261433 100644 (file)
@@ -256,6 +256,11 @@ GTM_ThreadCreate(GTM_ConnectionInfo *conninfo,
                                                                                                           8 * 1024,
                                                                                                           false);
 
+       /*
+        * Set the sn_xip pointer to the statically-allocated array (see GTM_ThreadInfo).
+        */
+       thrinfo->thr_snapshot.sn_xip = thrinfo->thr_xip;
+
        thrinfo->thr_startroutine = startroutine;
 
        /*
index 37d31f6ba41b24c4ca54b3732370c1f657748c42..7b494fb6a25528a4e4c3ee955e594ee27c05fd92 100644 (file)
@@ -59,6 +59,13 @@ typedef struct GTM_ThreadInfo
        GTM_RWLock                      thr_lock;
        gtm_List                        *thr_cached_txninfo;
        GTM_SnapshotData    thr_snapshot;
+
+       /*
+        * Statically allocated XID array for the snapshot. Every thread will need
+        * a snapshot anyway, and this way we don't have to worry about allocation
+        * and freeing of the memory at thread exit.
+        */
+       GlobalTransactionId     thr_xip[GTM_MAX_GLOBAL_TRANSACTIONS];
 } GTM_ThreadInfo;
 
 typedef struct GTM_Threads