On Tue, Dec 11, 2018 at 10:14 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> > 11 дек. 2018 г., в 3:43, Alexander Korotkov <aekorotkov@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