On Mon, Nov 13, 2017 at 8:08 PM, Peter Geoghegan <p...@bowt.ie> wrote:
> On Mon, Nov 13, 2017 at 6:56 PM, Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > /* > > * We would like to prevent concurrent cleanup process. For that we > will > > * lock metapage in exclusive mode using LockPage() call. Nobody other > > * will use that lock for metapage, so we keep possibility of > concurrent > > * insertion into pending list > > */ > > > > So I conjecture that this has been introduced for not the reason why > > we need to detect deadlock but the reason why we need to a different > > lock from the lock used by insertion into pending list. > > I understood that much, but I think that we need to detect problems > and recover from them (something like _bt_page_recyclable()), rather > than preventing them with pessimistic locking -- or, at least, there > is no reason I know to think that the HW lock is sufficient, and I am > tempted to go that way to fix this. Commit e9568083, which added the > feature that led to commit e2c79e14, may itself be the basic problem > here. e2c79e14 was to fix a pre-existing bug, but probably e9568083 made that bug easier to hit than it was before. (Which is not to say that e9568083 can't contain bugs of its own, of course) > * According to the GIN README, the pending list cleanup by VACUUM has > a super-exclusive lock on the root, to block out concurrent inserters > (that hold a pin on the root throughout). That's why it was/is okay > that VACUUM recycled pending list pages without a RecentGlobalXmin > interlock. Not so easy, but still not hard. > The only reference to super-exclusive lock in src/backend/access/gin/README, that I can find, is about posting trees, not pending lists. Can you quote or give line numbers of the section you are referring to? Cheers, Jeff