Re: remove spurious CREATE INDEX CONCURRENTLY wait

Alvaro Herrera <alvherre@alvh.no-ip.org>

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>, Dmitry Dolgov <9erthalion6@gmail.com>, James Coleman <jtc331@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-11-17T00:08:32Z
Lists: pgsql-hackers
On 2020-Nov-09, Tom Lane wrote:

> Michael Paquier <michael@paquier.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.

On the other hand, if we stopped mirroring the flags in ProcGlobal, it
would mean we would have to access all procs' PGPROC entries in
GetSnapshotData, which is undesirable for performance reason (per commit
5788e258bb26).



Commits

  1. Restore lock level to set vacuum flags

  2. Restore lock level to update statusFlags

  3. Avoid spurious waits in concurrent indexing

  4. Centralize logic for skipping useless ereport/elog calls.

  5. Don't hold ProcArrayLock longer than needed in rare cases

  6. Relax lock level for setting PGPROC->statusFlags

  7. Rename PGPROC->vacuumFlags to statusFlags

  8. snapshot scalability: Move PGXACT->vacuumFlags to ProcGlobal->vacuumFlags.

  9. Allow an autovacuum worker to be interrupted automatically when it is found