From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, 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 13:54:33 |
Message-ID: | 55C0C419.1060407@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2015-08-04 13:54:51 | Re: cost_agg() with AGG_HASHED does not account for startup costs |
Previous Message | Andres Freund | 2015-08-04 13:52:09 | Re: patch: prevent user from setting wal_buffers over 2GB bytes |