On Thu, Dec 17, 2015 at 3:15 AM, Mithun Cy <mithun...@enterprisedb.com>
wrote:
> I have rebased the patch and tried to run pgbench.
> I see memory corruptions, attaching the valgrind report for the same.
Sorry forgot to add re-based patch, adding the same now.
After some analysis I saw writing to shared memory to store shared snapshot
is not protected under exclusive write lock, this leads to memory
corruptions.
I think until this is fixed measuring the performance will not be much
useful.
--
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com
*** a/src/backend/commands/cluster.c
--- b/src/backend/commands/cluster.c
***************
*** 1559,1564 **** finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
--- 1559,1565 ----
elog(ERROR, "cache lookup failed for relation %u", OIDOldHeap);
relform = (Form_pg_class) GETSTRUCT(reltup);
+ Assert(TransactionIdIsNormal(frozenXid));
relform->relfrozenxid = frozenXid;
relform->relminmxid = cutoffMulti;
*** a/src/backend/storage/ipc/procarray.c
--- b/src/backend/storage/ipc/procarray.c
***************
*** 410,415 **** ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
--- 410,416 ----
if (LWLockConditionalAcquire(ProcArrayLock, LW_EXCLUSIVE))
{
ProcArrayEndTransactionInternal(proc, pgxact, latestXid);
+ ProcGlobal->cached_snapshot_valid = false;
LWLockRelease(ProcArrayLock);
}
else
***************
*** 558,563 **** ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
--- 559,566 ----
nextidx = pg_atomic_read_u32(&proc->nextClearXidElem);
}
+ ProcGlobal->cached_snapshot_valid = false;
+
/* We're done with the lock now. */
LWLockRelease(ProcArrayLock);
***************
*** 1543,1548 **** GetSnapshotData(Snapshot snapshot)
--- 1546,1553 ----
errmsg("out of memory")));
}
+ snapshot->takenDuringRecovery = RecoveryInProgress();
+
/*
* It is sufficient to get shared lock on ProcArrayLock, even if we are
* going to set MyPgXact->xmin.
***************
*** 1557,1565 **** GetSnapshotData(Snapshot snapshot)
/* initialize xmin calculation with xmax */
globalxmin = xmin = xmax;
! snapshot->takenDuringRecovery = RecoveryInProgress();
! if (!snapshot->takenDuringRecovery)
{
int *pgprocnos = arrayP->pgprocnos;
int numProcs;
--- 1562,1592 ----
/* initialize xmin calculation with xmax */
globalxmin = xmin = xmax;
! if (!snapshot->takenDuringRecovery && ProcGlobal->cached_snapshot_valid)
! {
! Snapshot csnap = &ProcGlobal->cached_snapshot;
! TransactionId *saved_xip;
! TransactionId *saved_subxip;
! saved_xip = snapshot->xip;
! saved_subxip = snapshot->subxip;
!
! memcpy(snapshot, csnap, sizeof(SnapshotData));
!
! snapshot->xip = saved_xip;
! snapshot->subxip = saved_subxip;
!
! memcpy(snapshot->xip, csnap->xip,
! sizeof(TransactionId) * csnap->xcnt);
! memcpy(snapshot->subxip, csnap->subxip,
! sizeof(TransactionId) * csnap->subxcnt);
! globalxmin = ProcGlobal->cached_snapshot_globalxmin;
! xmin = csnap->xmin;
!
! Assert(TransactionIdIsValid(globalxmin));
! Assert(TransactionIdIsValid(xmin));
! }
! else if (!snapshot->takenDuringRecovery)
{
int *pgprocnos = arrayP->pgprocnos;
int numProcs;
***************
*** 1577,1591 **** GetSnapshotData(Snapshot snapshot)
TransactionId xid;
/*
! * Backend is doing logical decoding which manages xmin
! * separately, check below.
*/
! if (pgxact->vacuumFlags & PROC_IN_LOGICAL_DECODING)
! continue;
!
! /* Ignore procs running LAZY VACUUM */
! if (pgxact->vacuumFlags & PROC_IN_VACUUM)
! continue;
/* Update globalxmin to be the smallest valid xmin */
xid = pgxact->xmin; /* fetch just once */
--- 1604,1615 ----
TransactionId xid;
/*
! * Ignore procs running LAZY VACUUM (which don't need to retain
! * rows) and backends doing logical decoding (which manages xmin
! * separately, check below).
*/
! if (pgxact->vacuumFlags & (PROC_IN_LOGICAL_DECODING | PROC_IN_VACUUM))
! continue;
/* Update globalxmin to be the smallest valid xmin */
xid = pgxact->xmin; /* fetch just once */
***************
*** 1653,1658 **** GetSnapshotData(Snapshot snapshot)
--- 1677,1706 ----
}
}
}
+
+ /* upate cache */
+ {
+ Snapshot csnap = &ProcGlobal->cached_snapshot;
+ TransactionId *saved_xip;
+ TransactionId *saved_subxip;
+
+ ProcGlobal->cached_snapshot_globalxmin = globalxmin;
+
+ saved_xip = csnap->xip;
+ saved_subxip = csnap->subxip;
+ memcpy(csnap, snapshot, sizeof(SnapshotData));
+ csnap->xip = saved_xip;
+ csnap->subxip = saved_subxip;
+
+ /* not yet stored. Yuck */
+ csnap->xmin = xmin;
+
+ memcpy(csnap->xip, snapshot->xip,
+ sizeof(TransactionId) * csnap->xcnt);
+ memcpy(csnap->subxip, snapshot->subxip,
+ sizeof(TransactionId) * csnap->subxcnt);
+ ProcGlobal->cached_snapshot_valid = true;
+ }
}
else
{
*** a/src/backend/storage/lmgr/proc.c
--- b/src/backend/storage/lmgr/proc.c
***************
*** 114,119 **** ProcGlobalShmemSize(void)
--- 114,126 ----
size = add_size(size, mul_size(NUM_AUXILIARY_PROCS, sizeof(PGXACT)));
size = add_size(size, mul_size(max_prepared_xacts, sizeof(PGXACT)));
+ /* for the cached snapshot */
+ #define PROCARRAY_MAXPROCS (MaxBackends + max_prepared_xacts)
+ size = add_size(size, mul_size(sizeof(TransactionId), PROCARRAY_MAXPROCS));
+ #define TOTAL_MAX_CACHED_SUBXIDS \
+ ((PGPROC_MAX_CACHED_SUBXIDS + 1) * PROCARRAY_MAXPROCS)
+ size = add_size(size, mul_size(sizeof(TransactionId), TOTAL_MAX_CACHED_SUBXIDS));
+
return size;
}
***************
*** 275,280 **** InitProcGlobal(void)
--- 282,293 ----
/* Create ProcStructLock spinlock, too */
ProcStructLock = (slock_t *) ShmemAlloc(sizeof(slock_t));
SpinLockInit(ProcStructLock);
+
+ /* cached snapshot */
+ ProcGlobal->cached_snapshot_valid = false;
+ ProcGlobal->cached_snapshot.xip = ShmemAlloc(PROCARRAY_MAXPROCS * sizeof(TransactionId));
+ ProcGlobal->cached_snapshot.subxip = ShmemAlloc(TOTAL_MAX_CACHED_SUBXIDS * sizeof(TransactionId));
+ ProcGlobal->cached_snapshot_globalxmin = InvalidTransactionId;
}
/*
*** a/src/include/storage/proc.h
--- b/src/include/storage/proc.h
***************
*** 16,21 ****
--- 16,22 ----
#include "access/xlogdefs.h"
#include "lib/ilist.h"
+ #include "utils/snapshot.h"
#include "storage/latch.h"
#include "storage/lock.h"
#include "storage/pg_sema.h"
***************
*** 220,225 **** typedef struct PROC_HDR
--- 221,231 ----
int startupProcPid;
/* Buffer id of the buffer that Startup process waits for pin on, or -1 */
int startupBufferPinWaitBufId;
+
+ /* Cached snapshot */
+ bool cached_snapshot_valid;
+ SnapshotData cached_snapshot;
+ TransactionId cached_snapshot_globalxmin;
} PROC_HDR;
extern PROC_HDR *ProcGlobal;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
