Re: Improve checking for pg_index.xmin

On Tue, Mar 24, 2020 at 3:38 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Sun, Jan 12, 2020 at 3:33 AM Alexander Korotkov
> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> >
> > On Wed, Jan 8, 2020 at 4:37 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
> > > > On 01/11/2019 01:50, Alexander Korotkov wrote:
> > > >> This happens so, because we're checking that there is no broken HOT
> > > >> chains after index creation by comparison pg_index.xmin and
> > > >> TransactionXmin. So, we check that pg_index.xmin is in the past for
> > > >> current transaction in lossy way by comparison just xmins. Attached
> > > >> patch changes this check to XidInMVCCSnapshot().
> > > >> With patch the issue is gone. My doubt about this patch is that it
> > > >> changes check with TransactionXmin to check with GetActiveSnapshot(),
> > > >> which might be more recent. However, query shouldn't be executer with
> > > >> older snapshot than one it was planned with.
> > >
> > > > Hmm. Maybe you could construct a case like that with a creative mix of
> > > > stable and volatile functions? Using GetOldestSnapshot() would be safer.
> > >
> > > I really wonder if this is safe at all.
> > >
> > > (1) Can we assume that the query will be executed with same-or-newer
> > > snapshot as what was used by the planner? There's no such constraint
> > > in the plancache, I'm pretty sure.
> > >
> > > (2) Is committed-ness of the index-creating transaction actually
> > > sufficient to ensure that none of the broken HOT chains it saw are
> > > a problem for the onlooker transaction? This is, at best, really
> > > un-obvious. Some of those HOT chains could involve xacts that were
> > > still not committed when the index build finished, I believe.
> > >
> > > (3) What if the index was made with CREATE INDEX CONCURRENTLY ---
> > > which xid is actually on the pg_index row, and how does that factor
> > > into (1) and (2)?
> >
> > Thank you for pointing. I'll investigate these issues in detail.
> >
>
> Are you planning to work on this patch [1] for current CF? If not,
> then I think it is better to either move this to the next CF or mark
> it as RWF.

I didn't manage to investigate this subject and provide new patch
version. I'm marking this RWF.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company