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

On Fri, Jun 26, 2015 at 6:26 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Jun 25, 2015 at 11:57 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> >> >> 3. Add new view 'pg_stat_wait_event' with following info:
> >> >> pid - process id of this backend
> >> >> waiting - true for any form of wait, false otherwise
> >> >> wait_event_type - Heavy Weight Lock, Light Weight Lock, I/O wait,
etc
> >> >> wait_event - Lock (Relation), Lock (Relation Extension), etc
> >> I am pretty unconvinced that it's a good idea to try to split up the
> >> wait event into two columns. I'm only proposing ~20 wait states, so
> >> there's something like 5 bits of information here. Spreading that
> >> over two text columns is a lot, and note that Amit's text would
> >> basically recapitulate the contents of the first column in the second
> >> one, which I cannot get excited about.
> > There is an advantage in splitting the columns which is if
wait_event_type
> > column indicates Heavy Weight Lock, then user can go and check further
> > details in pg_locks, I think he can do that even by seeing wait_event
> > column, but that might not be as straightforward as with wait_event_type
> > column.
>
> It's just a matter of writing event_type LIKE 'Lock %' instead of
> event_type = 'Lock'.
>

Yes that way it can be done and may be that is not inconvenient for user,
but then there is other type of information which user might need like what
distinct resources on which wait is possible, which again he can easily
find with
different event_type column. I think some more discussion is required
before we
could freeze the user interface for this feature, but in the meantime I have
prepared an initial patch by adding a new column wait_event in
pg_stat_activity.
For now, I have added the support for Heavy-Weight locks, Light-Weight
locks [1]
and Buffer Cleanup Lock. I could add for other types (spin lock delay
sleep, IO,
network IO, etc.) if there is no objection in the approach used in patch to
implement
this feature.

[1] For LWLocks, currently I have used wait_event as OtherLock for locks
other than NUM_FIXED_LWLOCKS (Refer function NumLWLocks to see all
type of LWLocks). The reason is that there is no straight forward way to
get
the id (lockid) of such locks as for some of those (like shared_buffers,
MaxBackends) the number of locks will depend on run-time configuration
parameters. I think if we want to handle those then we could either do some
math to find out the lockid based on runtime values of these parameters or
we could add tag in LWLock structure (which indicates the lock type) and
fill it during Lock initialization or may be some other better way to do it.

I have still not added documentation and have not changed anything for
waiting column in pg_stat_activity as I think before that we need to
finalize
the user interface. Apart from that as mentioned above still wait for
some event types (like IO, netwrok IO, etc.) is not added and also I think
separate function/'s (like we have for existing ones
pg_stat_get_backend_waiting)
will be required which again depends upon user interface.

Suggestions?

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