On Thu, Jan 16, 2020 at 5:11 PM Peter Geoghegan <p...@bowt.ie> wrote: > I find this argument convincing. I'll try to get this committed soon. > > While you could have used bt_index_parent_check() or heapallindexed to > detect the issue, those two options are a lot more expensive (plus the > former option won't work on a standby). Relaxing the principle that > says that we shouldn't hold multiple buffer locks concurrently doesn't > seem like that much to ask for to get such a clear benefit.
Having looked into it some more, I now have my doubts about this patch. REDO routine code like btree_xlog_split() and btree_xlog_unlink_page() feel entitled to only lock one page at a time, which invalidates the assumption that we can couple locks on the leaf level to verify mutual agreement in left and right sibling links (with only an AccessShareLock on bt_index_check()'s target index relation). It would definitely be safe for bt_index_check() to so this were it not running in recovery mode, but that doesn't seem very useful on its own. I tried to come up with a specific example of how this could be unsafe, but my explanation was all over the place (this could have had something to do with it being Friday evening). Even still, it's up to the patch to justify why it's safe, and that seems even more difficult. -- Peter Geoghegan
