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

On Sat, Jul 25, 2015 at 10:30 PM, Ildus Kurbangaliev <
i(dot)kurbangaliev(at)postgrespro(dot)ru> wrote:

>
> > On Jul 24, 2015, at 10:02 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
> > Also, the patch should not invent a new array similar but not quite
> > identical to LockTagTypeNames[].
> >
> > This is goofy:
> >
> > + if (tranche_id > 0)
> > + result->tranche = tranche_id;
> > + else
> > + result->tranche = LW_USERDEFINED;
> >
> > If we're going to make everybody pass a tranche ID when they call
> > LWLockAssign(), then they should have to do that always, not sometimes
> > pass a tranche ID and otherwise pass 0, which is actually a valid
> > tranche ID but to which you've given a weird special meaning here.
> > I'd also name the constants differently, like LWLOCK_TRANCHE_XXX or
> > something. LW_ is a somewhat ambiguous prefix.
> >
> > The idea of tranches is really that each tranche is an array of items
> > each of which contains 1 or more lwlocks. Here you are intermingling
> > different tranches. I guess that won't really break anything but it
> > seems ugly.
>
> Maybe it will be better to split LWLockAssign to two functions then, keep
> name
> LWLockAssign for user defined locks and other function with tranche_id.
>
> > On Jul 25, 2015, at 1:54 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > That anyway he has to do it either you go by defining individual arrays
> > or having unified WaitEventType enum for individual arrays he has to
> > find out that array. Another thing is with that you can just encapsulate
> > this information in one byte in structure PgBackendStatus, rather than
> > using more number of bytes (4 bytes) and I think the function for
> reporting
> > Waitevent will be much more simplified.
>
> In my way there are no special meaning for names. Array with names
> located in lwlock.c and lock.c, and can be used for other things (for
> example
> tracing). One byte sounds good only for this case.

Do you mean to say that you need more than 256 events? I am not sure
if we can add that many events without adding performance penalty
in some path.

The original idea was proposed for one-byte and the patch was written
considering the same, now you are planning to extend (which is okay), but
modifying it without any prior consent is what slightly a matter of concern.

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