Re: RFC: replace pg_stat_activity.waiting with something more descriptive

Hi, I forgot to mention a significant point.

> At Wed, 22 Jul 2015 17:50:35 +0300, Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru> wrote in
<55AFADBB.9090203@postgrespro.ru>
> > On 07/22/2015 09:10 AM, Kyotaro HORIGUCHI wrote:
> > > Hello,
> > >
> > > At Tue, 21 Jul 2015 14:28:25 +0300, Ildus Kurbangaliev
> > > <i.kurbangaliev@postgrespro.ru> wrote in
> > > <55AE2CD9.4050005@postgrespro.ru>
> > >> On 07/21/2015 01:18 PM, Andres Freund wrote:
> > >>> I'd very much like to avoid increasing the size of struct LWLock. We
> > >>> have a lot of those and I'd still like to inline them with the buffer
> > >>> descriptors. Why do we need a separate group and can't reuse the
> > >>> tranche?  That might require creating a few more tranches, but ...?
> > >>>
> > >>> Andres
> > >> Do you mean moving LWLocks defined by offsets and with dynamic sizes
> > >> to separate tranches?
> > > I think it is too much for the purpose. Only two new tranches and
> > > maybe one or some new members (maybe representing the group) of
> > > trances will do, I suppose.
> > 
> > Can you explain why only two new tranches?
> > There is 13 types of lwlocks (besides individual), and we need
> > separate them somehow.
> 
> Sorry, I minunderstood about tranche.
> 
> Currently tranches other than main are used by WALInsertLocks and
> ReplicationOrigins. Other "dynamic locks" are defined as parts of
> main LWLokcs since they have the same shape with individual
> lwlocks. Leaving the individual locks, every lock groups may have
> their own tranche if we allow lwlocks to have own tranche even if
> it is in MainLWLockArray. New 13-16 trances will be added but no
> need to register their name in LWLOCK_GROUPS[]. After all, this
> array would be renamed such as "IndividualLWLockNames" and the
> name-lookup can be done by the follwoing simple steps.
> 
> - If the the lock is in main tranche, lookup the individual name
>   array for its name.
This lookup is doable by calculation and no need to scan.

> - Elsewise, use the name of its tranche.
> 
> Does this make sense?
> 
> > >> It sounds like an good option, but it will require refactoring of
> > >> current tranches. In current implementation
> > >> i tried not change current things.
> > > Now one of the most controversial points of this patch is the
> > > details of the implement, which largely affects performance drag,
> > > maybe.
> > >
> > >
> > >  From the viewpoint of performance, I have some comments on the
> > >  feature:
> > >
> > >   - LWLockReportStat runs a linear search loop which I suppose
> > >     should be avoided even if the loop count is rather small for
> > >     LWLocks, as Fujii-san said upthread or anywhere.
> > It runs only one time in first acquirement. In previous patch
> > it was much heavier. Anyway this code will be removed if we
> > split main tranche to smaller tranches.
> 
> Ah, this should be the same with what I wrote above, isn't it?
> 
> > >   - Currently pg_stat_activity view is the only API, which would
> > >     be a bit heavy for sampling use. It'd be appreciated to have a
> > >     far lighter means to know the same informtion.
> > Yes, pg_stat_activity is just information about current wait,
> > and it's too heavy for sampling.  Main goal of this patch was
> > creating base structures that can be used later.
> 
> Ok, I see it.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center