Re: Wrong assert in TransactionGroupUpdateXidStatus

On 2019-Dec-12, Amit Kapila wrote:

> On Thu, Dec 12, 2019 at 6:10 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(at)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