Re: [PATCH] Refactoring of LWLock tranches

On Thu, Dec 31, 2015 at 3:36 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> Going further on this work, I have written a patch for separating the
> tranches for extensions. The basic idea is to expose two new API's,
> first to request a new tranche and second to assign a lock from that
> tranche.
> RequestAddinLWLockTranche(const char *tranche_name, int num_lwlocks)
> will be used by extensions to request a new tranche with specified number
> of locks, this will be used instead of current API RequestAddinLWLocks().

I like this idea.

> LWLock *LWLockAssignFromTranche(const char *tranche_name) will
> assign a lock for the specified tranche. This also ensures that no
> more than requested number of LWLocks can be assigned from a
> specified tranche.

However, this seems like an awkward API, because if there are many
LWLocks you're going to need to look up the tranche name many times,
and then you're going to need to make an array of pointers to them
somehow, introducing a thoroughly unnecessary layer of indirection.
Instead, I suggest just having a function LWLockPadded
*GetLWLockAddinTranche(const char *tranche_name) that returns a
pointer to the base of the array.

> Also I have retained NUM_USER_DEFINED_LWLOCKS in
> MainLWLockArray so that if any extensions want to assign a LWLock
> after startup, it can be used from this pool. The tranche for such locks
> will be reported as main.

I don't like this. I think we should get rid of
NUM_USER_DEFINED_LWLOCKS altogether. It's just an accident waiting to
happen, and I don't think there are enough people using LWLocks from
extensions that we'll annoy very many people by breaking backward
compatibility here. If we are going to care about backward
compatibility, then I think the old-style lwlocks should go in their
own tranche, not main. But personally, I think it'd be better to just
force a hard break and make people switch to using the new APIs.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company