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. > > Also, it's somewhat odd that TransactionIdSetPageStatus() first has > > /* Can't use group update when PGPROC overflows. */ > StaticAssertStmt(THRESHOLD_SUBTRANS_CLOG_OPT <= > PGPROC_MAX_CACHED_SUBXIDS, > "group clog threshold less than > PGPROC cached subxids"); > > and then, within an if(): > > /* > * We don't try to do group update optimization if a process > has > * overflowed the subxids array in its PGPROC, since in that > case we > * don't have a complete list of XIDs for it. > */ > Assert(THRESHOLD_SUBTRANS_CLOG_OPT <= > PGPROC_MAX_CACHED_SUBXIDS); > > Even if these weren't redundant, it can't make sense to test such a > static condition only within an if? > I don't remember exactly the reason for this, but now I don't find the Assert within if () meaningful. I think we should remove the Assert inside if() unless Robert or someone see any use of it. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
