Hi, On 2021-03-22 12:02:39 +0900, Kyotaro Horiguchi wrote: > At Mon, 22 Mar 2021 09:55:59 +0900 (JST), Kyotaro Horiguchi > <horikyota....@gmail.com> wrote in > > At Thu, 18 Mar 2021 01:47:20 -0700, Andres Freund <and...@anarazel.de> > > wrote in > > > Hi, > > > > > > On 2021-03-18 16:56:02 +0900, Kyotaro Horiguchi wrote: > > > > At Tue, 16 Mar 2021 10:27:55 +0900 (JST), Kyotaro Horiguchi > > > > <horikyota....@gmail.com> wrote in > > > > Rebased and fixed two bugs. Not addressed received comments in this > > > > version. > > > > > > Since I am heavily editing the code, could you submit "functional" > > > changes (as opposed to fixing rebase issues) as incremental patches? > > > > Oh.. please wait for.. a moment, maybe. > > This is that.
Thanks! That change shouldn't be necessary on my branch - I did something to fix this kind of problem too. I decided that there's no point in doing hash table lookups for the database: It's not going to change in the life of a backend. So there's now two static "pending" entries: One for the current DB, one for the shared DB. There's only one place that needed to change, pgstat_report_checksum_failures_in_db(), which now reports the changes directly instead of going via pending. I suspect we should actually do that with a number of other DB specific functions. Things like recovery conflicts, deadlocks, checksum failures imo should really not be delayed till later. And you should never have enough of them to make contention a concern. You can see a somewhat sensible list of changes from your v52 at https://github.com/anarazel/postgres/compare/master...shmstat-before-split-2021-03-22 (I did fix some of damage from rebase in a non-incremental way, of course) My branch: https://github.com/anarazel/postgres/tree/shmstat It would be cool if you could check if there any relevant things between v52 and v56 that I should include. I think a lot of the concerns I had with the patch are addressed at the end of my series of changes. Please let me know what you think. My next step is going to be to squash all my changes into the base patch, and try to extract all the things that I think can be independently committed, and to reduce unnecessary diff noise. Once that's done I plan to post that series to the list. TODO: - explain the design at the top of pgstat.c - figure out a way to deal with the different demands on stats consistency / efficiency - see how hard it'd be to not need collect_oids() - split pgstat.c - consider removing PgStatTypes and replacing it with the oid of the table the type of stats reside in. So PGSTAT_TYPE_DB would be DatabaseRelationId, PGSTAT_TYPE_TABLE would be RelationRelationId, ... I think that'd make the system more cleanly extensible going forward? - I'm not yet happy with the naming schemes in use in pgstat.c. I feel like I improved it a bunch, but it's not yet there. - the replication slot stuff isn't quite right in my branch - I still don't quite like the reset_offset stuff - I wonder if we can find something better there. And if not, whether we can deduplicate the code between functions like pgstat_fetch_stat_checkpointer() and pgstat_report_checkpointer(). At the very least it'll need a lot better comments. - bunch of FIXMEs / XXXs Greetings, Andres Freund
