Re: shared-memory based stats collector - v70

Just skimming a bit here ...

On 2022-Apr-05, Andres Freund wrote:

> From 0532b869033595202d5797b148f22c61e4eb4969 Mon Sep 17 00:00:00 2001
> From: Andres Freund <andres@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"

> @@ -5302,7 +5317,9 @@ StartupXLOG(void)
>          performedWalRecovery = true;
>      }
>      else
> +    {
>          performedWalRecovery = false;
> +    }

Why? :-)

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.

(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.)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"How amazing is that? I call it a night and come back to find that a bug has
been identified and patched while I sleep."                (Robert Davidson)
               http://archives.postgresql.org/pgsql-sql/2006-03/msg00378.php