On Wed, Nov 24, 2021 at 9:53 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > OK, this makes a lot more sense. I wasn't aware of ae7291ac (and I > wasn't aware of the significance of 8523492d either, but that's not > really relevant here.)
Thanks for hearing me out about the significance of 8523492d. Having the right formalisms seems to really matter here, because they enable decoupling, which is generally very useful. This makes it easy to understand (just for example) that index vacuuming and heap vacuuming are just additive, optional steps (in principle) -- an idea that will become even more important once we get Robert's pending TID conveyor belt design. I believe that that design goes one step further than what we have today, by making index vacuuming and heap vacuuming occur in a distinct operation to VACUUM proper (VACUUM would only need to set up the LP_DEAD item list for index vacuuming and heap vacuuming, which may or may not happen immediately after). An interesting question (at least to me) is: within a non-aggressive VACUUM, what remaining steps are *not* technically optional? I am pretty sure that they're all optional in principle (or will be soon), because soon we will be able to independently advance relfrozenxid without freezing all tuples with XIDs before our original FreezeLimit (FreezeLimit should only be used to decide which tuples to freeze, not to decide on a new relfrozenxid). Soon almost everything will be decoupled, without changing the basic invariants that we've had for many years. This flexibility seems really important to me. That just leaves avoiding pruning without necessarily avoiding ostensibly related processing for indexes. We can already independently prune without doing index/heap vacuuming (the bypass indexes optimization). We will also be able to do the opposite thing, with my new patch: we can perform index/heap vacuuming *without* pruning ourselves. This makes sense in the case where we cannot acquire a cleanup lock on a heap page with preexisting LP_DEAD items. > I think we could say "LP_DEAD line pointer" and that would be perfectly > clear. Given how nuanced we have to be if we want to be clear about > this, I would rather not use "LP_DEAD item"; that seems slightly > contradictory, since the item is the storage and such a line pointer > does not have storage. Perhaps change that define in progress.h to > PROGRESS_VACUUM_NUM_DEAD_LPS, and, in the first comment in vacuumlazy.c, > use wording such as I agree with all that, I think. But it's still not clear what the variable dead_tuples should be renamed to within the structure that you lay out (I imagine that you agree with me that dead_tuples is now a bad name). This one detail affects more individual lines of code than the restructuring of comments. -- Peter Geoghegan