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-24T17:20:50Z
Lists: pgsql-hackers 
On Wed, Mar 23, 2022 at 6:28 PM Peter Geoghegan <pg@bowt.ie> wrote:
> It would be great if you could take a look v11-0002-*, Robert. Does it
> make sense to you?

You're probably not going to love hearing this, but I think you're
still explaining things here in ways that are too baroque and hard to
follow. I do think it's probably better. But, for example, in the
commit message for 0001, I think you could change the subject line to
"Allow non-aggressive vacuums to advance relfrozenxid" and it would be
clearer. And then I think you could eliminate about half of the first
paragraph, starting with "There is no fixed relationship", and all of
the third paragraph (which starts with "Later work..."), and I think
removing all that material would make it strictly more clear than it
is currently. I don't think it's the place of a commit message to
speculate too much on future directions or to wax eloquent on
theoretical points. If that belongs anywhere, it's in a mailing list
discussion.

It seems to me that 0002 mixes code movement with functional changes.
I'm completely on board with moving the code that decides how much to
skip into a function. That seems like a great idea, and probably
overdue. But it is not easy for me to see what has changed
functionally between the old and new code organization, and I bet it
would be possible to split this into two patches, one of which creates
a function, and the other of which fixes the problem, and I think that
would be a useful service to future readers of the code. I have a hard
time believing that if someone in the future bisects a problem back to
this commit, they're going to have an easy time finding the behavior
change in here. In fact I can't see it myself. I think the actual
functional change is to fix what is described in the second paragraph
of the commit message, but I haven't been able to figure out where the
logic is actually changing to address that. Note that I would be happy
with the behavior change happening either before or after the code
reorganization.

I also think that the commit message for 0002 is probably longer and
more complex than is really helpful, and that the subject line is too
vague, but since I don't yet understand exactly what's happening here,
I cannot comment on how I think it should be revised at this point,
except to say that the second paragraph of that commit message looks
like the most useful part.

I would also like to mention a few things that I do like about 0002.
One is that it seems to collapse two different pieces of logic for
page skipping into one. That seems good. As mentioned, it's especially
good because that logic is abstracted into a function. Also, it looks
like it is making a pretty localized change to one (1) aspect of what
VACUUM does -- and I definitely prefer patches that change only one
thing at a time.

Hope that's helpful.

-- 
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