Re: Wrong assert in TransactionGroupUpdateXidStatus

On 2019-Dec-11, Amit Kapila wrote:

> On Wed, Dec 11, 2019 at 4:02 AM Andres Freund <andres(at)anarazel(dot)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(at)SYD1217 )

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