Commit 4b25f5d0 authored by Peter Geoghegan's avatar Peter Geoghegan

Revise BTP_HAS_GARBAGE nbtree VACUUM comments.

_bt_delitems_vacuum() comments claimed that it isn't worth another scan
of the page to avoid falsely unsetting the BTP_HAS_GARBAGE page flag
hint (this happens to be the same wording that was removed from
_bt_delitems_delete() by my recent commit fe97c61c).  The comments made
little sense, though.  The issue can't have much to do with performing a
second scan of the target leaf page, since an LP_DEAD test could easily
be performed in the first scan of the page anyway (the scan that takes
place in btvacuumpage() caller).

Revise the explanation.  It makes much more sense to frame this as an
issue about recovery conflicts.  _bt_delitems_vacuum() cannot easily
generate an XID cutoff in the same way that _bt_delitems_delete() is
designed to.

Falsely unsetting the page flag is not ideal, and is likely to happen
more often than was supposed by the original comments.  Explain why it
usually isn't a problem in practice.  There may be an argument for
_bt_delitems_vacuum() not clearing the BTP_HAS_GARBAGE bit, removing the
question of it being falsely unset by VACUUM (there may even be an
argument for not using a page level hint at all).  This can be revisited
later.
parent 823e739d
......@@ -1001,9 +1001,16 @@ _bt_delitems_vacuum(Relation rel, Buffer buf,
/*
* Mark the page as not containing any LP_DEAD items. This is not
* certainly true (there might be some that have recently been marked, but
* weren't included in our target-item list), but it will almost always be
* true and it doesn't seem worth an additional page scan to check it.
* Remember that BTP_HAS_GARBAGE is only a hint anyway.
* weren't targeted by VACUUM's heap scan), but it will be true often
* enough. VACUUM does not delete items purely because they have their
* LP_DEAD bit set, since doing so would necessitate explicitly logging a
* latestRemovedXid cutoff (this is how _bt_delitems_delete works).
*
* The consequences of falsely unsetting BTP_HAS_GARBAGE should be fairly
* limited, since we never falsely unset an LP_DEAD bit. Workloads that
* are particularly dependent on LP_DEAD bits being set quickly will
* usually manage to set the BTP_HAS_GARBAGE flag before the page fills up
* again anyway.
*/
opaque->btpo_flags &= ~BTP_HAS_GARBAGE;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment