Date: Wed, 21 Jul 2021 13:55:30 -0400
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
Subject: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data
Message-ID: <20210721175530.GA1090577@gust.leadboat.com>
References: <20210719181005.GB972361@gust.leadboat.com>
 <1727EC55-0709-4E06-91C9-212682A134C7@yandex-team.ru>
 <BFC46D37-D67B-42DA-8B2C-6CA761911DF7@yandex-team.ru>
 <20210720214936.GA1040253@gust.leadboat.com>
 <E50B139F-5B6B-41F2-8F6D-CC474290CCBA@yandex-team.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <E50B139F-5B6B-41F2-8F6D-CC474290CCBA@yandex-team.ru>
Archived-At: 
 <https://www.postgresql.org/message-id/20210721175530.GA1090577%40gust.leadboat.com>
Precedence: bulk

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.