On Mon, Oct 2, 2017 at 8:00 PM, Alexander Korotkov
<a.korot...@postgrespro.ru> wrote:
> What happen if exactly this "continue" fires?
>
>> if (GistFollowRight(stack->page))
>> {
>> if (!xlocked)
>> {
>> LockBuffer(stack->buffer, GIST_UNLOCK);
>> LockBuffer(stack->buffer, GIST_EXCLUSIVE);
>> xlocked = true;
>> /* someone might've completed the split when we unlocked */
>> if (!GistFollowRight(stack->page))
>> continue;
>
>
> In this case we might get xlocked == true without calling
> CheckForSerializableConflictIn().
Indeed! I've overlooked it. I'm remembering this issue, we were
considering not fixing splits if in Serializable isolation, but
dropped the idea.
CheckForSerializableConflictIn() must be after every exclusive lock.> I think it would be rather safe and easy for understanding to more > CheckForSerializableConflictIn() directly before gistinserttuple(). The difference is that after lock we have conditions to change page, and before gistinserttuple() we have actual intent to change page. >From the point of future development first version is better (if some new calls occasionally spawn in), but it has 3 calls while your proposal have 2 calls. It seems to me that CheckForSerializableConflictIn() before gistinserttuple() is better as for now. Best regards, Andrey Borodin. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
