Re: [BUG FIX] Removing NamedLWLockTrancheArray

At Fri, 03 Mar 2017 17:13:42 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20170303(dot)171342(dot)134582021(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> Hello, some of my collegues found that orafce crashes with
> postgresql compliled with dtrace.
>
> === The cause
>
> The immediate cause was I think that it just did
> LWLockNewTrancheId and forget to do LWLockRegisterTranche. But
> this is hardly detectable by a module developer.

I'm sorry, I found that orafce is calling LWLockRegisterTranche
so this might be a different problem but anyway the problem with
RequestNamedLWLockTranche occurs.

>
> Typical tracepoint looks like the following.
>
> > TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), mode);
>
> Where T_NAME is (LWLockTrancheArray[(lock)->tranche]), as you see
> T_NAME assumes that all tranches pointed from lwlocks are
> available on the array but the tranches without
> LWLockRegisterTranche are not. What is worse this doesn't harm so
> much without dtrace (or LWLOCK_STATS or error in LWLockRelease)
> . Even if dtrace is activated one or two unregistred tranches
> (faultly) have their seat (filled with trash) at the end of the
> array with the current code.
>
> As my understanding there are two ways to use lwlocks in
> extension. One is using LWLockNewTrancheId and
> LWLockRegisterTanche on shared memory provided by the module. The
> other is using RequestNamedLWLockTranche in _PG_init and
> GetNamedLWLockTranche to acquire locks provided by postgresql.
>
> I couldn't find a documentation about lwlock and trance in
> extentions, is there any?
>
>
> === How to fix this
>
> The most straightforward way to fix this is chekcing the validity
> of tranche id on initilization of a lwlock. Even though I think
> that degradation won't be a matter here, NamedLWLockTrancheArray
> makes the things very ineffective. After some consideration I
> decided to remove NamedLWLockTrancheArray.
>
>
> === The patch
>
> The attached patch (is for current master) is aming to fix all
> the problem by doing the following two things.
>
> - Remove NameLWLockTrancheArray and all tranches are registered
> in LWLockTrancheArray. This seems to work at least for
> !EXEC_BAKCEND environment but I haven't tested with EXEC_BACKEND.
>
> - Check tranche id in LWLockInitialize.
>
> The first one required refactoring of CreateLWLocks. It is
> changed to register tranches first then initialize lwlokcs.
>
> The problem is found with PG9.6 and this should be backpatched at
> least to the version. I haven't tested PG9.5 and 9.4 but it seems
> to need different amendment. 9.3 doesn't has tranche.
>
> regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center