Re: Overhauling "Routine Vacuuming" docs, particularly its handling of freezing

Hi Samay,

On Tue, May 2, 2023 at 11:40 PM samay sharma <smilingsamay(at)gmail(dot)com> wrote:
> Thanks for taking the time to do this. It is indeed difficult work.

Thanks for the review! I think that this is something that would
definitely benefit from a perspective such as yours.

> There are things I like about the changes you've proposed and some where I feel that the previous section was easier to understand.

That makes sense, and I think that I agree with every point you've
raised, bar none. I'm pleased to see that you basically agree with the
high level direction.

I would estimate that the version you looked at (v2) is perhaps 35%
complete. So some of the individual problems you noticed were a direct
consequence of the work just not being anywhere near complete. I'll
try to do a better job of tracking the relative maturity of each
commit/patch in each commit message, going forward.

Anything that falls under "25.2.1. Recovering Disk Space" is
particularly undeveloped in v2. The way that I broke that up into a
bunch of WARNINGs/NOTEs/TIPs was just a short term way of breaking it
up into pieces, so that the structure was very approximately what I
wanted. I actually think that the stuff about CLUSTER and VACUUM FULL
belongs in a completely different chapter. Since it is not "Routine
Vacuuming" at all.

>> 2. Renamed "Preventing Transaction ID Wraparound Failures" to
>> "Freezing to manage the transaction ID space". Now we talk about
>> wraparound as a subtopic of freezing, not vice-versa. (This is a
>> complete rewrite, as described by later items in this list).
>
> +1 on this too. Freezing is a normal part of vacuuming and while the aggressive vacuums are different, I think just talking about the worst case scenario while referring to it is alarmist.

Strangely enough, Postgres 16 is the first version that instruments
freezing in its autovacuum log reports. I suspect that some long term
users will find it quite surprising to see how much (or how little)
freezing takes place in non-aggressive VACUUMs.

The introduction of page-level freezing will make it easier and more
natural to tune settings like vacuum_freeze_min_age, with the aim of
smoothing out the burden of freezing over time (particularly by making
non-aggressive VACUUMs freeze more). Page-level freezing removes any
question of not freezing every tuple on a page (barring cases where
"removable cutoff" is noticeably held back by an old MVCC snapshot).
This makes it more natural to think of freezing as a process that
makes it okay to store data in individual physical heap pages, long
term.

> 1) While I agree that bundling VACUUM and VACUUM FULL is not the right way, moving all VACUUM FULL references into tips and warnings also seems excessive. I think it's probably best to just have a single paragraph which talks about VACUUM FULL as I do think it should be mentioned in the reclaiming disk space section.

As I mentioned briefly already, my intention is to move it to another
chapter entirely. I was thinking of "Chapter 29. Monitoring Disk
Usage". The "Routine Vacuuming" docs would then link to this sect1 --
something along the lines of "non-routine commands to reclaim a lot of
disk space in the event of extreme bloat".

> 2) I felt that the new section, "Freezing to manage the transaction ID space" could be made simpler to understand. As an example, I understood what the parameters (autovacuum_freeze_max_age, vacuum_freeze_table_age) do and how they interact better in the previous version of the docs.

Agreed. I'm going to split it up some more. I think that the current
"25.2.2.1. VACUUM's Aggressive Strategy" should be split in two, so we
go from talking about aggressive VACUUMs to Antiwraparound
autovacuums. Finding the least confusing way of explaining it has been
a focus of mine in the last few days.

> 4) I think we should explicitly call out that seeing an anti-wraparound VACUUM or "VACUUM table (to prevent wraparound)" is normal and that it's just a VACUUM triggered due to the table having unfrozen rows with an XID older than autovacuum_freeze_max_age. I've seen many users panicking on seeing this and feeling that they are close to a wraparound.

That has also been my exact experience. Users are terrified, usually
for no good reason at all. I'll make sure that this comes across in
the next revision of the patch series.

> Also, we should be more clear about how it's different from VACUUMs triggered due to the scale factors (cancellation behavior, being triggered when autovacuum is disabled etc.).

Right. Though I think that the biggest point of confusion for users is
how *few* differences there really are between antiwraparound
autovacuum, and any other kind of autovacuum that happens to use
VACUUM's aggressive strategy. There is really only one important
difference: the autocancellation behavior. This is an autovacuum
behavior, not a VACUUM behavior -- so the "VACUUM side" doesn't know
anything about that at all.

> 5) Can we use a better name for the XidStopLimit mode? It seems like a very implementation centric name. Maybe a better version of "Running out of the XID space" or something like that?

Coming up with a new user-facing name for xidStopLimit is already on
my TODO list (it's surprisingly hard). I have used that name so far
because it unambiguously refers to the exact thing that I want to talk
about when discussing the worst case. Other than that, it's a terrible
name.

> 6) In the XidStopLimit mode section, it would be good to explain briefly why you could get to this scenario. It's not something which should happen in a normal running system unless you have a long running transaction or inactive replication slots or a badly configured system or something of that sort.

I agree that that's important. Note that there is already something
about "removable cutoff" being held back at the start of the
discussion of freezing -- that will prevent freezing in exactly the
same way as it prevents cleanup of dead tuples.

That will become a WARNING box in the next revision. There should also
be a similar, analogous WARNING box (about "removable cutoff" being
held back) much earlier on in the docs -- this should appear in
"25.2.1. Recovering Disk Space". Obviously this structure suggests
that there is an isomorphism between freezing and removing bloat. For
example, if you cannot "freeze" an XID that appears in some tuple's
xmax, then you also cannot remove that tuple because VACUUM only sees
it as a recently dead tuple (if xmax is >= OldestXmin/removable
cutoff, and from a deleter that already committed).

I don't think that we need to spell the "isomorphism" point out to the
reader directly, but having a subtle cue that that's how it works
seems like a good idea.

> If you got to this point, other than running VACUUM to get out of the situation, it's also important to figure out what got you there in the first place as many VACUUMs should have attempted to advance the relfrozenxid and failed.

It's also true that problems that can lead to the system entering
xidStopLimit mode aren't limited to cases where doing required
freezing is fundamentally impossible due to something holding back
"removable cutoff". It's also possible that VACUUM simply can't keep
up (though the failsafe has helped with that problem a lot).

I tend to agree that there needs to be more about this in the
xidStopLimit subsection (discussion of freezing being held back by
"removable cutoff" is insufficient), but FWIW that seems like it
should probably be treated as out of scope for this patch. It is more
the responsibility of the other patch [1] that aims to put the
xidStopLimit documentation on a better footing (and remove that
terrible HINT about single user mode).

Of course, that other patch is closely related to this patch -- the
precise boundaries are unclear at this point. In any case I think that
this should happen, because I think that it's a good idea.

> There are a few other small things I noticed along the way but my goal was to look at the overall structure.

Thanks again! This is very helpful.

[1] https://www.postgresql.org/message-id/flat/CAJ7c6TM2D277U2wH8X78kg8pH3tdUqebV3_JCJqAkYQFHCFzeg%40mail.gmail.com
--
Peter Geoghegan