Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-15 Thread Simon Riggs
On Thu, Dec 15, 2011 at 2:13 PM, Robert Haas wrote: > On Thu, Dec 15, 2011 at 8:19 AM, Simon Riggs wrote: >> Comments? > > I think there is a small bug here: > > +                       TransactionId xid = pgxact->xid; > + > +                       /* > +                        * Don't record loc

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-15 Thread Robert Haas
On Thu, Dec 15, 2011 at 8:19 AM, Simon Riggs wrote: > Comments? I think there is a small bug here: + TransactionId xid = pgxact->xid; + + /* +* Don't record locks for transactions if we know they have already +

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-15 Thread Simon Riggs
On Wed, Dec 14, 2011 at 3:20 PM, Tom Lane wrote: > Pavan Deolasee writes: >> Looking at CommitTransaction(), it seems quite clear to me that we >> call ProcArrayEndTransaction() before releasing the locks held by the >> transaction. So its quite possible that when >> GetRunningTransactionLocks go

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-14 Thread Simon Riggs
On Wed, Dec 14, 2011 at 3:20 PM, Tom Lane wrote: > Pavan Deolasee writes: >> Looking at CommitTransaction(), it seems quite clear to me that we >> call ProcArrayEndTransaction() before releasing the locks held by the >> transaction. So its quite possible that when >> GetRunningTransactionLocks go

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-14 Thread Tom Lane
Pavan Deolasee writes: > Looking at CommitTransaction(), it seems quite clear to me that we > call ProcArrayEndTransaction() before releasing the locks held by the > transaction. So its quite possible that when > GetRunningTransactionLocks goes through the list of currently held > locks, the pgxac

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-14 Thread Tom Lane
Pavan Deolasee writes: > BTW, on an unrelated note, I was looking at the way ShmemInitStruct() > is used. It internally uses ShmemAlloc to allocate from shared memory. > ShmemAlloc always MAXALIGN the requested size. But while calculating > the total shared memory requirement, we don't always MAXA

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-14 Thread Pavan Deolasee
On Wed, Dec 14, 2011 at 12:20 PM, Pavan Deolasee wrote: > On Wed, Dec 14, 2011 at 4:15 AM, Tom Lane wrote: > >> >> Without the added assert, you'd only notice this if you were running a >> standby slave --- the zero xid results in an assert failure in WAL >> replay on the slave end, which is how

Re: [HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-14 Thread Pavan Deolasee
On Wed, Dec 14, 2011 at 4:15 AM, Tom Lane wrote: > > Without the added assert, you'd only notice this if you were running a > standby slave --- the zero xid results in an assert failure in WAL > replay on the slave end, which is how I found out about this to start > with.  But since we've not hea

[HACKERS] Race condition in HEAD, possibly due to PGPROC splitup

2011-12-13 Thread Tom Lane
If you add this Assert to lock.c: diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 3ba4671..d9c15e0 100644 *** a/src/backend/storage/lmgr/lock.c --- b/src/backend/storage/lmgr/lock.c *** GetRunningTransactionLocks(int *nlocks) *** 3195,3200 ---