Обсуждение: Safely calling index_getprocinfo() while holding an nbtree exclusivebuffer lock
Safely calling index_getprocinfo() while holding an nbtree exclusivebuffer lock
От
Peter Geoghegan
Дата:
My nbtree patch [1] needs to call index_getprocinfo() with an exclusive buffer lock held during a leaf page split. This has an undetectable self-deadlock (LWLock self-deadlock) risk: a syscache lookup against pg_proc might have a catcache miss, ending with an index scan that needs to access the very same buffer. That's not acceptable. There is very similar code to this in SP-GiST: there is a index_getprocinfo() call within doPickSplit(), to get the user-defined method for a split (SPGIST_PICKSPLIT_PROC). My nbtree patch builds a new insertion scankey to determine how many attributes we can safely truncate away in new pivot tuples -- it would be tricky to do this lookup outside of the split function. I suppose that it's okay to do this in SP-GiST without special care because there cannot be an SP-GiST index on a system catalog. I'll need to do something else about it given that I'm doing this within nbtree, though -- I don't want to complicate the code that deals with insertion scankeys to make this work. Here is a strategy that fixes the problem without complicating matters for nbtree: It should be safe if I make a point of using a special comparator (the bitwise one that we already use in other contexts in the patch) with system catalog indexes. We know that they cannot be of types that have a varlena header + typstorage != 'p', which ensures that there are no cases where bitwise equality fails to be a reliable indicator of opclass equality (e.g. there are no cases like numeric display scale). We could make sure that this requirement isn't violated in the future by adding a pg_index test to opr_sanity.sql, limiting system catalog indexes to opclasses that are known-safe for the bitwise comparator. Does that seem sane? [1] https://commitfest.postgresql.org/21/1787/ -- Peter Geoghegan
Peter Geoghegan <pg@bowt.ie> writes: > My nbtree patch [1] needs to call index_getprocinfo() with an > exclusive buffer lock held during a leaf page split. I think you should stop right there and ask why. Surely that info can be obtained before starting the operation? Quite aside from the deadlock hazard, I do not think holding an exclusive buffer lock for long enough to go consult a system catalog will be acceptable from a performance/concurrency standpoint. regards, tom lane
Re: Safely calling index_getprocinfo() while holding an nbtreeexclusive buffer lock
От
Peter Geoghegan
Дата:
On Mon, Jan 14, 2019 at 7:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I think you should stop right there and ask why. Surely that info > can be obtained before starting the operation? *Thinks some more* Uh, I'm already telling the same _bt_truncate() code path that it is being called from a CREATE INDEX, allowing it to avoid accessing the metapage. I now think that it would be perfectly acceptable to just pass down the insertion scan key for the tuple that caused the split, instead of that build bool, and handle both deadlock issues (index_getprocinfo() hazard and metapage hazard) that way instead. Heikki expressed some concerns about the way the patch accesses the metapage already. I jumped the gun with this catalog index business. Clearly I'd be much better off avoiding *all* new buffer lock protocol stuff by getting both pieces of information up-front -- for some reason I thought that that would be harder than it now appears. Thanks -- Peter Geoghegan