On Wed, Dec 11, 2019 at 11:00 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Dec 11, 2019 at 4:02 AM Andres Freund <and...@anarazel.de> wrote: > > On 2019-12-10 13:55:40 +0530, Dilip Kumar wrote: > > > > > /* > > > * Overflowed transactions should not use group XID status update > > > * mechanism. > > > */ > > > Assert(!pgxact->overflowed); > > > > > > A solution could be either we remove this assert or change this assert > > > to Assert(pgxact->nxids <= THRESHOLD_SUBTRANS_CLOG_OPT); > > > > Maybe I'm missing something, but isn't this a bug then? IIRC We can't > > rely on MyProc->subxids once we overflowed, even if since then the > > remaining number of children has become low enough. > > > > AFAICS, the MyProc->subxids is maintained properly if the number of > subtransactions is lesser than PGPROC_MAX_CACHED_SUBXIDS (64). Can > you explain the case where that won't be true? Also, even if what you > are saying is true, I think the memcmp in TransactionIdSetPageStatus > should avoid taking us a wrong decision. >
I am able to reproduce the issue by reducing the values of PGPROC_MAX_CACHED_SUBXIDS and THRESHOLD_SUBTRANS_CLOG_OPT to 2. Below is what I did after reducing the values: Session-1 -------------- postgres=# begin; BEGIN postgres=# insert into t1 values(1); INSERT 0 1 postgres=# savepoint s1; SAVEPOINT postgres=# insert into t1 values(2); INSERT 0 1 postgres=# savepoint s2; SAVEPOINT postgres=# insert into t1 values(3); INSERT 0 1 postgres=# savepoint s3; SAVEPOINT postgres=# insert into t1 values(4); INSERT 0 1 postgres=# rollback to s2; ROLLBACK Session-2 --------------- insert into t1 values(4); -- attach debugger and stop in TransactionIdSetPageStatus after acquiring CLogControlLock Session-1 --------------- Commit; -- This will wait to acquire CLogControlLock in a group update path (TransactionGroupUpdateXidStatus). Now, continue in the session-2 debugger. After that continue in session-1's debugger and it will hit the Assert. The attached patch fixes it by changing the Assert. I have additionally removed the redundant Assert in TransactionIdSetPageStatus as pointed out by Andres. I am planning to commit and backpatch this early next week (Monday) unless someone wants to review it further or has objections to it. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
v2-0001-Change-overly-strict-Assert-in-TransactionGroupUpdat.patch
Description: Binary data
