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 00:08:32
Message-ID: 20201117000832.GA19824@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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).

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Nancarrow 2020-11-17 00:11:22 Crash in virtual file descriptor FDDEBUG code
Previous Message David G. Johnston 2020-11-16 23:59:15 Re: Terminate the idle sessions