Use pgstat_kind_infos to read fixed shared statistics

Shared statistics with a fixed number of objects are read from the stats
file in pgstat_read_statsfile() using members of PgStat_ShmemControl and
following an order based on their PgStat_Kind value.

Instead of being explicit, this commit changes the stats read to iterate
over the pgstat_kind_infos array to find the memory locations to read
into, based on a new shared_ctl_off in PgStat_KindInfo that can be used
to define the position of this stats kind in shared memory.  This makes
the read logic simpler, and eases the introduction of future
improvements aimed at making this area more pluggable for external
modules.

Original idea suggested by Andres Freund.

Author: Tristan Partin
Reviewed-by: Andres Freund, Michael Paquier
Discussion: https://postgr.es/m/D12SQ7OYCD85.20BUVF3DWU5K7@neon.tech

2 files changed, 42 insertions, 38 deletions

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 44f0d3ede72..27783d004a5 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -341,6 +341,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, archiver),
+		.shared_data_off = offsetof(PgStatShared_Archiver, stats),
+		.shared_data_len = sizeof(((PgStatShared_Archiver *) 0)->stats),
+
 		.reset_all_cb = pgstat_archiver_reset_all_cb,
 		.snapshot_cb = pgstat_archiver_snapshot_cb,
 	},
@@ -350,6 +354,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, bgwriter),
+		.shared_data_off = offsetof(PgStatShared_BgWriter, stats),
+		.shared_data_len = sizeof(((PgStatShared_BgWriter *) 0)->stats),
+
 		.reset_all_cb = pgstat_bgwriter_reset_all_cb,
 		.snapshot_cb = pgstat_bgwriter_snapshot_cb,
 	},
@@ -359,6 +367,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, checkpointer),
+		.shared_data_off = offsetof(PgStatShared_Checkpointer, stats),
+		.shared_data_len = sizeof(((PgStatShared_Checkpointer *) 0)->stats),
+
 		.reset_all_cb = pgstat_checkpointer_reset_all_cb,
 		.snapshot_cb = pgstat_checkpointer_snapshot_cb,
 	},
@@ -368,6 +380,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, io),
+		.shared_data_off = offsetof(PgStatShared_IO, stats),
+		.shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats),
+
 		.reset_all_cb = pgstat_io_reset_all_cb,
 		.snapshot_cb = pgstat_io_snapshot_cb,
 	},
@@ -377,6 +393,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, slru),
+		.shared_data_off = offsetof(PgStatShared_SLRU, stats),
+		.shared_data_len = sizeof(((PgStatShared_SLRU *) 0)->stats),
+
 		.reset_all_cb = pgstat_slru_reset_all_cb,
 		.snapshot_cb = pgstat_slru_snapshot_cb,
 	},
@@ -386,6 +406,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, wal),
+		.shared_data_off = offsetof(PgStatShared_Wal, stats),
+		.shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats),
+
 		.reset_all_cb = pgstat_wal_reset_all_cb,
 		.snapshot_cb = pgstat_wal_snapshot_cb,
 	},
@@ -1519,47 +1543,21 @@ pgstat_read_statsfile(void)
 		format_id != PGSTAT_FILE_FORMAT_ID)
 		goto error;
 
-	/*
-	 * XXX: The following could now be generalized to just iterate over
-	 * pgstat_kind_infos instead of knowing about the different kinds of
-	 * stats.
-	 */
-
-	/*
-	 * Read archiver stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->archiver.stats))
-		goto error;
-
-	/*
-	 * Read bgwriter stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->bgwriter.stats))
-		goto error;
-
-	/*
-	 * Read checkpointer stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->checkpointer.stats))
-		goto error;
+	/* Read various stats structs with fixed number of objects */
+	for (int kind = PGSTAT_KIND_FIRST_VALID; kind <= PGSTAT_KIND_LAST; kind++)
+	{
+		char	   *ptr;
+		const PgStat_KindInfo *info = pgstat_get_kind_info(kind);
 
-	/*
-	 * Read IO stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->io.stats))
-		goto error;
+		if (!info->fixed_amount)
+			continue;
 
-	/*
-	 * Read SLRU stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->slru.stats))
-		goto error;
+		Assert(info->shared_ctl_off != 0);
 
-	/*
-	 * Read WAL stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->wal.stats))
-		goto error;
+		ptr = ((char *) shmem) + info->shared_ctl_off + info->shared_data_off;
+		if (!read_chunk(fpin, ptr, info->shared_data_len))
+			goto error;
+	}
 
 	/*
 	 * We found an existing statistics file. Read it and put all the hash
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index f6031995a9d..e21ef4e2c98 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -200,6 +200,12 @@ typedef struct PgStat_KindInfo
 	uint32		shared_size;
 
 	/*
+	 * The offset of the statistics struct in the containing shared memory
+	 * control structure PgStat_ShmemControl, for fixed-numbered statistics.
+	 */
+	uint32		shared_ctl_off;
+
+	/*
 	 * The offset/size of statistics inside the shared stats entry. Used when
 	 * [de-]serializing statistics to / from disk respectively. Separate from
 	 * shared_size because [de-]serialization may not include in-memory state