Re: shared-memory based stats collector

At Tue, 6 Apr 2021 09:32:16 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> Hi,
>
> On 2021-04-05 02:29:14 -0700, Andres Freund wrote:
..
> I'm inclined to push patches
> [PATCH v60 05/17] pgstat: split bgwriter and checkpointer messages.
> [PATCH v60 06/17] pgstat: Split out relation stats handling from AtEO[Sub]Xact_PgStat() etc.
> [PATCH v60 09/17] pgstat: xact level cleanups / consolidation.
> [PATCH v60 10/17] pgstat: Split different types of stats into separate files.
> [PATCH v60 12/17] pgstat: reorder file pgstat.c / pgstat.h contents.

FWIW..

05 is a straight forward code-rearrange and reasonable to apply.

06 is same as above and it seems to make things cleaner.

09 mainly adds ensure_tabtat_xact_level() to remove repeated code
blocks a straight-forward way. I wonder if
pgstat_xact_stack_level_get() might better be
pgstat_get_xact_stack_level(), but I'm fine with the name in the
patch.

10 I found that the kind in "pgstat_kind" meant the placeholder for
specific types. It looks good to separate them into smaller pieces.
It is also a simple rearrangement of code.

> pgstat.c is very long, and it's hard to find an order that makes sense
> and is likely to be maintained over time. Splitting the different

I deeply agree to "hard to find an order that makes sense".

12 I'm not sure how it looks after this patch (I failed to apply 09 at
my hand.), but it is also a simple rearrangement of code blocks.

> to v14. They're just moving things around, so are fairly low risk. But
> they're going to be a pain to maintain. And I think 10 and 12 make
> pgstat.c a lot easier to understand.

I think that pgstat.c doesn't get frequent back-patching. It seems to
me that at least 10 looks good.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center