Subject: Add callback in pgstats for backend initialization

From: Michael Paquier <[email-masked]>
Date: Tue, 3 Sep 2024 10:52:20 +0900

Hi all,

Currently, the backend-level initialization of pgstats happens in
pgstat_initialize(), where we are using a shortcut for the WAL stats,
with pgstat_init_wal().

I'd like to propose to add a callback for that, so as in-core or
custom pgstats kinds can assign custom actions when initializing a
backend.  The use-case I had for this one are pretty close to what we
do for WAL, where we would rely on a difference of state depending on
what may have existed before reaching the initialization path.  So
this can offer more precision.  Another case, perhaps less
interesting, is to be able to initialize some static backend state.

I wanted to get that sent for the current commit fest, but did not get
back in time to it.  Anyway, here it is.  This gives the simple patch
attached.

Thanks,
--
Michael
From 1e41a5155fb689f1d4ae8a5cfb244b824e913bae Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Tue, 3 Sep 2024 10:38:15 +0900
Subject: [PATCH] Add callback for backend initialization in pgstats

This is currently used by the WAL stats, and is now made available for
others as well.
---
 src/include/utils/pgstat_internal.h     |  7 +++++++
 src/backend/utils/activity/pgstat.c     | 14 ++++++++++++--
 src/backend/utils/activity/pgstat_wal.c |  2 +-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index fb132e439d..53f7d5c98c 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -229,6 +229,12 @@ typedef struct PgStat_KindInfo
 	 */
 	uint32		pending_size;
 
+	/*
+	 * Perform custom actions when initializing a backend (standalone or under
+	 * postmaster). Optional.
+	 */
+	void		(*init_backend_cb) (void);
+
 	/*
 	 * For variable-numbered stats: flush pending stats. Required if pending
 	 * data is used.
@@ -676,6 +682,7 @@ extern bool pgstat_flush_wal(bool nowait);
 extern void pgstat_init_wal(void);
 extern bool pgstat_have_pending_wal(void);
 
+extern void pgstat_wal_init_backend_cb(void);
 extern void pgstat_wal_init_shmem_cb(void *stats);
 extern void pgstat_wal_reset_all_cb(TimestampTz ts);
 extern void pgstat_wal_snapshot_cb(void);
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index b2ca3f39b7..3b6983ff5d 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -441,6 +441,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_off = offsetof(PgStatShared_Wal, stats),
 		.shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats),
 
+		.init_backend_cb = pgstat_wal_init_backend_cb,
 		.init_shmem_cb = pgstat_wal_init_shmem_cb,
 		.reset_all_cb = pgstat_wal_reset_all_cb,
 		.snapshot_cb = pgstat_wal_snapshot_cb,
@@ -604,10 +605,19 @@ pgstat_initialize(void)
 
 	pgstat_attach_shmem();
 
-	pgstat_init_wal();
-
 	pgstat_init_snapshot_fixed();
 
+	/* Backend initialization callbacks */
+	for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
+	{
+		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
+
+		if (kind_info == NULL || kind_info->init_backend_cb == NULL)
+			continue;
+
+		kind_info->init_backend_cb();
+	}
+
 	/* Set up a process-exit hook to clean up */
 	before_shmem_exit(pgstat_shutdown_hook, 0);
 
diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index e2a3f6b865..8c19c3f2fd 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -138,7 +138,7 @@ pgstat_flush_wal(bool nowait)
 }
 
 void
-pgstat_init_wal(void)
+pgstat_wal_init_backend_cb(void)
 {
 	/*
 	 * Initialize prevWalUsage with pgWalUsage so that pgstat_flush_wal() can
-- 
2.45.2

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 3 Sep 2024 05:00:54 +0000

Hi,

> interesting, is to be able to initialize some static backend state.

I think the proposal makes sense and I can see the use cases too, so +1.

> This gives the simple patch
> attached.

Should we add a few words about this new callback (and the others in
PgStat_KindInfo while at it) in the "Custom Cumulative Statistics" section of
xfunc.sgml?

Regards,

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

---

From: Michael Paquier <[email-masked]>
Date: Tue, 3 Sep 2024 14:58:49 +0900

> PgStat_KindInfo while at it) in the "Custom Cumulative Statistics" section of

Not sure if it is worth having.  This adds extra maintenance and
there's already a mention of src/include/utils/pgstat_internal.h
telling where the callbacks are described.
--
Michael

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Wed, 04 Sep 2024 14:15:43 +0900 (JST)

At Tue, 3 Sep 2024 05:00:54 +0000, Bertrand Drouvot <[email-masked]> wrote in 
> I think the proposal makes sense and I can see the use cases too, so +1.

+1, too.

The name "init_backend" makes it sound like the function initializes
the backend. backend_init might be a better choice, but I'm not sure.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center