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
signature.asc
Description: PGP signature
