Horton
Menu

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

Andrey Borodin <x4mmm@yandex-team.ru>

View thread
From: Andrey Borodin <x4mmm@yandex-team.ru>
To: Noah Misch <noah@leadboat.com>
Cc: Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Date: 2021-07-21T08:13:08Z
Lists: pgsql-bugs

Attachments


> 21 июля 2021 г., в 02:49, Noah Misch <noah@leadboat.com> написал(а):
> 
> On Wed, Jul 21, 2021 at 12:38:25AM +0500, Andrey Borodin wrote:
>>> 19 июля 2021 г., в 23:41, Andrey Borodin <x4mmm@yandex-team.ru> написал(а):
>>>> 19 июля 2021 г., в 23:10, Noah Misch <noah@leadboat.com> написал(а):
>>>> Suppose we
>>>> have three PGPROC entries with the same VXID, two prepared transactions and
>>>> one regular transaction.  Waiting for all three could be tolerable, though
>>>> avoiding that would be nice.  Should we follow transactions differently to
>>>> avoid that?
>>> 
>>> We don’t have to wait for regular Xid in this case at all. Because it would be finished with VXID.
> 
> I don't understand those two sentences.  Could you write more?
Suppose we have a VXID conflicting with reindexed relation lock, and have a PGPROC with regular Xid (not 2PC) for this VXID.
We do not need to return this xid from TwoPhaseGetXidByVXid() for extra wait. This situation is covered by normal vxid handling in VirtualXactLock().
+    /* Save the xid to test if transaction coverted to 2pc later */
+    xid = proc->xid;


>>> We are looking for transaction that was only VXID during GetLockConflicts(). In conflicts array we may have each VXID only once.
>>> Other 2PCs with same VXID may be older or newer than target 2PC.
>>> Older 2PCs must be with XID in conflicts array. So we might wait for all 2PC with known XIDs. Then for each ambiguous VXID->XID mapping choose oldest XID.
> 
>> Unfortunately, this is just wrong. Older 2PC with same VXID don't have to be in conflicts array. They might be of some other unrelated 2PC working with different relations.
> 
> I agree.  Would it work to do the following sequence in WaitForLockers()?
> 
> 1. In GetLockConflicts(), record a list of conflicting XIDs.  Also record a
>   list of conflicting LXIDs having no XID.
> 2. Wait on all LXIDs recorded in (1).  They have either ended or converted to
>   prepared transactions.
> 3. Inner-join the present list of prepared transactions with the list of
>   LXIDs from (1).  Record the XIDs of matched entries.
> 4. Wait on all XIDs recorded in (1) and (3).
> 
> While that may wait on some prepared XIDs needlessly, it can't degrade into
> persistent starvation.  We could reduce the chance of a needless XID wait by
> remembering the list of old prepared XIDs before (1) and not adding any of
> those remembered, old XIDs in (3).  That last bit probably would be
> unjustified complexity, but maybe not.
I think this protocol is equivalent to waiting on all Xids with VXid.
I consider this protocol safe. FPA implementation.
Patch 0001 is intact version of previous patch.
There are two additions:
1. Prefer xids to vxids in GetLockConflicts()
2. Wait on all 2PCs with given VXid.

Thanks!

Best regards, Andrey Borodin.

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Improve contrib/amcheck's tests for CREATE INDEX CONCURRENTLY.

  2. Fix minor memory leaks in pg_dump.

  3. Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY.

  4. Fix CREATE INDEX CONCURRENTLY for the newest prepared transactions.

  5. Add -w back to the flags for pg_ctl (re)start in PostgresNode

  6. Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions.