From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, James Coleman <jtc331(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: remove spurious CREATE INDEX CONCURRENTLY wait |
Date: | 2020-11-10 01:28:26 |
Message-ID: | 20201110012826.GC1887@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Nov 09, 2020 at 04:47:43PM +0100, Dmitry Dolgov wrote:
> > On Tue, Nov 03, 2020 at 07:14:47PM +0100, Dmitry Dolgov wrote:
> > > On Thu, Aug 20, 2020 at 03:11:19PM +0900, Michael Paquier wrote:
> > > On Wed, Aug 19, 2020 at 02:16:46PM -0400, Alvaro Herrera wrote:
> > > > I did not set the flag in REINDEX CONCURRENTLY, but as I understand it
> > > > can be done too, since in essence it's the same thing as a CIC from a
> > > > snapshot management point of view.
> > >
> > > Yes, I see no problems for REINDEX CONCURRENTLY as well as long as
> > > there are no predicates and expressions involved. The transactions
> > > that should be patched are all started in ReindexRelationConcurrently.
> > > The transaction of index_concurrently_swap() cannot set up that
> > > though. Only thing to be careful is to make sure that safe_flag is
> > > correct depending on the list of indexes worked on.
> >
> > Hi,
> >
> > After looking through the thread and reading the patch it seems good,
> > and there are only few minor questions:
> >
> > * Doing the same for REINDEX CONCURRENTLY, which does make sense. In
> > fact it's already mentioned in the commentaries as done, which a bit
> > confusing.
>
> Just to give it a shot, would the attached change be enough?
Could it be possible to rename vacuumFlags to statusFlags first? I
did not see any objection to do this suggestion.
> + 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.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-11-10 01:32:13 | Re: remove spurious CREATE INDEX CONCURRENTLY wait |
Previous Message | Peter Smith | 2020-11-10 01:23:23 | Re: [HACKERS] logical decoding of two-phase transactions |