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

Поиск
Список
Период
Сортировка
От Amit Kapila
Тема Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Дата
Msg-id CAA4eK1+za9zHHk7YJXJMk+udnoVzB=nVb54gT5DP2fMbe_t0Ug@mail.gmail.com
обсуждение исходный текст
Ответ на Re: RFC: replace pg_stat_activity.waiting with something more descriptive  (Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>)
Список pgsql-hackers
On Sat, Jul 25, 2015 at 10:30 PM, Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru> wrote:

> On Jul 24, 2015, at 10:02 PM, Robert Haas <robertmhaas@gmail.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.kapila16@gmail.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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Re: Failing assertions in indxpath.c, placeholder.c and brin_minmax.c
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: New functions