Re: [PATCH] Refactoring of LWLock tranches

On Tue, Feb 9, 2016 at 11:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, Feb 9, 2016 at 7:53 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> > On Fri, Feb 5, 2016 at 3:17 AM, Robert Haas <robertmhaas(at)gmail(dot)com>
wrote:
> >> I think we ought to move the buffer mapping, lock manager, and
> >> predicate lock manager locks into their own tranches also, perhaps
> >> using this new named-tranche facility.
> >
> > Makes sense and attached patch implements it using new named
> > tranches facility. One thing to note is that to make it work on
> > EXEC_BACKEND builds, I have passed the individual LWLock
> > array via save-restore backendparams mechanism. Now instead
> > we could have initialised the LWLock arrays in each backend at
> > start-up and I have tried to analyse that way as well, but we need
> > to use NamedLWLockTrancheArray instead of
> > NamedLWLockTrancheRequestArray in GetNamedLWLockTranche()
> > and not only that, we also need to store number of locks
> > information in NamedLWLockTrancheArray as well. So it seems
> > it is better to use save-restore backendparams mechanism for
> > passing LWLock arrays.
>
> I'm not very keen on the way this puts RequestNamedTranches() into
> miscinit.c. That doesn't really seem like it goes there, and I feel
> like each subsystem should be responsible for requesting its own named
> tranche rather than having it be centralized like this.
>

The reason for using centralized way is that we need to request
named tranches before initialization of shared memory and as far as
I can see, currently there is no way in the subsystems where they can
issue such a request, so one possibility is that we introduce new API's
like InitBufferLWLocks(), InitLmgrLWLocks(), InitPredicateLWLocks()
in respective subsystem and call them in
CreateSharedMemoryAndSemaphores() before shared memory
initialization. Does by doing that way addresses your concern?

> I think it might be helpful to split this into two patches. The first
> would remove LWLockAssign() and NumLWLocks() and related bits, and
> would use NUM_FIXED_LWLOCKS any place we currently call NumLWLocks().

Okay, attached patch does that.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com