Horton
Menu

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

Noah Misch <noah@leadboat.com>

View thread
From: Noah Misch <noah@leadboat.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Date: 2021-07-21T17:55:30Z
Lists: pgsql-bugs 
On Wed, Jul 21, 2021 at 01:13:08PM +0500, Andrey Borodin wrote:
> > 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;

Got it.

> > 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.

These drafts use reasonable concepts.  Would you develop them into a
ready-to-commit patch?  You may be able to translate your probabilistic test
procedures from https://gist.github.com/x4m/8b6025995eedf29bf588727375014dfc
into something like the src/bin/pgbench/t/001_pgbench_with_server.pl test of
pg_enum_oid_index.

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.