чт, 12 нояб. 2020 г. в 23:00, Peter Geoghegan <pg@bowt.ie>:
It would be helpful if you could take a look at the nbtree patch -- particularly the changes related to deprecating the page-level BTP_HAS_GARBAGE flag. I would like to break those parts out into a separate patch, and commit it in the next week or two. It's just refactoring, really. (This commit can also make nbtree only use the word VACUUM for things that strictly involve VACUUM. For example, it'll rename _bt_vacuum_one_page() to _bt_delete_or_dedup_one_page().)
We almost don't care about the flag already, so there is almost no behavioral change from deprecated BTP_HAS_GARBAGE in this way.
Indexes that use deduplication already don't rely on BTP_HAS_GARBAGE being set ever since deduplication was added to Postgres 13 (the deduplication code doesn't want to deal with LP_DEAD bits, and cannot trust that no LP_DEAD bits can be set just because BTP_HAS_GARBAGE isn't set in the special area). Trusting the BTP_HAS_GARBAGE flag can cause us to miss out on deleting items with their LP_DEAD bits set -- we're better off "assuming that BTP_HAS_GARBAGE is always set", and finding out if there really are LP_DEAD bits set for ourselves each time.
Missing LP_DEAD bits like this can happen when VACUUM unsets the page-level flag without actually deleting the items at the same time, which is expected when the items became garbage (and actually had their LP_DEAD bits set) after VACUUM began, but before VACUUM reached the leaf pages. That's really wasteful, and doesn't actually have any upside -- we're scanning all of the line pointers only when we're about to split (or dedup) the same page anyway, so the extra work is practically free.
I've looked over the BTP_HAS_GARBAGE modifications, they look sane. I've double checked that heapkeyspace indexes don't use this flag (don't rely on it), while pre-v4 ones still use it.
I have a question. This flag is raised in the _bt_check_unique() and in _bt_killitems(). If we're deprecating this flag, perhaps it'd be good to either avoid raising it at least for
_bt_check_unique(), as it seems to me that conditions are dealing with postings, therefore
we are speaking of heapkeyspace indexes here.
If we'll conditionally raise this flag in the functions above, we can get rid of blocks that drop it
