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

From: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Date: 2015-08-04 10:44:13
Message-ID: 55C0977D.4020204@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 08/03/2015 04:25 PM, Ildus Kurbangaliev wrote:
> On 07/28/2015 10:28 PM, Heikki Linnakangas wrote:
>> On 07/27/2015 01:20 PM, Ildus Kurbangaliev wrote:
>>> Hello.
>>> In the attached patch I've made a refactoring for tranches.
>>> The prefix for them was extended, and I've did a split of LWLockAssign
>>> to two
>>> functions (one with tranche and second for user defined LWLocks).
>>
>> This needs some work in order to be maintainable:
>>
>> * The patch requires that the LWLOCK_INDIVIDUAL_NAMES array is kept
>> in sync with the list of individual locks in lwlock.h. Sooner or
>> later someone will add an LWLock and forget to update the
>> names-array. That needs to be made less error-prone, so that the
>> names are maintained in the same place as the #defines. Perhaps
>> something like rmgrlist.h.
>>
>> * The "base" tranches are a bit funny. They all have the same
>> array_base, pointing to MainLWLockArray. If there are e.g. 5 clog
>> buffer locks, I would expect the T_NAME() to return "ClogBufferLocks"
>> for all of them, and T_ID() to return numbers between 0-4. But in
>> reality, T_ID() will return something like 55-59.
>>
>> Instead of passing a tranche-id to LWLockAssign(), I think it would
>> be more clear to have a new function to allocate a contiguous block
>> of lwlocks as a new tranche. It could then set the base correctly.
>>
>> * Instead of having LWLOCK_INDIVIDUAL_NAMES to name "individual"
>> locks, how about just giving each one of them a separate tranche?
>>
>> * User manual needs to be updated to explain the new column in
>> pg_stat_activity.
>>
>> - Heikki
>>
>
> Hello. Thanks for review. I attached new version of patch.
>
> It adds new field in pg_stat_activity that shows current wait in backend.
>
> I've did a some refactoring LWLocks tranche mechanism. In lwlock.c only
> invididual and user defined LWLocks is creating, other LWLocks created by
> modules who need them. I think that is more logical (user know about
> module,
> not module about of all users). It also simplifies LWLocks acquirement.
>
> Now each individual LWLock and other groups of LWLocks have their
> tranche, and can
> be easily identified. If somebody will add new individual LWLock and
> forget to add
> its name, postgres will show a message. Individual LWLocks still
> allocated in
> one array but tranches for them point to particular index in main array.
>
> Sample:
>
> b1=# select pid, wait_event from pg_stat_activity; \watch 1
>
> pid | wait_event
> ------+-------------------------
> 7722 | Storage: READ
> 7653 |
> 7723 | Network: WRITE
> 7725 | Network: READ
> 7727 | Locks: Transaction
> 7731 | Storage: READ
> 7734 | Network: READ
> 7735 | Storage: READ
> 7739 | LWLocks: WALInsertLocks
> 7738 | Locks: Transaction
> 7740 | LWLocks: BufferMgrLocks
> 7741 | Network: READ
> 7742 | Network: READ
> 7743 | Locks: Transaction

Attatched new version of patch with some small fixes in code

--
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
extend_pg_stat_activity_v6.patch text/x-patch 57.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2015-08-04 11:12:23 Re: GROUP BY before JOIN
Previous Message David Rowley 2015-08-04 10:30:14 Re: GROUP BY before JOIN