Re: [DOC] Document concurrent index builds waiting on each other

On Wed, Apr 15, 2020 at 6:31 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2020-04-15 09:31:58 -0400, James Coleman wrote:
> > On Wed, Mar 25, 2020 at 3:58 PM Andres Freund <andres@anarazel.de> wrote:
> > > On 2020-03-25 16:30:10 -0300, Alvaro Herrera wrote:
> > > > I posted this in November
> > > > https://postgr.es/m/20191101203310.GA12239@alvherre.pgsql but I didn't
> > > > put time to go through the issues there.
> > >
> > > Oh, missed that.
> > >
> > >
> > > > I don't know if my approach is exactly what Andres has in mind
> > >
> > > Not quite. I don't think it's generally correct for CIC to set
> > > PROC_IN_VACUUM. I'm doubtful it's the case even just for plain indexes -
> > > we don't want rows to be pruned away from under us. I also think we'd
> > > want to set such a flag during all of the CIC phases?
> > >
> > > What I was thinking of was a new flag, with a distinct value from
> > > PROC_IN_VACUUM. It'd currently just be specified in the
> > > GetCurrentVirtualXIDs() calls in WaitForOlderSnapshots(). That'd avoid
> > > needing to wait for other CICs on different relations. Since CIC is not
> > > permitted on system tables, and CIC doesn't do DML on normal tables, it
> > > seems fairly obviously correct to exclude other CICs.
> >
> > That would keep CIC from blocking other CICs, but it wouldn't solve
> > the problem of CIC blocking vacuum on unrelated tables, right? Perhaps
> > that's orthogonal though.
>
> I am not sure what blocking you are referring to here? CIC shouldn't
> block vacuum on other tables from running? Or do you just mean that
> vacuum will not be able to remove some rows due to the snapshot from the
> CIC? That'd be an orthogonal problem, yes.
>
> If it's about the xmin horizon for vacuum: I think we could probably
> avoid that using the same flag. As vacuum cannot be run against a table
> that has a CIC running (although it'd theoretically be possible to allow
> that), it should be safe to ignore PROC_IN_CIC backends in vacuum's
> GetOldestXmin() call. That might not be true for system relations, but
> we don't allow CIC on those.

Yeah, I mean that if I have a CIC running on table X then vacuum can't
remove dead tuples (from after the CIC's snapshot) on table Y.

That's a pretty significant danger, given the combination of:
1. Index builds on very large tables can take many days, and
2. The well understood problems of high update tables with dead tuples
and poor plans.

I've previously discussed this with other hackers and the reasoning
they'd understood way that we couldn't always safely ignore
PROC_IN_CIC backends in the vacuum's oldest xmin call because of
function indexes, and the fact that (despite clear recommendations to
the contrary), there's nothing actually preventing someone from adding
a function index on table X that queries table Y.

I'm not sure I buy that we should care about people doing something
clearly so dangerous, but...I grant that it'd be nice not to cause new
crashes.

James