Robert Haas <robertmhaas@gmail.com> wrote:
>> We now see that no update ever kills items within _bt_killitems(),
>> because our own update to the index leaf page itself nullifies our
>> ability to kill anything, by changing the page LSN from the one
>> stashed in the index scan state variable. Fortunately, we are not
>> really "self-blocking" dead item cleanup here, because the
>> _bt_check_unique() logic for killing all_dead index entries saves the
>> day by not caring about LSNs. However, that only happens because the
>> index on "aid" is a unique index.
>
>This seems like an oversight. If we modify the page ourselves, could
>we check whether the saved LSN is still current just before, and if
>so, update the saved LSN just after?
I really don't know if that would be worthwhile. It would certainly fix
the regression shown in my test case, but that might not go far enough.
I strongly suspect that there are more complicated workloads where
LP_DEAD cleanup from SELECT statements matters, which is prevented by
the LSN check thing, just because there are always other sessions that
modify the page concurrently. This might be true of Alik's Zipfian test
case, for example.
In Alik's workload, there are two queries: One UPDATE, one SELECT. Even
though the bloated index was a unique index, and so still gets
_bt_check_unique() item killing, the regression is still going to block
LP_DEAD cleanup by the SELECTs, which seems like it might be quite
important there. After all, _bt_check_unique() cleanup has to happen
with an exclusive buffer lock held, whereas the kill_prior_tuple stuff
happens with only a shared buffer lock held. It's not hard to imaging
that there will be a whole lot less LP_DEAD setting, overall.
Alik's workload was one with a high degree of contention on just a few
leaf pages, pages that become very bloated. It's not fair to blame the
bloat we saw there on this regression, but I have to wonder how much it
may have contributed.
--
Peter Geoghegan