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

From: "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>
To: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, 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>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Date: 2015-09-01 22:43:51
Message-ID: 20150901224351.GB11484@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015-08-04 23:37:08 +0300, Ildus Kurbangaliev wrote:
> diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
> index 3a58f1e..10c25cf 100644
> --- a/src/backend/access/transam/clog.c
> +++ b/src/backend/access/transam/clog.c
> @@ -457,7 +457,8 @@ CLOGShmemInit(void)
> {
> ClogCtl->PagePrecedes = CLOGPagePrecedes;
> SimpleLruInit(ClogCtl, "CLOG Ctl", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
> - CLogControlLock, "pg_clog");
> + CLogControlLock, "pg_clog",
> + "CLogBufferLocks");
> }

I'd rather just add the name "clog" (etc) as a string once to
SimpleLruInit instead of now four 3 times.

> +/* Lock names. For monitoring purposes */
> +const char *LOCK_NAMES[] =
> +{
> + "Relation",
> + "RelationExtend",
> + "Page",
> + "Tuple",
> + "Transaction",
> + "VirtualTransaction",
> + "SpeculativeToken",
> + "Object",
> + "Userlock",
> + "Advisory"
> +};

Why do we need this rather than DescribeLockTag?

> + /* Create tranches for individual LWLocks */
> + for (i = 0; i < NUM_INDIVIDUAL_LWLOCKS; i++, tranche++)
> + {
> + int id = LWLockNewTrancheId();
> +
> + /*
> + * We need to be sure that generated id is equal to index
> + * for individual LWLocks
> + */
> + Assert(id == i);
> +
> + tranche->array_base = MainLWLockArray;
> + tranche->array_stride = sizeof(LWLockPadded);
> + MemSet(tranche->name, 0, LWLOCK_MAX_TRANCHE_NAME);
> +
> + /* Initialize individual LWLock */
> + LWLockInitialize(&MainLWLockArray[i].lock, id);
> +
> + /* Register new tranche in tranches array */
> + LWLockRegisterTranche(id, tranche);
> + }
> +
> + /* Fill individual LWLock names */
> + InitLWLockNames();

Why a new tranche for each of these? And it can't be correct that each
has the same base?

I don't really like the tranche model as in the patch right now. I'd
rather have in a way that we have one tranch for all the individual
lwlocks, where the tranche points to an array of names alongside the
tranche's name. And then for the others we just supply the tranche name,
but leave the name array empty, whereas a name can be generated.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Petr Jelinek 2015-09-01 23:14:10 Re: Horizontal scalability/sharding
Previous Message Alvaro Herrera 2015-09-01 22:27:59 Re: WIP: About CMake v2