On Fri, Jan 10, 2025 at 01:46:53PM +0900, Michael Paquier wrote: > I'd rather use RecoveryInProgress() here even if XLogInsertAllowed() > is a synonym of that, minus the update of LocalXLogInsertAllowed for > the local process.
I've applied v2-0002 for the new header as it is useful on its own. Rebased to avoid the wrath of the CF bot, as v3. -- Michael
From 7d4aa6fc0e348065b46c31cbb21d7a4d2f21e5b2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 10 Jan 2025 13:26:46 +0900
Subject: [PATCH v3 3/4] Add RMGR and WAL-logging API for pgstats
This commit adds a new facility in the backend that offers to pgstats
kinds the possibility to WAL-log data that need to be persisted across
restarts:
- A new callback called redo_cb can be defined by a pgstats_kind.
- pgstat_xlog.c includes one API able to register some payload data and
its size. Stats kinds doing WAL-logging require a definition of
redo_cb.
- pg_waldump is able to show the data logged, as hexadedimal data.
This facility has applications for in-core and custom stats kinds, with
one primary case being the possibility to WAL-log some statistics
related to relations so as these are not lost post-crash. This is left
as future work.
---
src/include/access/rmgrlist.h | 1 +
src/include/utils/pgstat_internal.h | 8 +++
src/include/utils/pgstat_xlog.h | 41 ++++++++++++++
src/backend/access/rmgrdesc/Makefile | 1 +
src/backend/access/rmgrdesc/meson.build | 1 +
src/backend/access/rmgrdesc/pgstatdesc.c | 49 ++++++++++++++++
src/backend/access/transam/rmgr.c | 1 +
src/backend/utils/activity/Makefile | 1 +
src/backend/utils/activity/meson.build | 1 +
src/backend/utils/activity/pgstat_xlog.c | 72 ++++++++++++++++++++++++
src/bin/pg_waldump/.gitignore | 1 +
src/bin/pg_waldump/rmgrdesc.c | 1 +
src/bin/pg_waldump/t/001_basic.pl | 3 +-
src/tools/pgindent/typedefs.list | 1 +
14 files changed, 181 insertions(+), 1 deletion(-)
create mode 100644 src/include/utils/pgstat_xlog.h
create mode 100644 src/backend/access/rmgrdesc/pgstatdesc.c
create mode 100644 src/backend/utils/activity/pgstat_xlog.c
diff --git a/src/include/access/rmgrlist.h b/src/include/access/rmgrlist.h
index 8e7fc9db87..409397cb21 100644
--- a/src/include/access/rmgrlist.h
+++ b/src/include/access/rmgrlist.h
@@ -47,3 +47,4 @@ PG_RMGR(RM_COMMIT_TS_ID, "CommitTs", commit_ts_redo, commit_ts_desc, commit_ts_i
PG_RMGR(RM_REPLORIGIN_ID, "ReplicationOrigin", replorigin_redo, replorigin_desc, replorigin_identify, NULL, NULL, NULL, NULL)
PG_RMGR(RM_GENERIC_ID, "Generic", generic_redo, generic_desc, generic_identify, NULL, NULL, generic_mask, NULL)
PG_RMGR(RM_LOGICALMSG_ID, "LogicalMessage", logicalmsg_redo, logicalmsg_desc, logicalmsg_identify, NULL, NULL, NULL, logicalmsg_decode)
+PG_RMGR(RM_PGSTAT_ID, "PgStat", pgstat_redo, pgstat_desc, pgstat_identify, NULL, NULL, NULL, NULL)
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 4bb8e5c53a..7a87d4e2f6 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -258,6 +258,14 @@ typedef struct PgStat_KindInfo
*/
void (*init_backend_cb) (void);
+ /*
+ * Perform custom actions when replaying WAL related to a statistics kind.
+ *
+ * "data" is a pointer to the stats information that can be used by the
+ * stats kind at redo.
+ */
+ void (*redo_cb) (void *data, size_t data_size);
+
/*
* For variable-numbered stats: flush pending stats. Required if pending
* data is used. See flush_fixed_cb for fixed-numbered stats.
diff --git a/src/include/utils/pgstat_xlog.h b/src/include/utils/pgstat_xlog.h
new file mode 100644
index 0000000000..8e229b4f12
--- /dev/null
+++ b/src/include/utils/pgstat_xlog.h
@@ -0,0 +1,41 @@
+/* ----------
+ * pgstat_xlog.h
+ * Exports from utils/activity/pgstat_xlog.c
+ *
+ * Copyright (c) 2001-2025, PostgreSQL Global Development Group
+ *
+ * src/include/utils/pgstat_xlog.h
+ * ----------
+ */
+#ifndef PGSTAT_XLOG_H
+#define PGSTAT_XLOG_H
+
+#include "access/xlog.h"
+#include "access/xlogdefs.h"
+#include "access/xlogreader.h"
+#include "utils/pgstat_kind.h"
+
+/*
+ * Generic WAL record for pgstat data.
+ */
+typedef struct xl_pgstat_data
+{
+ PgStat_Kind stats_kind;
+ size_t data_size; /* size of the data */
+ /* stats data, worth data_size */
+ char data[FLEXIBLE_ARRAY_MEMBER];
+} xl_pgstat_data;
+
+#define SizeOfPgStatData (offsetof(xl_pgstat_data, data))
+
+extern XLogRecPtr pgstat_xlog_data(PgStat_Kind stats_kind,
+ const void *data,
+ size_t data_size);
+
+/* RMGR API */
+#define XLOG_PGSTAT_DATA 0x00
+extern void pgstat_redo(XLogReaderState *record);
+extern void pgstat_desc(StringInfo buf, XLogReaderState *record);
+extern const char *pgstat_identify(uint8 info);
+
+#endif /* PGSTAT_XLOG_H */
diff --git a/src/backend/access/rmgrdesc/Makefile b/src/backend/access/rmgrdesc/Makefile
index cd95eec37f..3979d22946 100644
--- a/src/backend/access/rmgrdesc/Makefile
+++ b/src/backend/access/rmgrdesc/Makefile
@@ -21,6 +21,7 @@ OBJS = \
logicalmsgdesc.o \
mxactdesc.o \
nbtdesc.o \
+ pgstatdesc.o \
relmapdesc.o \
replorigindesc.o \
rmgrdesc_utils.o \
diff --git a/src/backend/access/rmgrdesc/meson.build b/src/backend/access/rmgrdesc/meson.build
index 96c98e800c..91c53e634c 100644
--- a/src/backend/access/rmgrdesc/meson.build
+++ b/src/backend/access/rmgrdesc/meson.build
@@ -14,6 +14,7 @@ rmgr_desc_sources = files(
'logicalmsgdesc.c',
'mxactdesc.c',
'nbtdesc.c',
+ 'pgstatdesc.c',
'relmapdesc.c',
'replorigindesc.c',
'rmgrdesc_utils.c',
diff --git a/src/backend/access/rmgrdesc/pgstatdesc.c b/src/backend/access/rmgrdesc/pgstatdesc.c
new file mode 100644
index 0000000000..233f9f1994
--- /dev/null
+++ b/src/backend/access/rmgrdesc/pgstatdesc.c
@@ -0,0 +1,49 @@
+/*-------------------------------------------------------------------------
+ *
+ * pgstatdesc.c
+ * rmgr descriptor routines for utils/activity/pgstat_xlog.c
+ *
+ * Portions Copyright (c) 2001-2025, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/access/rmgrdesc/pgstatdesc.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "utils/pgstat_xlog.h"
+
+void
+pgstat_desc(StringInfo buf, XLogReaderState *record)
+{
+ char *rec = XLogRecGetData(record);
+ uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
+
+ if (info == XLOG_PGSTAT_DATA)
+ {
+ xl_pgstat_data *xlrec = (xl_pgstat_data *) rec;
+ char *sep = "";
+
+ appendStringInfo(buf, "stats kind \"%u\"; with data (%zu bytes): ",
+ xlrec->stats_kind, xlrec->data_size);
+
+ /* Write stats data as a series of hex bytes */
+ for (int cnt = 0; cnt < xlrec->data_size; cnt++)
+ {
+ appendStringInfo(buf, "%s%02X", sep, (unsigned char) xlrec->data[cnt]);
+ sep = " ";
+ }
+ }
+}
+
+const char *
+pgstat_identify(uint8 info)
+{
+ if ((info & ~XLR_INFO_MASK) == XLOG_PGSTAT_DATA)
+ return "PGSTAT_DATA";
+
+ return NULL;
+}
diff --git a/src/backend/access/transam/rmgr.c b/src/backend/access/transam/rmgr.c
index 1b7499726e..945843a78e 100644
--- a/src/backend/access/transam/rmgr.c
+++ b/src/backend/access/transam/rmgr.c
@@ -39,6 +39,7 @@
#include "replication/message.h"
#include "replication/origin.h"
#include "storage/standby.h"
+#include "utils/pgstat_xlog.h"
#include "utils/relmapper.h"
/* IWYU pragma: end_keep */
diff --git a/src/backend/utils/activity/Makefile b/src/backend/utils/activity/Makefile
index 9c2443e1ec..fdbdd6bfe6 100644
--- a/src/backend/utils/activity/Makefile
+++ b/src/backend/utils/activity/Makefile
@@ -33,6 +33,7 @@ OBJS = \
pgstat_subscription.o \
pgstat_wal.o \
pgstat_xact.o \
+ pgstat_xlog.o \
wait_event.o \
wait_event_funcs.o
diff --git a/src/backend/utils/activity/meson.build b/src/backend/utils/activity/meson.build
index d8e56b49c2..533efef2cb 100644
--- a/src/backend/utils/activity/meson.build
+++ b/src/backend/utils/activity/meson.build
@@ -18,6 +18,7 @@ backend_sources += files(
'pgstat_subscription.c',
'pgstat_wal.c',
'pgstat_xact.c',
+ 'pgstat_xlog.c',
)
# this includes a .c file with contents generated in ../../../include/activity,
diff --git a/src/backend/utils/activity/pgstat_xlog.c b/src/backend/utils/activity/pgstat_xlog.c
new file mode 100644
index 0000000000..59a06cd9e1
--- /dev/null
+++ b/src/backend/utils/activity/pgstat_xlog.c
@@ -0,0 +1,72 @@
+/* ----------
+ * pgstat_xlog.c
+ * WAL replay logic for cumulative statistics.
+ *
+ *
+ * Copyright (c) 2001-2025, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/backend/utils/activity/pgstat_xlog.c
+ * ----------
+ */
+
+#include "postgres.h"
+
+#include <unistd.h>
+
+#include "access/xlog.h"
+#include "access/xloginsert.h"
+#include "utils/pgstat_internal.h"
+#include "utils/pgstat_xlog.h"
+
+/*
+ * Write pgstats record data into WAL.
+ */
+XLogRecPtr
+pgstat_xlog_data(PgStat_Kind stats_kind, const void *data,
+ size_t data_size)
+{
+ xl_pgstat_data xlrec;
+ XLogRecPtr lsn;
+
+ xlrec.stats_kind = stats_kind;
+ xlrec.data_size = data_size;
+
+ XLogBeginInsert();
+ XLogRegisterData((char *) &xlrec, SizeOfPgStatData);
+ XLogRegisterData(data, data_size);
+
+ lsn = XLogInsert(RM_PGSTAT_ID, XLOG_PGSTAT_DATA);
+ return lsn;
+}
+
+/*
+ * Redo just passes down to the stats kind expected by this record
+ * data included in it.
+ */
+void
+pgstat_redo(XLogReaderState *record)
+{
+ uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
+ xl_pgstat_data *xlrec;
+ PgStat_Kind stats_kind;
+ const PgStat_KindInfo *kind_info;
+
+ if (info != XLOG_PGSTAT_DATA)
+ elog(PANIC, "pgstat_redo: unknown op code %u", info);
+
+ xlrec = (xl_pgstat_data *) XLogRecGetData(record);
+
+ stats_kind = xlrec->stats_kind;
+ kind_info = pgstat_get_kind_info(stats_kind);
+
+ if (kind_info == NULL)
+ elog(FATAL, "pgstat_redo: invalid stats kind found: kind=%u",
+ stats_kind);
+
+ if (kind_info->redo_cb == NULL)
+ elog(FATAL, "pgstat_redo: no redo callback found: kind=%s",
+ kind_info->name);
+
+ kind_info->redo_cb(xlrec->data, xlrec->data_size);
+}
diff --git a/src/bin/pg_waldump/.gitignore b/src/bin/pg_waldump/.gitignore
index ec51f41c76..e5089b322d 100644
--- a/src/bin/pg_waldump/.gitignore
+++ b/src/bin/pg_waldump/.gitignore
@@ -13,6 +13,7 @@
/logicalmsgdesc.c
/mxactdesc.c
/nbtdesc.c
+/pgstatdesc.c
/relmapdesc.c
/replorigindesc.c
/rmgrdesc_utils.c
diff --git a/src/bin/pg_waldump/rmgrdesc.c b/src/bin/pg_waldump/rmgrdesc.c
index 6b8c17bb4c..05b4f2acd1 100644
--- a/src/bin/pg_waldump/rmgrdesc.c
+++ b/src/bin/pg_waldump/rmgrdesc.c
@@ -30,6 +30,7 @@
#include "replication/origin.h"
#include "rmgrdesc.h"
#include "storage/standbydefs.h"
+#include "utils/pgstat_xlog.h"
#include "utils/relmapper.h"
#define PG_RMGR(symname,name,redo,desc,identify,startup,cleanup,mask,decode) \
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index 8d574a410c..70256ecccb 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -73,7 +73,8 @@ BRIN
CommitTs
ReplicationOrigin
Generic
-LogicalMessage$/,
+LogicalMessage
+PgStat$/,
'rmgr list');
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index eb93debe10..4db1e7cf4b 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -4135,6 +4135,7 @@ xl_multixact_create
xl_multixact_truncate
xl_overwrite_contrecord
xl_parameter_change
+xl_pgstat_data
xl_relmap_update
xl_replorigin_drop
xl_replorigin_set
--
2.47.1
From 397540085a0f176806aa36d0c8c85f8a6dbbf938 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 10 Jan 2025 13:36:28 +0900
Subject: [PATCH v3 4/4] injection_points: Add option and tests for WAL-logging
This serves as a template for WAL-logging related to pgstats, based on
the variable-numbered stats the module includes. Some TAP tests are
added to check the backend facility.
---
.../injection_points/injection_points.c | 19 ++++++-
.../injection_points/injection_stats.c | 54 +++++++++++++++++++
.../injection_points/injection_stats.h | 3 +-
.../modules/injection_points/t/001_stats.pl | 22 ++++++++
src/tools/pgindent/typedefs.list | 1 +
5 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index ad528d7775..c7f0d6d242 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -104,7 +104,7 @@ extern PGDLLEXPORT void injection_wait(const char *name,
static bool injection_point_local = false;
/*
- * GUC variable
+ * GUC variables
*
* This GUC is useful to control if statistics should be enabled or not
* during a test with injection points, like for example if a test relies
@@ -112,6 +112,12 @@ static bool injection_point_local = false;
*/
bool inj_stats_enabled = false;
+/*
+ * This GUC controls if statistics of injection points should be logged
+ * into WAL or not.
+ */
+bool inj_stats_wal_enabled = false;
+
/* Shared memory init callbacks */
static shmem_request_hook_type prev_shmem_request_hook = NULL;
static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
@@ -534,6 +540,17 @@ _PG_init(void)
NULL,
NULL);
+ DefineCustomBoolVariable("injection_points.log_stats",
+ "Enables WAL-logging for injection points statistics.",
+ NULL,
+ &inj_stats_wal_enabled,
+ false,
+ PGC_POSTMASTER,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+
MarkGUCPrefixReserved("injection_points");
/* Shared memory initialization */
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index 5db62bca66..df52cb026e 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -21,6 +21,7 @@
#include "pgstat.h"
#include "utils/builtins.h"
#include "utils/pgstat_internal.h"
+#include "utils/pgstat_xlog.h"
/* Structures for statistics of injection points */
typedef struct PgStat_StatInjEntry
@@ -34,6 +35,14 @@ typedef struct PgStatShared_InjectionPoint
PgStat_StatInjEntry stats;
} PgStatShared_InjectionPoint;
+/* Structure for data of injection points logged in WAL */
+typedef struct PgStat_StatInjRecord
+{
+ uint64 objid; /* hash of the point name */
+ PgStat_StatInjEntry entry; /* stats data */
+} PgStat_StatInjRecord;
+
+static void injection_stats_redo_cb(void *data, size_t data_size);
static bool injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
static const PgStat_KindInfo injection_stats = {
@@ -48,6 +57,7 @@ static const PgStat_KindInfo injection_stats = {
.shared_data_off = offsetof(PgStatShared_InjectionPoint, stats),
.shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
.pending_size = sizeof(PgStat_StatInjEntry),
+ .redo_cb = injection_stats_redo_cb,
.flush_pending_cb = injection_stats_flush_cb,
};
@@ -64,6 +74,31 @@ static const PgStat_KindInfo injection_stats = {
/* Track if stats are loaded */
static bool inj_stats_loaded = false;
+/*
+ * REDO callback for injection point stats.
+ */
+static void
+injection_stats_redo_cb(void *data, size_t data_size)
+{
+ PgStat_StatInjRecord *record_data = (PgStat_StatInjRecord *) data;
+ PgStat_StatInjEntry record_entry = record_data->entry;
+ PgStat_StatInjEntry *stat_entry;
+ PgStatShared_InjectionPoint *shstatent;
+ PgStat_EntryRef *entry_ref;
+
+ Assert(data_size == sizeof(PgStat_StatInjRecord));
+
+ /* create or fetch existing entry */
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_INJECTION, InvalidOid,
+ record_data->objid, NULL);
+
+ shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats;
+ stat_entry = &shstatent->stats;
+
+ /* Update the injection point statistics, overwriting any existing data */
+ *stat_entry = record_entry;
+}
+
/*
* Callback for stats handling
*/
@@ -72,6 +107,9 @@ injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
{
PgStat_StatInjEntry *localent;
PgStatShared_InjectionPoint *shfuncent;
+ PgStat_StatInjRecord record_data;
+
+ memset(&record_data, 0, sizeof(PgStat_StatInjRecord));
localent = (PgStat_StatInjEntry *) entry_ref->pending;
shfuncent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats;
@@ -81,8 +119,24 @@ injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
shfuncent->stats.numcalls += localent->numcalls;
+ record_data.objid = entry_ref->shared_entry->key.objid;
+ record_data.entry = shfuncent->stats;
+
pgstat_unlock_entry(entry_ref);
+ /*
+ * Store the data in WAL, if not in recovery and if the option is enabled.
+ */
+ if (!RecoveryInProgress() && inj_stats_wal_enabled)
+ {
+ XLogRecPtr lsn;
+
+ lsn = pgstat_xlog_data(PGSTAT_KIND_INJECTION, &record_data,
+ sizeof(PgStat_StatInjRecord));
+
+ /* Force a flush, to ensure persistency */
+ XLogFlush(lsn);
+ }
return true;
}
diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h
index ba310c52c7..baa8bb506c 100644
--- a/src/test/modules/injection_points/injection_stats.h
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -15,8 +15,9 @@
#ifndef INJECTION_STATS
#define INJECTION_STATS
-/* GUC variable */
+/* GUC variables */
extern bool inj_stats_enabled;
+extern bool inj_stats_wal_enabled;
/* injection_stats.c */
extern void pgstat_register_inj(void);
diff --git a/src/test/modules/injection_points/t/001_stats.pl b/src/test/modules/injection_points/t/001_stats.pl
index d4539fe872..4b34dd0a5a 100644
--- a/src/test/modules/injection_points/t/001_stats.pl
+++ b/src/test/modules/injection_points/t/001_stats.pl
@@ -75,4 +75,26 @@ $node->stop;
$node->adjust_conf('postgresql.conf', 'shared_preload_libraries', "''");
$node->start;
+# Stop the server and enable WAL, the stats are preserved after recovery.
+$node->stop;
+$node->append_conf(
+ 'postgresql.conf', qq(
+shared_preload_libraries = 'injection_points'
+injection_points.log_stats = true
+));
+$node->start;
+
+# Two calls, both WAL-logged.
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('stats-wal-notice', 'notice');");
+$node->safe_psql('postgres',
+ "SELECT injection_points_run('stats-wal-notice');");
+$node->safe_psql('postgres',
+ "SELECT injection_points_run('stats-wal-notice');");
+$node->stop('immediate');
+$node->start;
+$numcalls = $node->safe_psql('postgres',
+ "SELECT injection_points_stats_numcalls('stats-wal-notice');");
+is($numcalls, '2', 'number of stats after crash with WAL-logging enabled');
+
done_testing();
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 4db1e7cf4b..303a06b3fb 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2167,6 +2167,7 @@ PgStat_StatDBEntry
PgStat_StatFuncEntry
PgStat_StatInjEntry
PgStat_StatInjFixedEntry
+PgStat_StatInjRecord
PgStat_StatReplSlotEntry
PgStat_StatSubEntry
PgStat_StatTabEntry
--
2.47.1
signature.asc
Description: PGP signature
