Re: shared-memory based stats collector

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: alvherre(at)2ndquadrant(dot)com
Cc: tomas(dot)vondra(at)2ndquadrant(dot)com, andres(at)anarazel(dot)de, ah(at)cybertec(dot)at, magnus(at)hagander(dot)net, robertmhaas(at)gmail(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)postgresql(dot)org
Subject: Re: shared-memory based stats collector
Date: 2018-11-09 08:33:15
Message-ID: 20181109.173315.29453727.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hello. This is rebased version.

At Thu, 8 Nov 2018 16:06:49 +0100, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote in <de249c3f-79c9-b75c-79a3-5e2d008548a8(at)2ndquadrant(dot)com>
> However a quite a few extensions in contrib seem are broken now. It
> seems fixing it is as simple as including the new bestatus.h next to
> pgstat.h.

The additional 0009 does that.

At Thu, 8 Nov 2018 12:39:41 -0300, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote in <20181108153941(dot)txjb6rg3y7q26ldm(at)alvherre(dot)pgsql>
> On 2018-Nov-08, Tomas Vondra wrote:
>
> > I'm not sure splitting the headers like this is needed, actually. It's true
> > we're replacing pgstat.c with something else, but it's still related to
> > stats, backing pg_stat_* system views etc. So I'd keep as much of the
> > definitions in pgstat.h, so that it's enough to include that one header
> > file. That would "unbreak" the extensions.
>
> pgstat.h includes a lot of other stuff that presumably isn't needed if
> all some .c wants is in bestatus.h, so my vote would be to make this
> change *if it's actually possible to do it*: you want the affected
> headers to compile standalone (use cpluspluscheck or similar to verify
> this), for one thing.

cpluspluscheck doesn't complain to this change. I'm afraid that I
didn't read you cleary, but I counted up files that needs only
pgstat/ only bestatus / both in $(TOPDIR). (containing contrib)

only (new) pgstat.h : 33
only bestatus.h : 47
both : 22

> > Renaming pgstat_report_* functions to bestatus_report_* seems unnecessary to
> > me too. The original names seem quite fine to me.
>
> Yeah, this probably keeps churn to a minimum.

Reverted the names. pgstat_intialize() and
pgstat_clear_snapshot() were splitted into two functions for each
pgstat.c and bestatus.c. (and pgstat_clear_snapstop() is calling
pgstat_bestatus_clear_snapshot()!, and the names of the functions
looks somewhat inconsistent. Will fix later.)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
v8-0001-sequential-scan-for-dshash.patch text/x-patch 10.6 KB
v8-0002-Add-conditional-lock-feature-to-dshash.patch text/x-patch 4.1 KB
v8-0003-Shared-memory-based-stats-collector.patch text/x-patch 212.4 KB
v8-0004-Make-archiver-process-an-auxiliary-process.patch text/x-patch 11.9 KB
v8-0005-Let-pg_stat_statements-not-to-use-PG_STAT_TMP_DIR.patch text/x-patch 1.9 KB
v8-0006-Remove-pg_stat_tmp-exclusion-from-pg_rewind.patch text/x-patch 1.1 KB
v8-0007-Documentation-update.patch text/x-patch 6.1 KB
v8-0008-Split-out-backend-status-monitor-part-from-pgstat.patch text/x-patch 173.1 KB
v8-0009-Fix-include-files-of-contribs.patch text/x-patch 1.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-11-09 08:55:05 Re: speeding up planning with partitions
Previous Message Amit Langote 2018-11-09 08:26:27 Re: speeding up planning with partitions