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>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "aekorotkov(at)gmail(dot)com" <aekorotkov(at)gmail(dot)com>, "andres(at)anarazel(dot)de" <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 20:37:08
Message-ID: 32F926FF-3E53-40CE-B176-21CEC60E3EF1@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On Aug 4, 2015, at 4:54 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 08/04/2015 03:15 PM, Robert Haas wrote:
>> On Tue, Jul 28, 2015 at 3:28 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>> * 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.
>>
>> This is a good idea, but it's not easy to do in the style of
>> rmgrlist.h, because I don't believe there's any way to define a macro
>> that expands to a preprocessor directive. Attached is a patch that
>> instead generates the list of macros from a text file, and also
>> generates an array inside lwlock.c with the lock names that gets used
>> by the Trace_lwlocks stuff where applicable.
>>
>> Any objections to this solution to the problem? If not, I'd like to
>> go ahead and push this much. I can't test the Windows changes
>> locally, though, so it would be helpful if someone could check that
>> out.
>
> A more low-tech solution would be to something like this in lwlocknames.c:
>
> static char *MainLWLockNames[NUM_INDIVIDUAL_LWLOCKS];
>
> /* Turn pointer into one of the LWLocks in main array into an index number */
> #define NAME_LWLOCK(l, name) MainLWLockNames[l - MainLWLockArray)] = name
>
> InitLWLockNames()
> {
> NAME_LWLOCK(ShmemIndexLock, "ShmemIndexLock");
> NAME_LWLOCK(OidGenLock, "OidGenLock");
> ...
> }
>
> That would not be auto-generated, so you'd need to keep that list in sync with lwlock.h, but it would be much better than the original patch because if you forgot to add an entry in the names-array, the numbering of all the other locks would not go wrong. And you could have a runtime check that complains if there's an entry missing, like Ildus did in his latest patch.
>
> I have no particular objection to your perl script either, though. I'll leave it up to you.
>
> - Heikki
>

A new version of the patch. I used your idea with macros, and with tranches that
allowed us to remove array with names (they can be written directly to the corresponding
tranche).

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

Attachment Content-Type Size
extend_pg_stat_activity_v7.patch application/octet-stream 58.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-08-04 20:38:58 Re: more-helpful-izing a debug message
Previous Message Robert Haas 2015-08-04 20:37:04 Re: Dependency between bgw_notify_pid and bgw_flags