Re: remove spurious CREATE INDEX CONCURRENTLY wait

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, James Coleman <jtc331(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: remove spurious CREATE INDEX CONCURRENTLY wait
Date: 2020-11-17 15:55:01
Message-ID: 20201117155501.GA13805@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020-Nov-16, Alvaro Herrera wrote:

> On 2020-Nov-09, Tom Lane wrote:
>
> > Michael Paquier <michael(at)paquier(dot)xyz> writes:
> > >> + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
> > >> + MyProc->vacuumFlags |= PROC_IN_SAFE_IC;
> > >> + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlags;
> > >> + LWLockRelease(ProcArrayLock);
> >
> > > I can't help noticing that you are repeating the same code pattern
> > > eight times. I think that this should be in its own routine, and that
> > > we had better document that this should be called just after starting
> > > a transaction, with an assertion enforcing that.
> >
> > Do we really need exclusive lock on the ProcArray to make this flag
> > change? That seems pretty bad from a concurrency standpoint.
>
> BTW I now know that the reason for taking ProcArrayLock is not the
> vacuumFlags itself, but rather MyProc->pgxactoff, which can move.

... ah, but I realize now that this means that we can use shared lock
here, not exclusive, which is already an enormous improvement. That's
because ->pgxactoff can only be changed with exclusive lock held; so as
long as we hold shared, the array item cannot move.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-11-17 16:04:33 Re: Is postgres ready for 2038?
Previous Message Heikki Linnakangas 2020-11-17 15:46:25 Re: Protect syscache from bloating with negative cache entries