Re: GiST VACUUM
От | Andrey Borodin |
---|---|
Тема | Re: GiST VACUUM |
Дата | |
Msg-id | 89E1C149-8E24-4FF6-BF77-6119606AFCA9@yandex-team.ru обсуждение исходный текст |
Ответ на | Re: GiST VACUUM (Heikki Linnakangas <hlinnaka@iki.fi>) |
Список | pgsql-hackers |
> 12 июля 2018 г., в 20:40, Heikki Linnakangas <hlinnaka@iki.fi> написал(а): > > On 12/07/18 19:06, Andrey Borodin wrote: >>> 11 июля 2018 г., в 0:07, Heikki Linnakangas <hlinnaka@iki.fi> >>> написал(а): >>> This seems misplaced. This code deals with internal pages, and as >>> far as I can see, this patch never marks internal pages as deleted, >>> only leaf pages. However, we should have something like this in the >>> leaf-page branch, to deal with the case that an insertion lands on >>> a page that was concurrently deleted. Did you have any tests, where >>> an insertion runs concurrently with vacuum, that would exercise >>> this? >> That bug could manifest only in case of crash between removing >> downlinks and marking pages deleted. > > Hmm. The downlink is removed first, so I don't think you can see that situation after a crash. After a crash, you mighthave some empty, orphaned, pages that have already been unlinked from the parent, but a search/insert should never encounterthem. > > Actually, now that I think about it more, I'm not happy with leaving orphaned pages like that behind. Let's WAL-log theremoval of the downlink, and marking the leaf pages as deleted, in one WAL record, to avoid that. OK, will do this. But this will complicate WAL replay seriously, and I do not know a proper way to test that (BTW there isGiST amcheck in progress, but I decided to leave it for a while). > > But the situation in gistdoinsert(), where you encounter a deleted leaf page, could happen during normal operation, ifvacuum runs concurrently with an insert. Insertion locks only one page at a time, as it descends the tree, so after ithas released the lock on the parent, but before it has locked the child, vacuum might have deleted the page. In the latestpatch, you're checking for that just before swapping the shared lock for an exclusive one, but I think that's wrong;you need to check for that after swapping the lock, because otherwise vacuum might delete the page while you're notholding the lock. Looks like a valid concern, I'll move that code again. > >> I do not know how to test this >> reliably. Internal pages are locked before leafs and locks are >> coupled. No cuncurrent backend can see downlinks to pages being >> deleted, unless crash happens. > > Are you sure? At a quick glance, I don't think the locks are coupled. Sorry for overquoting + /* rescan inner pages that had empty child pages */ + foreach(cell,rescanList) + { + Buffer buffer; + Page page; + OffsetNumber i, + maxoff; + IndexTuple idxtuple; + ItemId iid; + OffsetNumber todelete[MaxOffsetNumber]; + Buffer buftodelete[MaxOffsetNumber]; + int ntodelete = 0; + + buffer = ReadBufferExtended(rel, MAIN_FORKNUM, (BlockNumber)lfirst_int(cell), + RBM_NORMAL, info->strategy); + LockBuffer(buffer, GIST_EXCLUSIVE); Here's first lock + gistcheckpage(rel, buffer); + page = (Page) BufferGetPage(buffer); + + Assert(!GistPageIsLeaf(page)); + + maxoff = PageGetMaxOffsetNumber(page); + + for (i = OffsetNumberNext(FirstOffsetNumber); i <= maxoff; i = OffsetNumberNext(i)) + { + Buffer leafBuffer; + Page leafPage; + + iid = PageGetItemId(page, i); + idxtuple = (IndexTuple) PageGetItem(page, iid); + + leafBuffer = ReadBufferExtended(rel, MAIN_FORKNUM, ItemPointerGetBlockNumber(&(idxtuple->t_tid)), + RBM_NORMAL, info->strategy); + LockBuffer(leafBuffer, GIST_EXCLUSIVE); now locks are coupled in a top-down descent > > We do need some way of testing this.. Can we test replication of concurrent VACUUM and inserts in existing test suit? I just do not know. I can do this tests manually if this is enough. Best regards, Andrey Borodin.
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Alvaro HerreraДата:
Сообщение: Re: Internal error XX000 with enable_partition_pruning=on, pg 11beta1 on Debian
Следующее
От: Alvaro HerreraДата:
Сообщение: Re: Cannot dump foreign key constraints on partitioned table