Re: shared-memory based stats collector

From: Andres Freund <andres(at)anarazel(dot)de>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: masao(dot)fujii(at)oss(dot)nttdata(dot)com, gkokolatos(at)protonmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: shared-memory based stats collector
Date: 2021-04-06 16:32:16
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2021-04-05 02:29:14 -0700, Andres Freund wrote:
> I've spent most of the last 2 1/2 weeks on this now. Unfortunately I think
> that, while it has gotten a lot closer, it's still about a week's worth of
> work away from being committable.
> My main concerns are:
> - Test Coverage:
> I've added a fair bit of tests, but it's still pretty bad. There were a lot
> of easy-to-hit bugs in earlier versions that nevertheless passed the test
> just fine.
> Due to the addition of pg_stat_force_next_flush(), and that there's no need
> to wait for the stats collector to write out files, it's now a lot more
> realistic to have proper testing of a lot of the pgstat.c code.
> - Architectural Review
> I rejiggered the patchset pretty significantly, and I think it needs more
> review than I see as realistic in the next two days. In particular I don't
> think
> - Performance Testing
> I did a small amount, but given that this touches just about every query
> etc, I think that's not enough. My changes unfortunately are substantial
> enough to invalidate Horiguchi-san's earlier tests.
> - Currently there's a corner case in which function (but not table!) stats
> for a dropped function may not be removed. That possibly is not too bad,
> - Too many FIXMEs still open
> It is quite disappointing to not have the patch go into v14 :(. But I just
> don't quite see the path right now. But maybe I am just too tired right now,
> and it'll look better tomorrow (err today, in a few hours).
> [...]
> I now changed the split so that there is
> utils/activity/pgstat_{database,functions,global,relation}.c
> For me that makes the code a lot more readable. Before this change I found it
> really hard to know where code should best be put etc, or where to find
> code. I found it to be pretty nice to work with after the new split.

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.

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.


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Kazutaka Onishi 2021-04-06 16:44:53 Re: TRUNCATE on foreign table
Previous Message Andres Freund 2021-04-06 16:28:19 Re: subtransaction performance regression [kind of] due to snapshot caching