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

From: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, "aekorotkov(at)gmail(dot)com" <aekorotkov(at)gmail(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Subject: Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Date: 2015-07-24 07:42:05
Message-ID: B065041E-876D-44D7-B169-4D2B45B9CF6E@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On Jul 24, 2015, at 7:26 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Jul 24, 2015 at 12:31 AM, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru> wrote:
> Hello.
> I’ve changed the previous patch. `group` field in LWLock is removed, so the size of LWLock will not increase.
> Instead of the `group` field I've created new tranches for LWLocks from MainLWLocksArray.
>
> @@ -448,6 +549,30 @@ CreateLWLocks(void)
> + /* Initialized tranche id for buffer mapping lwlocks */
> + for (id = 0; id < NUM_BUFFER_PARTITIONS; id++)
> + {
> + LWLock *lock;
> + lock = &MainLWLockArray[BUFFER_MAPPING_LWLOCK_OFFSET + id].lock;
> + lock->tranche = LW_BUFFER_MAPPING;
> + }
> +
> + /* Initialized tranche id for lock manager lwlocks */
> + for (id = 0; id < NUM_LOCK_PARTITIONS; id++)
> + {
> + LWLock *lock;
> + lock = &MainLWLockArray[LOCK_MANAGER_LWLOCK_OFFSET + id].lock;
> + lock->tranche = LW_LOCK_MANAGER;
> + }
> +
> + /* Initialized tranche id for predicate lock manager lwlocks */
> + for (id = 0; id < NUM_PREDICATELOCK_PARTITIONS; id++)
> + {
> + LWLock *lock;
> + lock = &MainLWLockArray[PREDICATELOCK_MANAGER_LWLOCK_OFFSET + id].lock;
> + lock->tranche = LW_PREDICATELOCK_MANAGER;
> + }
>
>
> I don't think we need to separately set tranche for above locks, we
> can easily identify the same by lockid as is done in my patch.

Tranches have been added because we need to identify other 10 types of LWLocks from
main tranche allocated dynamically. These 3 can be identified much easier, by there is no point to
identify them by other way.

>
> 2.
> +const char *
> +pgstat_get_wait_event_name(uint8 classId, uint8 eventId)
> {
> ..
> }
>
> I don't understand why a single WaitEventType enum is not sufficient
> as in the patch provided by me and why we need to divide it into
> separate structures for each kind of wait, I find that way easily
> understandable.

In current patch if somebody adds new lock or individual lwlock he just add
its name to the corresponding array. With WaitEventType first of all he suppose to
know about WaitEventType, then add new item to enum, add new
struct in types map, and keep in mind that be something wrong with
offsets in types map.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2015-07-24 08:36:32 Re: pgbench - allow backslash-continuations in custom scripts
Previous Message Michael Paquier 2015-07-24 07:05:19 Re: Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );