Re: BUG #17116: Assert failed in SerialSetActiveSerXmin() on commit of parallelized serializable transaction

On Sat, Jul 24, 2021 at 12:56 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> On Thu, Jul 22, 2021 at 11:23 PM PG Bug reporting form
> <noreply(at)postgresql(dot)org> wrote: '.
> > #2 0x0000555e1740b1ac in ExceptionalCondition (
> > conditionName=conditionName(at)entry=0x555e17581ce8
> > "!TransactionIdIsValid(serialControl->tailXid) || TransactionIdFollows(xid,
> > serialControl->tailXid)", errorType=errorType(at)entry=0x555e17466028
> > "FailedAssertion",
> > fileName=fileName(at)entry=0x555e17581789 "predicate.c",
> > lineNumber=lineNumber(at)entry=1056) at assert.c:67
>
> Thanks. Repro'd here. Not immediately sure what's happening here,
> but I will look into it next week.

Yeah, I goofed commit 47a338cf. It could double-release a sort of
reference count. I think the reason I didn't notice and nothing bad
happened is that the global xmin tracking is effectively
self-correcting in this rare case. Once the count reaches zero, we
recompute the count by linear search for the new oldest xmin
(something on my list of things to try to improve, but in this case
the saving grace).

Here's a draft patch that includes your test spec. Without the
included change to predicate.c, it hits the assertion. With it, it
doesn't, and pg_locks shows no leaked locks after the isolation checks
run (that was the problem 47a338cf fixed before 12 was released, but
it fixed it a little too far in the opposite direction, fortunately
without ill effect AFAICS besides wasted CPU cycles).

I'll see if I can think of some more ways to test this logic, and some
way to make the coding a little clearer.