Re: Wrong assert in TransactionGroupUpdateXidStatus - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Wrong assert in TransactionGroupUpdateXidStatus
Date
Msg-id 20191212124006.GA7980@alvherre.pgsql
Whole thread Raw
In response to Re: Wrong assert in TransactionGroupUpdateXidStatus  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Wrong assert in TransactionGroupUpdateXidStatus
List pgsql-hackers
On 2019-Dec-11, Amit Kapila wrote:

> On Wed, Dec 11, 2019 at 4:02 AM Andres Freund <andres@anarazel.de> wrote:
> > On 2019-12-10 13:55:40 +0530, Dilip Kumar wrote:

> >                 /*
> >                  * 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.

The more I look at both these asserts, the less sense they make.  Why
does clog.c care about PGPROC at all?  Looking at the callers of that
routine, nowhere do they concern themselves with whether the overflowed
flag has been set or not.  It seems to me that the StaticAssert() should
be near the PGPROC_MAX_CACHED_SUBXIDS definition, not the SUBTRANS
definition (maybe as StaticAssertDecl, as in
201DD0641B056142AC8C6645EC1B5F62014B8E8030@SYD1217 )

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Suraj Kharage
Date:
Subject: Re: backup manifests
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: Append with naive multiplexing of FDWs