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-03 13:25:17
Message-ID: 55BF6BBD.6090207@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

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

Attachment Content-Type Size
extend_pg_stat_activity_v5.patch text/x-patch 57.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2015-08-03 13:37:05 Re: pg_rewind failure by file deletion in source server
Previous Message Merlin Moncure 2015-08-03 13:09:40 Re: Autonomous Transaction is back