Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations - Mailing list pgsql-hackers

From Peter Geoghegan
Subject Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
Date
Msg-id CAH2-WzmwM81qMxjYdm4-Z51PKwsgXXAr21bfBRncXCR7VB8W1g@mail.gmail.com
Whole thread Raw
In response to Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
List pgsql-hackers
On Tue, Mar 29, 2022 at 11:10 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> +                               diff = (int32) (vacrel->NewRelfrozenXid - vacrel->relfrozenxid);
> +                               Assert(diff > 0);
>
> Did you see that this crashed on windows cfbot?
>
> https://api.cirrus-ci.com/v1/artifact/task/4592929254670336/log/tmp_check/postmaster.log
> TRAP: FailedAssertion("diff > 0", File: "c:\cirrus\src\backend\access\heap\vacuumlazy.c", Line: 724, PID: 5984)

That's weird. There are very similar assertions a little earlier, that
must have *not* failed here, before the call to vac_update_relstats().
I was actually thinking of removing this assertion for that reason --
I thought that it was redundant.

Perhaps something is amiss inside vac_update_relstats(), where the
boolean flag that indicates that pg_class.relfrozenxid was advanced is
set:

    if (frozenxid_updated)
        *frozenxid_updated = false;
    if (TransactionIdIsNormal(frozenxid) &&
        pgcform->relfrozenxid != frozenxid &&
        (TransactionIdPrecedes(pgcform->relfrozenxid, frozenxid) ||
         TransactionIdPrecedes(ReadNextTransactionId(),
                               pgcform->relfrozenxid)))
    {
        if (frozenxid_updated)
            *frozenxid_updated = true;
        pgcform->relfrozenxid = frozenxid;
        dirty = true;
    }

Maybe the "existing relfrozenxid is in the future, silently update
relfrozenxid" part of the condition (which involves
ReadNextTransactionId()) somehow does the wrong thing here. But how?

The other assertions take into account the fact that OldestXmin can
itself "go backwards" across VACUUM operations against the same table:

    Assert(!aggressive || vacrel->NewRelfrozenXid == OldestXmin ||
           TransactionIdPrecedesOrEquals(FreezeLimit,
                                         vacrel->NewRelfrozenXid));

Note the "vacrel->NewRelfrozenXid == OldestXmin", without which the
assertion will fail pretty easily when the regression tests are run.
Perhaps I need to do something like that with the other assertion as
well (or more likely just get rid of it). Will figure it out tomorrow.

-- 
Peter Geoghegan



pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: Identify missing publications from publisher while create/alter subscription.
Next
From: Peter Eisentraut
Date:
Subject: Re: Add header support to text format and matching feature