On Wed, Apr 22, 2020 at 9:05 PM Peter Geoghegan <p...@bowt.ie> wrote: > > Given hint bits it seems fairly hard to make that a reliable check. > > I don't follow. It doesn't have to be a perfect check. Detecting if > there is *any* buffer lock held at all would be a big improvement.
It is true that the assumptions that heapam makes about what a buffer pin will prevent (that a pin will prevent any kind of page defragmentation) are not really compatible with marking pages as undefined in lower level code like bufmgr.c. There are too many exceptions for it to work like that. The case I was really thinking about was the nbtree _bt_drop_lock_and_maybe_pin() stuff, which is very confusing. The confusing structure of the BTScanPosIsPinned()/_bt_drop_lock_and_maybe_pin() code more or less caused the skip scan patch to have numerous bugs involving code holding a buffer pin, but not a buffer lock, at least when I last looked at it a couple of months ago. The only thing having a pin on a leaf page guarantees is that the TIDs from tuples on the page won't be concurrently recycled by VACUUM. This is a very weak guarantee -- in particular, it's much weaker than the guarantees around buffer pins that apply in heapam. It's certainly not going to prevent any kind of defragmentation of the page -- the page can even split, for example. Any code that relies on holding a pin to prevent anything more than that is broken, but possibly only in a subtle way. It's not like page splits happen all that frequently. Given that I was concerned about a fairly specific situation, a specific solution seems like it might be the best way to structure the extra checks. The attached rough patch shows the kind of approach that might be practical in specific index access methods. This works on top of the patch I posted yesterday. The idea is to mark the buffer's page as a noaccess region within _bt_drop_lock_and_maybe_pin(), and then mark it defined again at either of the two points that we might have to relock (but not repin) the buffer to re-read the page. This doesn't cause any regression test failures, so maybe there are no bugs like this currently, but it still seems like it might be worth pursuing on top of the buffer pin stuff. -- Peter Geoghegan
0003-Mark-lockless-nbtree-leaf-page-memory-undefined.patch
Description: Binary data