Hi, On 2022-04-06 13:31:31 +0200, Alvaro Herrera wrote: > Just skimming a bit here ...
Thanks! > On 2022-Apr-05, Andres Freund wrote: > > > From 0532b869033595202d5797b148f22c61e4eb4969 Mon Sep 17 00:00:00 2001 > > From: Andres Freund <and...@anarazel.de> > > Date: Mon, 4 Apr 2022 16:53:16 -0700 > > Subject: [PATCH v70 10/27] pgstat: store statistics in shared memory. > > > + <entry><literal>PgStatsData</literal></entry> > > + <entry>Waiting fo shared memory stats data access</entry> > > + </row> > > Typo "fo" -> "for" Oh, oops. I had fixed that in the wrong patch. > > @@ -5302,7 +5317,9 @@ StartupXLOG(void) > > performedWalRecovery = true; > > } > > else > > + { > > performedWalRecovery = false; > > + } > > Why? :-) Damage from merging two commits yesterday. I'd left open where exactly we'd reset stats, with the "main commit" implementing the current behaviour more closely, and then a followup commit implementing something a bit better. Nobody seemed to argue for keeping the behaviour 1:1, so I merged them. Without removing the parens again :) > Why give pgstat_get_entry_ref the responsibility of initializing > created_entry to false? The vast majority of callers don't care about > that flag; it seems easier/cleaner to set it to false in > pgstat_init_function_usage (the only caller that cares that I could > find) before calling pgstat_prep_pending_entry. It's annoying to have to initialize it, I agree. But I think it's bugprone for the caller to know that it has to be pre-initialized to false. > (I suggest pgstat_prep_pending_entry should have a comment line stating > "*created_entry, if not NULL, is set true if the entry required to be > created.", same as pgstat_get_entry_ref.) Added something along those lines. Greetings, Andres Freund