Date: Wed, 6 Apr 2022 13:31:31 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Andres Freund <andres@anarazel.de>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, melanieplageman@gmail.com,
	Justin Pryzby <pryzby@telsasoft.com>,
	Thomas Munro <thomas.munro@gmail.com>,
	"David G. Johnston" <david.g.johnston@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: shared-memory based stats collector - v70
Message-ID: <202204061131.lozz5n34d5w5@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220406030008.2qxipjxo776dwnqs@alap3.anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/202204061131.lozz5n34d5w5%40alvherre.pgsql>
Precedence: bulk

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