Horton

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

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: pgsql-hackers@lists.postgresql.org, Peter Geoghegan <pg@bowt.ie>
Cc: Robert Haas <robertmhaas@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-02-20T00:12:31Z
Lists: pgsql-hackers 
Hi,

(On phone, so crappy formatting and no source access)

On February 19, 2022 3:08:41 PM PST, Peter Geoghegan <pg@bowt.ie> wrote:
>On Fri, Feb 18, 2022 at 5:00 PM Peter Geoghegan <pg@bowt.ie> wrote:
>> Another testing strategy occurs to me: we could stress-test the
>> implementation by simulating an environment where the no-cleanup-lock
>> path is hit an unusually large number of times, possibly a fixed
>> percentage of the time (like 1%, 5%), say by making vacuumlazy.c's
>> ConditionalLockBufferForCleanup() call return false randomly. Now that
>> we have lazy_scan_noprune for the no-cleanup-lock path (which is as
>> similar to the regular lazy_scan_prune path as possible), I wouldn't
>> expect this ConditionalLockBufferForCleanup() testing gizmo to be too
>> disruptive.
>
>I tried this out, using the attached patch. It was quite interesting,
>even when run against HEAD. I think that I might have found a bug on
>HEAD, though I'm not really sure.
>
>If you modify the patch to simulate conditions under which
>ConditionalLockBufferForCleanup() fails about 2% of the time, you get
>much better coverage of lazy_scan_noprune/heap_tuple_needs_freeze,
>without it being so aggressive as to make "make check-world" fail --
>which is exactly what I expected. If you are much more aggressive
>about it, and make it 50% instead (which you can get just by using the
>patch as written), then some tests will fail, mostly for reasons that
>aren't surprising or interesting (e.g. plan changes). This is also
>what I'd have guessed would happen.
>
>However, it gets more interesting. One thing that I did not expect to
>happen at all also happened (with the current 50% rate of simulated
>ConditionalLockBufferForCleanup() failure from the patch): if I run
>"make check" from the pg_surgery directory, then the Postgres backend
>gets stuck in an infinite loop inside lazy_scan_prune, which has been
>a symptom of several tricky bugs in the past year (not every time, but
>usually). Specifically, the VACUUM statement launched by the SQL
>command "vacuum freeze htab2;" from the file
>contrib/pg_surgery/sql/heap_surgery.sql, at line 54 leads to this
>misbehavior.


>This is a temp table, which is a choice made by the tests specifically
>because they need to "use a temp table so that vacuum behavior doesn't
>depend on global xmin". This is convenient way of avoiding spurious
>regression tests failures (e.g. from autoanalyze), and relies on the
>GlobalVisTempRels behavior established by Andres' 2020 bugfix commit
>94bc27b5.

We don't have a blocking path for cleanup locks of temporary buffers IIRC (normally not reachable). So I wouldn't be surprised if a cleanup lock failing would cause some odd behavior.

>It's quite possible that this is nothing more than a bug in my
>adversarial gizmo patch -- since I don't think that
>ConditionalLockBufferForCleanup() can ever fail with a temp buffer
>(though even that's not completely clear right now). Even if the
>behavior that I saw does not indicate a bug on HEAD, it still seems
>informative. At the very least, it wouldn't hurt to Assert() that the
>target table isn't a temp table inside lazy_scan_noprune, documenting
>our assumptions around temp tables and
>ConditionalLockBufferForCleanup().

Definitely worth looking into more.


This reminds me of a recent thing I noticed in the aio patch. Spgist can end up busy looping when buffers are locked, instead of blocking. Not actually related, of course.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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