Date: Wed, 6 Nov 2024 13:51:02 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: per backend I/O statistics
Message-ID: <Zyt0Rh6Uwvf+mlA1@ip-10-97-1-34.eu-west-3.compute.internal>
References: <CAN55FZ0xEC6K=EqTwboHJtzZnENDoEwvWOerKkHbZwFT-cU6XQ@mail.gmail.com>
 <ZumKktat4RtHmxL/@ip-10-97-1-34.eu-west-3.compute.internal>
 <Zuz5iQ4AjcuOMx_w@paquier.xyz>
 <ZwOvzX219kDhEEMu@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZwS5H5Fu0LnSev90@paquier.xyz>
 <ZwVdt8/0tTu5pv8K@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZyMRJIbUpNPoCXUe@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZyibjiLgoLx+kS4n@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZypXy3fH/EyALaTR@ip-10-97-1-34.eu-west-3.compute.internal>
 <Zyqsm6ilLLADAGw2@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Zyqsm6ilLLADAGw2@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/Zyt0Rh6Uwvf%2BmlA1%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Wed, Nov 06, 2024 at 08:39:07AM +0900, Michael Paquier wrote:
> On Tue, Nov 05, 2024 at 05:37:15PM +0000, Bertrand Drouvot wrote:
> > I'm starting working on option 2, I think it will be easier to discuss with
> > a patch proposal to look at.
> > 
> > If in the meantime, one strongly disagree with option 2 (means implement a brand
> > new PGSTAT_KIND_BACKEND and keep PGSTAT_KIND_IO), please let me know.
> 
> Sorry for the late reply, catching up a bit.

No problem at all, thanks for looking at it!

> As you are quoting in [1], you do not expect the backend-io stats and
> the more global pg_stat_io to achieve the same level of consistency as
> the backend stats would be gone at restart, and wiped out when a
> backend shuts down.

Yes.

> So, splitting them with a different stats kind
> feels more natural because it would be possible to control how each
> stat kind behaves depending on the code shutdown and reset paths
> within their own callbacks rather than making the callbacks of
> PGSTAT_KIND_IO more complex than they already are.

Yeah, thanks for sharing your thoughts.

> And pg_stat_io is
> a fixed-numbered stats kind because of the way it aggregates its stats
> with a number states defined at compile-time.
> 
> Is the structure you have in mind different than PgStat_BktypeIO?

Very close.

> Perhaps a split is better anyway with that in mind.

The in-progress patch (not shared yet) is using the following:

"
typedef struct PgStat_Backend
{
       TimestampTz stat_reset_timestamp;
       BackendType bktype;
       PgStat_BktypeIO stats;
} PgStat_Backend;
"

The bktype is used to be able to filter the stats correctly when we display them.

> The amount of memory required to store the snapshots of backend-IO
> does not worry me much, TBH, but you are worried about a high turnover
> of connections that could cause a lot of bloat in the backend-IO
> snapshots because of the persistency that these stats would have,
> right?

Not only a high turnover but also a high number of entries created in the hash.

Furthermore I don't see any use case of relying on stats_fetch_consistency
while querying other backend's stats.

> If possible, supporting snapshots would be
> more consistent with the other stats.

I have I mind to support the snapshots _only_ when querying our own stats. I can
measure the memory impact if we use them also when querying other backends stats
too (though I don't see a use case).

> Just to be clear, I am not in favor of making PgStat_HashKey larger
> than it already is.

That's not needed, the patch I'm working on stores the proc number in the
objid field of the key.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com