Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data

> 19 дек. 2020 г., в 22:48, Victor Yegorov <vyegorov(at)gmail(dot)com> написал(а):
>
> сб, 19 дек. 2020 г. в 18:13, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>:
> I observe:
> NOTICE: heap tuple (1,8) from table "t1" lacks matching index tuple within index "i1"
> I expect: awaiting 'x' commit before index is created, correct index after.
>
> I agree, that behaviour is unexpected. But getting a notice that requires me
> to re-create the index some time later is not better (from DBA perspective).
>
> Maybe it'd be better to wait on prepared xacts like on other open ordinary transactions?

This is not a real NOTICE. I just used a bit altered amcheck to diagnose the problem. It's an incorrect index. It lacks some tuples. It will not find existing data, failing to provide "read committed" consistency guaranties.
Fix waits for prepared xacts just like any other transaction.

> 19 дек. 2020 г., в 22:57, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> написал(а):
>
>> Meanwhile CREATE INDEX CONCURRENTLY expects that locks are dropped only
>> when transaction commit is visible.
>
> Don't follow your point here --- I'm pretty sure that prepared xacts
> continue to hold their locks.
Uhmm, yes, locks are there. Relation is locked with RowExclusiveLock, but this lock is ignored by WaitForLockers(heaplocktag, ShareLock, true). Locking relation with ShareLock works as expected.

>> PFA draft of a fix.
>
> Haven't you completely broken VirtualXactLock()? Certainly, whether the
> target is a normal or prepared transaction shouldn't alter the meaning
> of the "wait" flag.
You are right, the patch has a bug here.

> In general, I wonder whether WaitForLockersMultiple and GetLockConflicts
> need to gain an additional parameter indicating whether to consider
> prepared xacts. It's not clear to me that their current behavior is wrong
> for all possible uses.
I don't see usages besides indexing stuff. But maybe it worth to analyse each case...

BTW do we need a test for this? Will isolation test be good at checking this?

Thanks!

Best regards, Andrey Borodin.