From 9004abf6206e815d25b7b763c756cf97b457f3cd Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 1 Jul 2024 14:26:25 +0900 Subject: [PATCH] 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 --- src/backend/utils/activity/pgstat.c | 74 ++++++++++++++--------------- src/include/utils/pgstat_internal.h | 6 +++ 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 @@ -199,6 +199,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 -- 2.30.2