Re: Wrong assert in TransactionGroupUpdateXidStatus

On 2019-Dec-12, Amit Kapila wrote:

> On Thu, Dec 12, 2019 at 6:10 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

> > The more I look at both these asserts, the less sense they make.  Why
> > does clog.c care about PGPROC at all?
> 
> It is mainly for group updates.  Basically, we want to piggyback the
> procs that are trying to update clog at the same time on the proc
> which got the CLogControlLock. This avoids taking/releasing that lock
> multiple times.  See TransactionGroupUpdateXidStatus.

Yeah, I (think I) understand that.  My point is that conceptually, the
fact that a PGPROC has overflowed does not really affect clog.c in any
way.

> > 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 )
> 
> Sounds reasonable.  We can do that once the patch mentioned by you got
> committed.  For now, we are planning to just remove the Assert inside
> if() condition. Do you see any problem with that?

Nope.

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