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-30T07:01:06Z
Lists: 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

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