Horton

Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Andres Freund <andres@anarazel.de>, Masahiko Sawada <sawada.mshk@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-31T00:50:42Z
Lists: pgsql-hackers 
On Wed, Mar 30, 2022 at 12:01 AM Peter Geoghegan <pg@bowt.ie> wrote:
> 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?

I tried several times to recreate this issue on CI. No luck with that,
though -- can't get it to fail again after 4 attempts.

This was a VACUUM of pg_database, run from an autovacuum worker. I am
vaguely reminded of the two bugs fixed by Andres in commit a54e1f15.
Both were issues with the shared relcache init file affecting shared
and nailed catalog relations. Those bugs had symptoms like " ERROR:
found xmin ... from before relfrozenxid ..." for various system
catalogs.

We know that this particular assertion did not fail during the same VACUUM:

    Assert(vacrel->NewRelfrozenXid == OldestXmin ||
           TransactionIdPrecedesOrEquals(vacrel->relfrozenxid,
                                         vacrel->NewRelfrozenXid));

So it's hard to see how this could be a bug in the patch -- the final
new relfrozenxid is presumably equal to VACUUM's OldestXmin in the
problem scenario seen on the CI Windows instance yesterday (that's why
this earlier assertion didn't fail).  The assertion I'm showing here
needs the "vacrel->NewRelfrozenXid == OldestXmin" part of the
condition to account for the fact that
OldestXmin/GetOldestNonRemovableTransactionId() is known to "go
backwards". Without that the regression tests will fail quite easily.

The surprising part of the CI failure must have taken place just after
this assertion, when VACUUM's call to vacuum_set_xid_limits() actually
updates pg_class.relfrozenxid with vacrel->NewRelfrozenXid --
presumably because the existing relfrozenxid appeared to be "in the
future" when we examine it in pg_class again. We see evidence that
this must have happened afterwards, when the closely related assertion
(used only in instrumentation code) fails:

From my patch:

>             if (frozenxid_updated)
>             {
> -               diff = (int32) (FreezeLimit - vacrel->relfrozenxid);
> +               diff = (int32) (vacrel->NewRelfrozenXid - vacrel->relfrozenxid);
> +               Assert(diff > 0);
>                 appendStringInfo(&buf,
>                                  _("new relfrozenxid: %u, which is %d xids ahead of previous value\n"),
> -                                FreezeLimit, diff);
> +                                vacrel->NewRelfrozenXid, diff);
>             }

Does anybody have any ideas about what might be going on here?

-- 
Peter Geoghegan

Commits

  1. Have VACUUM warn on relfrozenxid "in the future".

  2. vacuumlazy.c: Further consolidate resource allocation.

  3. Generalize how VACUUM skips all-frozen pages.

  4. Set relfrozenxid to oldest extant XID seen by VACUUM.

  5. Doc: Add relfrozenxid Tip to XID wraparound section.

  6. vacuumlazy.c: document vistest and OldestXmin.

  7. Increase hash_mem_multiplier default to 2.0.

  8. Consolidate VACUUM xid cutoff logic.

  9. Add VACUUM instrumentation for scanned pages, relfrozenxid.

  10. Simplify lazy_scan_heap's handling of scanned pages.

  11. Try to stabilize reloptions test, again.

  12. Unify VACUUM VERBOSE and autovacuum logging.

  13. Fix possible HOT corruption when RECENTLY_DEAD changes to DEAD while pruning.

  14. pg_resetxlog: add option to set oldest xid & use by pg_upgrade

  15. Teach VACUUM to bypass unnecessary index vacuuming.

  16. Centralize horizon determination for temp tables, fixing bug due to skew.

  17. pg_surgery: Try to stabilize regression tests.

  18. Add "split after new tuple" nbtree optimization.

  19. Fix bugs in vacuum of shared rels, by keeping their relcache entries current.

  20. Avoid useless truncation attempts during VACUUM.

  21. Only skip pages marked as clean in the visibility map, if the last 32

  22. Fix recently-understood problems with handling of XID freezing, particularly