Horton

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

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: 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-29T17:03:32Z
Lists: pgsql-hackers 
On Sun, Mar 27, 2022 at 11:24 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v12. My current goal is to commit all 3 patches before
> feature freeze. Note that this does not include the more complicated
> patch including with previous revisions of the patch series (the
> page-level freezing work that appeared in versions before v11).

Reviewing 0001, focusing on the words in the patch file much more than the code:

I can understand this version of the commit message. Woohoo! I like
understanding things.

I think the header comments for FreezeMultiXactId() focus way too much
on what the caller is supposed to do and not nearly enough on what
FreezeMultiXactId() itself does. I think to some extent this also
applies to the comments within the function body.

On the other hand, the header comments for heap_prepare_freeze_tuple()
seem good to me. If I were thinking of calling this function, I would
know how to use the new arguments. If I were looking for bugs in it, I
could compare the logic in the function to what these comments say it
should be doing. Yay.

I think I understand what the first paragraph of the header comment
for heap_tuple_needs_freeze() is trying to say, but the second one is
quite confusing. I think this is again because it veers into talking
about what the caller should do rather than explaining what the
function itself does.

I don't like the statement-free else block in lazy_scan_noprune(). I
think you could delete the else{} and just put that same comment there
with one less level of indentation. There's a clear "return false"
just above so it shouldn't be confusing what's happening.

The comment hunk at the end of lazy_scan_noprune() would probably be
better if it said something more specific than "caller can tolerate
reduced processing." My guess is that it would be something like
"caller does not need to do something or other."

I have my doubts about whether the overwrite-a-future-relfrozenxid
behavior is any good, but that's a topic for another day. I suggest
keeping the words "it seems best to", though, because they convey a
level of tentativeness, which seems appropriate.

I am surprised to see you write in maintenance.sgml that the VACUUM
which most recently advanced relfrozenxid will typically be the most
recent aggressive VACUUM. I would have expected something like "(often
the most recent VACUUM)".

-- 
Robert Haas
EDB: http://www.enterprisedb.com

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