> 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. We are going to extend
waits monitoring, add more views, some profiling. That’s why waits have to be
groupable by classes.
----
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company