On Tue, Dec 11, 2018 at 10:14 AM Andrey Borodin <x4...@yandex-team.ru> wrote: > > 11 дек. 2018 г., в 3:43, Alexander Korotkov <aekorot...@gmail.com> > > написал(а): > > > > Attached patch appears to be incomplete. GinPageSetDeleteXid() is > > called only in ginRedoDeletePage(), so only in recovery, while it > > should be set during normal work too. deleteXid field of > > ginxlogDeletePage is never set. > > Sorry, I've messed it again. Forgot to include ginvacuum.c changes. Here it > is.
BTW, I still can't see you're setting deleteXid field of ginxlogDeletePage struct anywhere. Also, I note that protection against re-usage of recently deleted pages is implemented differently than it is in B-tree. 1) You check TransactionIdPrecedes(GinPageGetDeleteXid(page), RecentGlobalDataXmin)), while B-tree checks TransactionIdPrecedes(opaque->btpo.xact, RecentGlobalXmin). Could you clarify why do we use RecentGlobalDataXmin instead of RecentGlobalXmin here? 2) B-tree checks this condition both before putting page into FSM and after getting page from FSM. You're checking only after getting page from FSM. Approach of B-tree looks better for me. It's seems more consistent when FSM pages are really free for usage. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
