public inbox for [email protected]  
help / color / mirror / Atom feed
From: Sami Imseih <[email protected]>
To: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Date: Tue, 2 Dec 2025 14:58:32 -0600
Message-ID: <CAA5RZ0ufds2r3H7QUro_yJDrnUmsQ4ocufqgcsRGoZvxQKeMrA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAA5RZ0s9SDOu+Z6veoJCHWk+kDeTktAtC-KY9fQ9Z6BJdDUirQ@mail.gmail.com>
	<[email protected]>
	<CAA5RZ0ux07=E97mJ3JCy2jhs349NKzj7SPJNbDWZ11m9BDkTQQ@mail.gmail.com>
	<CAA5RZ0tnopAS4MCcoxXdJShYoGxYZgsoDXCjcie0Q-2TBPuJwA@mail.gmail.com>
	<[email protected]>
	<CAA5RZ0skT-uWwgrCg3Hj3AQaBbQdN0h72scD9GBLxH1nEKSNwQ@mail.gmail.com>
	<CAA5RZ0tH6ih3anQ=Lcwttm+8BURJ8VkBoy_9V8Ty73ZA1r384Q@mail.gmail.com>
	<[email protected]>
	<CAA5RZ0tvDvHYYLnu-T0DRK0xv1qv2_EK1yHjth5uAqDWz_GvfA@mail.gmail.com>
	<CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
	<[email protected]>

> > Also, I am now leaning towards creating a separate test module rather than
> > trying to do too much unrelated testing in injection points. It is definitely
> > convenient to use injection points, but I think we can do better testing with
> > a separate module. This module can also serve as an example for extension
> > developers.
>
> You are right that it may be cleaner this way.  Do you think that it
> could make sense to move some of the existing "template" code of
> injection_points there?

By "template" code, do you mean Something like?

include/utils/custom_statkinds.h
backend/utils/misc/custom_statkinds.c

Where the template code here is PgStat_kind definition, callbacks, etc. for
injection_points or the new test module that is using a custom kind.

A few benefits I see for this is we can point extension developers to
this as an example in [0] and we are also maintaining the kind ids in
a single place. These may not be strong points, but may be worth while.

v2 attached is something that may be closer to what we've been discussing

v2-0001 are much simplified changes to pgstat.c that simply invoke the callbacks
and all the work is on the extension to implement what it needs to do.
This includes
a callback at the end of WRITE, READ, DISCARD with a flag passed to the caller
so they can perform the necessary clean-up actions.

v2-0002 implements a new test module that tests mainly that the recovery,
clean and crash, are working as expected.

I created a new tap test for this which performs a test similar to what is
done in recovery/029_stats_restart.pl. I could merge the new test there, but
I am reluctant to add a dependency on a new module to recovery. What
do you think?

[0] https://www.postgresql.org/docs/current/xfunc-c.html#XFUNC-ADDIN-CUSTOM-CUMULATIVE-STATISTICS


--
Sami Imseih
Amazon Web Services (AWS)


Attachments:

  [application/octet-stream] v2-0002-Tests-for-custom-stat-kinds.patch (27.9K, ../CAA5RZ0ufds2r3H7QUro_yJDrnUmsQ4ocufqgcsRGoZvxQKeMrA@mail.gmail.com/2-v2-0002-Tests-for-custom-stat-kinds.patch)
  download | inline diff:
From 50fb48cfce161438ad9d3f8d39ffe0d4c7d542a4 Mon Sep 17 00:00:00 2001
From: Ubuntu <[email protected]>
Date: Tue, 2 Dec 2025 17:00:04 +0000
Subject: [PATCH v2 2/2] Tests for custom stat kinds

Creates a new test module to test custom stat
kinds. This also updates documentation to use
this module as an example for extension
developers that use custom stat kinds.
---
 doc/src/sgml/xfunc.sgml                       |   6 +-
 src/test/modules/Makefile                     |   1 +
 src/test/modules/meson.build                  |   1 +
 .../modules/test_custom_statkind/.gitignore   |   4 +
 .../modules/test_custom_statkind/Makefile     |  23 +
 .../modules/test_custom_statkind/meson.build  |  34 ++
 .../test_custom_statkind/t/001_custom_stat.pl |  92 +++
 .../test_custom_statkind--1.0.sql             |  19 +
 .../test_custom_statkind.c                    | 570 ++++++++++++++++++
 .../test_custom_statkind.control              |   4 +
 10 files changed, 753 insertions(+), 1 deletion(-)
 create mode 100644 src/test/modules/test_custom_statkind/.gitignore
 create mode 100644 src/test/modules/test_custom_statkind/Makefile
 create mode 100644 src/test/modules/test_custom_statkind/meson.build
 create mode 100644 src/test/modules/test_custom_statkind/t/001_custom_stat.pl
 create mode 100644 src/test/modules/test_custom_statkind/test_custom_statkind--1.0.sql
 create mode 100644 src/test/modules/test_custom_statkind/test_custom_statkind.c
 create mode 100644 src/test/modules/test_custom_statkind/test_custom_statkind.control

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 537ee6fa254..d50626523f2 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3967,6 +3967,9 @@ static const PgStat_KindInfo custom_stats = {
     .shared_data_off = offsetof(PgStatShared_Custom, stats),
     .shared_data_len = sizeof(((PgStatShared_Custom *) 0)->stats),
     .pending_size = sizeof(PgStat_StatCustomEntry),
+    .to_serialized_extra_stats = custom_stats_serialize,
+    .from_serialized_extra_stats = custom_stats_deserialize,
+    .end_extra_stats = custom_stats_file_cleanup,
 }
 </programlisting>
 
@@ -4005,7 +4008,8 @@ extern PgStat_Kind pgstat_register_kind(PgStat_Kind kind,
 
     <para>
      An example describing how to register and use custom statistics can be
-     found in <filename>src/test/modules/injection_points</filename>.
+     found in <filename>src/test/modules/test_custom_statkind</filename> and
+     <filename>src/test/modules/injection_points</filename>
     </para>
    </sect2>
 
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index d079b91b1a2..d35e83296e6 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -21,6 +21,7 @@ SUBDIRS = \
 		  test_bloomfilter \
 		  test_copy_callbacks \
 		  test_custom_rmgrs \
+		  test_custom_statkind \
 		  test_ddl_deparse \
 		  test_dsa \
 		  test_dsm_registry \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index f5114469b92..4f19298b6ec 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -20,6 +20,7 @@ subdir('test_bitmapset')
 subdir('test_bloomfilter')
 subdir('test_copy_callbacks')
 subdir('test_custom_rmgrs')
+subdir('test_custom_statkind')
 subdir('test_ddl_deparse')
 subdir('test_dsa')
 subdir('test_dsm_registry')
diff --git a/src/test/modules/test_custom_statkind/.gitignore b/src/test/modules/test_custom_statkind/.gitignore
new file mode 100644
index 00000000000..5dcb3ff9723
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_custom_statkind/Makefile b/src/test/modules/test_custom_statkind/Makefile
new file mode 100644
index 00000000000..42f48e9c8f4
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/Makefile
@@ -0,0 +1,23 @@
+# src/test/modules/test_custom_statkind/Makefile
+
+MODULE_big = test_custom_statkind
+OBJS = \
+	$(WIN32RES) \
+	test_custom_statkind.o
+PGFILEDESC = "test_custom_statkind - test code for custom stat kinds"
+
+EXTENSION = test_custom_statkind
+DATA = test_custom_statkind--1.0.sql
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_custom_statkind
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_custom_statkind/meson.build b/src/test/modules/test_custom_statkind/meson.build
new file mode 100644
index 00000000000..50dc4dfd387
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/meson.build
@@ -0,0 +1,34 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+test_custom_statkind_sources = files(
+  'test_custom_statkind.c',
+)
+
+if host_system == 'windows'
+  test_custom_statkind_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'test_custom_statkind',
+    '--FILEDESC', 'test_custom_statkind - test code for custom stat kinds',])
+endif
+
+test_custom_statkind = shared_module('test_custom_statkind',
+  test_custom_statkind_sources,
+  kwargs: pg_test_mod_args,
+)
+test_install_libs += test_custom_statkind
+
+test_install_data += files(
+  'test_custom_statkind.control',
+  'test_custom_statkind--1.0.sql',
+)
+
+tests += {
+  'name': 'test_custom_statkind',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+    'tests': [
+      't/001_custom_stat.pl',
+    ],
+    'runningcheck': false,
+  },
+}
diff --git a/src/test/modules/test_custom_statkind/t/001_custom_stat.pl b/src/test/modules/test_custom_statkind/t/001_custom_stat.pl
new file mode 100644
index 00000000000..00327407292
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/t/001_custom_stat.pl
@@ -0,0 +1,92 @@
+# Copyright (c) 2021-2025, PostgreSQL Global Development Group
+
+#
+# Test custom statistics functionality
+#
+# This test verifies that custom statistics entries can be created,
+# incremented, reported, and persisted across server restarts.
+#
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use File::Copy;
+
+# Create a new PostgreSQL test cluster
+my $node = PostgreSQL::Test::Cluster->new('main');
+
+# Initialize the cluster and configure it to load our test module
+$node->init;
+$node->append_conf('postgresql.conf',
+	"shared_preload_libraries = test_custom_statkind");
+$node->start;
+
+# Create the extension to enable custom statistics functions
+$node->safe_psql('postgres', q(CREATE EXTENSION test_custom_statkind));
+
+# Create two custom statistics entries with descriptions
+$node->safe_psql('postgres', q(select pgstat_create_custom_stat('entry1', 'this is the description for entry1')));
+$node->safe_psql('postgres', q(select pgstat_create_custom_stat('entry2', 'this is the description for entry2')));
+
+# Increment the statistics counters:
+# entry1: 2 calls, entry2: 3 calls
+$node->safe_psql('postgres', q(select pgstat_call_custom_stat('entry1')));
+$node->safe_psql('postgres', q(select pgstat_call_custom_stat('entry1')));
+$node->safe_psql('postgres', q(select pgstat_call_custom_stat('entry2')));
+$node->safe_psql('postgres', q(select pgstat_call_custom_stat('entry2')));
+$node->safe_psql('postgres', q(select pgstat_call_custom_stat('entry2')));
+
+# Verify that the custom statistics are correctly stored and reported
+my $result = $node->safe_psql('postgres', q(select * from pgstat_report_custom_stat('entry1')));
+is($result, "entry1|this is the description for entry1|2", "entry1 should have 2 calls with description");
+
+$result = $node->safe_psql('postgres', q(select * from pgstat_report_custom_stat('entry2')));
+is($result, "entry2|this is the description for entry2|3", "entry2 should have 3 calls with description");
+
+# Test persistence across server restart
+# Perform a clean shutdown to ensure statistics are written to disk
+$node->stop();
+
+# Create a backup of the statistics file for later testing
+my $statsfile = $PostgreSQL::Test::Utils::tmp_check . '/' . "discard_custom_stats";
+ok(!-f "$statsfile", "backup statsfile should not already exist");
+
+# Locate and backup the original statistics file
+my $datadir = $node->data_dir();
+my $og_stats = "$datadir/pg_stat/test_custom_statkind.stat";
+ok(-f "$og_stats", "original stats file should exist after shutdown");
+copy($og_stats, $statsfile) or die "Copy failed: $!";
+
+# Restart the server to test statistics persistence
+$node->start();
+
+# Verify that statistics persisted across the restart
+$result = $node->safe_psql('postgres', q(select * from pgstat_report_custom_stat('entry1')));
+is($result, "entry1|this is the description for entry1|2", "entry1 stats should persist after restart");
+
+$result = $node->safe_psql('postgres', q(select * from pgstat_report_custom_stat('entry2')));
+is($result, "entry2|this is the description for entry2|3", "entry2 stats should persist after restart");
+
+# Test statistics reset behavior
+# Perform an immediate shutdown (simulates crash) to prevent stats writing
+$node->stop('immediate');
+
+# Restore the backed up statistics file and restart
+copy($statsfile, $og_stats) or die "Copy failed: $!";
+$node->start;
+
+# After immediate shutdown, the stats file should be cleaned up
+ok(!-f "$og_stats", "stats file should be removed after immediate shutdown recovery");
+
+# Verify that statistics are reset (no descriptions, zero counts)
+$result = $node->safe_psql('postgres', q(select * from pgstat_report_custom_stat('entry1')));
+is($result, "entry1||0", "entry1 should be reset after stats file cleanup");
+
+$result = $node->safe_psql('postgres', q(select * from pgstat_report_custom_stat('entry2')));
+is($result, "entry2||0", "entry2 should be reset after stats file cleanup");
+
+# Test completed successfully
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_custom_statkind/test_custom_statkind--1.0.sql b/src/test/modules/test_custom_statkind/test_custom_statkind--1.0.sql
new file mode 100644
index 00000000000..8b085206ac8
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/test_custom_statkind--1.0.sql
@@ -0,0 +1,19 @@
+/* src/test/modules/test_custom_statkind/test_custom_statkind--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_custom_statkind" to load this file. \quit
+
+CREATE FUNCTION pgstat_create_custom_stat(IN name TEXT, IN description text)
+RETURNS void
+AS 'MODULE_PATHNAME', 'pgstat_create_custom_stat'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
+CREATE FUNCTION pgstat_call_custom_stat(IN name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'pgstat_call_custom_stat'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
+CREATE FUNCTION pgstat_report_custom_stat(INOUT name TEXT , OUT description TEXT, OUT calls BIGINT)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'pgstat_report_custom_stat'
+LANGUAGE C STRICT PARALLEL UNSAFE;
diff --git a/src/test/modules/test_custom_statkind/test_custom_statkind.c b/src/test/modules/test_custom_statkind/test_custom_statkind.c
new file mode 100644
index 00000000000..bc9077f80a6
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/test_custom_statkind.c
@@ -0,0 +1,570 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_custom_statkind.c
+ *		Test module for pgstats
+ *
+ * Copyright (c) 2024-2025, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		src/test/modules/test_custom_statkind/test_custom_statkind.c
+ *
+ * -------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "common/hashfn.h"
+#include "funcapi.h"
+#include "pgstat.h"
+#include "storage/dsm_registry.h"
+#include "utils/builtins.h"
+#include "utils/guc.h"
+#include "utils/pgstat_internal.h"
+
+PG_MODULE_MAGIC_EXT(
+					.name = "test_custom_statkind",
+					.version = PG_VERSION
+);
+
+/*--------------------------------------------------------------------------
+ * Macros and constants
+ *--------------------------------------------------------------------------
+ */
+
+/*
+ * Kind ID reserved for test_custom_statkind. This re-uses the same ID as
+ * Injection points to avoid reserving another kind id.
+ */
+#define PGSTAT_KIND_TEST_CUSTOM_STATKIND 25
+
+/*
+ * Compute statistics entry index from point name using an 8-byte hash.
+ */
+#define PGSTAT_CUSTOM_STAT_IDX(name) hash_bytes_extended((const unsigned char *) name, strlen(name), 0)
+#define PGSTAT_CUSTOM_EXTRA_DATA_DESC "pg_stat/test_custom_statkind.stat"
+
+/*--------------------------------------------------------------------------
+ * Type definitions
+ *--------------------------------------------------------------------------
+ */
+
+/* Local statistics entry for pending data */
+typedef struct PgStat_StatCustomEntry
+{
+	PgStat_Counter numcalls;	/* number of times the entry has been looked
+								 * up */
+}			PgStat_StatCustomEntry;
+
+/* Shared memory statistics entry */
+typedef struct PgStatShared_CustomEntry
+{
+	PgStatShared_Common header;
+	PgStat_StatCustomEntry stats;
+	char		name[NAMEDATALEN];
+	dsa_pointer description;
+}			PgStatShared_CustomEntry;
+
+/*--------------------------------------------------------------------------
+ * Global variables
+ *--------------------------------------------------------------------------
+ */
+
+/* Flag indicating if custom statistics kind is loaded */
+static bool pgstat_custom_kind_loaded = false;
+
+/* File handle for statistics serialization */
+static FILE *fd = NULL;
+
+/* DSA area to store custom statistics descriptions */
+dsa_area   *custom_stats_description_dsa = NULL;
+
+/*--------------------------------------------------------------------------
+ * Function prototypes
+ *--------------------------------------------------------------------------
+ */
+
+/* Statistics callback functions */
+static bool pgstat_custom_entry_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+static void pgstat_custom_entry_extra_serialize(const PgStat_HashKey *key,
+												const PgStatShared_Common *header, FILE *statfile);
+static void pgstat_custom_entry_extra_deserialize(const PgStat_HashKey *key,
+												  const PgStatShared_Common *header, FILE *statfile);
+static void pgstat_custom_entry_end_extra(PgStat_StatsFileOp status);
+
+/*--------------------------------------------------------------------------
+ * Custom kind configuration
+ *--------------------------------------------------------------------------
+ */
+
+static const PgStat_KindInfo custom_stats = {
+	.name = "test_custom_pgstat",
+	.fixed_amount = false,		/* Bounded by the number of points */
+	.write_to_file = true,
+	.track_entry_count = true,
+	.accessed_across_databases = true,	/* System-wide statistics */
+	.shared_size = sizeof(PgStatShared_CustomEntry),
+	.shared_data_off = offsetof(PgStatShared_CustomEntry, stats),
+	.shared_data_len = sizeof(((PgStatShared_CustomEntry *) 0)->stats),
+	.pending_size = sizeof(PgStat_StatCustomEntry),
+	.flush_pending_cb = pgstat_custom_entry_flush_cb,
+	.to_serialized_extra_stats = pgstat_custom_entry_extra_serialize,
+	.from_serialized_extra_stats = pgstat_custom_entry_extra_deserialize,
+	.end_extra_stats = pgstat_custom_entry_end_extra,
+};
+
+/*--------------------------------------------------------------------------
+ * Module initialization
+ *--------------------------------------------------------------------------
+ */
+
+void
+_PG_init(void)
+{
+	if (!process_shared_preload_libraries_in_progress)
+		return;
+
+	pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_STATKIND, &custom_stats);
+
+	pgstat_custom_kind_loaded = true;
+}
+
+/*--------------------------------------------------------------------------
+ * Statistics callback functions
+ *--------------------------------------------------------------------------
+ */
+
+/*
+ * pgstat_custom_entry_flush_cb() -
+ *
+ * Flush callback for custom statistics entries. Merges pending statistics
+ * data from local memory into shared memory.
+ *
+ * Returns true if the flush was successful, false if we couldn't acquire
+ * the necessary locks (when nowait is true).
+ */
+static bool
+pgstat_custom_entry_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+{
+	PgStat_StatCustomEntry *localent;
+	PgStatShared_CustomEntry *shfuncent;
+
+	localent = (PgStat_StatCustomEntry *) entry_ref->pending;
+	shfuncent = (PgStatShared_CustomEntry *) entry_ref->shared_stats;
+
+	if (!pgstat_lock_entry(entry_ref, nowait))
+		return false;
+
+	shfuncent->stats.numcalls += localent->numcalls;
+
+	pgstat_unlock_entry(entry_ref);
+
+	return true;
+}
+
+/*
+ * pgstat_custom_entry_extra_serialize() -
+ *
+ * Serialize extra data (descriptions) for custom statistics entries to
+ * the statistics file. Called during statistics file writing to preserve
+ * description strings across restarts.
+ */
+static void
+pgstat_custom_entry_extra_serialize(const PgStat_HashKey *key,
+									const PgStatShared_Common *header, FILE *statfile)
+{
+	char	   *description;
+	size_t		qlen;
+	PgStatShared_CustomEntry *entry = (PgStatShared_CustomEntry *) header;
+	bool		found;
+
+	if (!pgstat_custom_kind_loaded)
+		return;
+
+	if (!custom_stats_description_dsa)
+		custom_stats_description_dsa = GetNamedDSA("pgstat_custom_stat_dsa", &found);
+
+	/* Exit early if DSA is not available */
+	if (!custom_stats_description_dsa)
+		return;
+
+	/* Open statistics file for writing if not already open */
+	if (!fd)
+	{
+		fd = AllocateFile(PGSTAT_CUSTOM_EXTRA_DATA_DESC, PG_BINARY_W);
+		if (fd == NULL)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					 errmsg("could not open statistics file \"%s\" for writing: %m",
+							PGSTAT_CUSTOM_EXTRA_DATA_DESC)));
+			return;
+		}
+	}
+
+	/* Write the hash key to identify this entry */
+	pgstat_write_chunk(fd, (void *) key, sizeof(PgStat_HashKey));
+
+	/* Handle entries without descriptions */
+	if (!DsaPointerIsValid(entry->description))
+	{
+		fputc('\0', fd);		/* Write null terminator for empty description */
+		return;
+	}
+
+	if (!custom_stats_description_dsa)
+	{
+		fputc('\0', fd);		/* Write null terminator if DSA unavailable */
+		return;
+	}
+
+	/* Retrieve description from DSA and write to file */
+	description = dsa_get_address(custom_stats_description_dsa, entry->description);
+	qlen = strlen(description) + 1; /* include null terminator */
+
+	pgstat_write_chunk(fd, description, qlen);
+}
+
+/*
+ * pgstat_custom_entry_extra_deserialize() -
+ *
+ * Deserialize extra data (descriptions) for custom statistics entries from
+ * the statistics file. Called during statistics file reading to restore
+ * description strings after a restart.
+ */
+static void
+pgstat_custom_entry_extra_deserialize(const PgStat_HashKey *key,
+									  const PgStatShared_Common *header, FILE *statfile)
+{
+	PgStatShared_CustomEntry *entry;
+	dsa_pointer dp;
+	size_t		bufsize;
+	size_t		len;
+	char	   *buffer;
+	int			c;
+	bool		found;
+
+	if (!pgstat_custom_kind_loaded)
+		return;
+
+	/* Initialize DSA if needed */
+	if (!custom_stats_description_dsa)
+		custom_stats_description_dsa = GetNamedDSA("pgstat_custom_stat_dsa", &found);
+
+	if (!custom_stats_description_dsa)
+		return;
+
+	/* Open statistics file for reading if not already open */
+	if (!fd)
+	{
+		fd = AllocateFile(PGSTAT_CUSTOM_EXTRA_DATA_DESC, PG_BINARY_R);
+		if (fd == NULL)
+		{
+			if (errno != ENOENT)
+				ereport(LOG,
+						(errcode_for_file_access(),
+						 errmsg("could not open statistics file \"%s\" for reading: %m",
+								PGSTAT_CUSTOM_EXTRA_DATA_DESC)));
+			/* Reset statistics if file is missing or unreadable */
+			pgstat_reset_of_kind(PGSTAT_KIND_TEST_CUSTOM_STATKIND);
+			return;
+		}
+	}
+
+	/* Read and verify the hash key */
+	if (!pgstat_read_chunk(fd, (void *) key, sizeof(PgStat_HashKey)))
+		return;
+
+	entry = (PgStatShared_CustomEntry *) header;
+
+	/* Read null-terminated description string from file */
+	bufsize = 128;
+	len = 0;
+	buffer = palloc(bufsize);
+
+	/* Read description character by character until null terminator */
+	while ((c = fgetc(fd)) != EOF)
+	{
+		/* Expand buffer if needed */
+		if (len + 1 >= bufsize)
+		{
+			bufsize *= 2;
+			buffer = repalloc(buffer, bufsize);
+		}
+
+		buffer[len++] = (char) c;
+
+		if (c == '\0')
+			break;
+	}
+
+	/* Handle unexpected EOF */
+	if (c == EOF)
+	{
+		pfree(buffer);
+		return;
+	}
+
+	/* Allocate space in DSA and copy the description */
+	dp = dsa_allocate(custom_stats_description_dsa, len);
+
+	memcpy(dsa_get_address(custom_stats_description_dsa, dp), buffer, len);
+	entry->description = dp;
+
+	pfree(buffer);
+}
+
+/*
+ * pgstat_custom_entry_end_extra() -
+ *
+ * Cleanup function called at the end of statistics file operations.
+ * Handles closing files and cleanup based on the operation type.
+ */
+static void
+pgstat_custom_entry_end_extra(PgStat_StatsFileOp status)
+{
+	if (!pgstat_custom_kind_loaded)
+		return;
+
+	/* Handle cleanup after writing statistics */
+	if (status == STATS_WRITE)
+	{
+		if (!fd)
+			return;
+
+		/* Check for write errors and cleanup if necessary */
+		if (ferror(fd))
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					 errmsg("could not write statistics file \"%s\": %m",
+							PGSTAT_CUSTOM_EXTRA_DATA_DESC)));
+			FreeFile(fd);
+			unlink(PGSTAT_CUSTOM_EXTRA_DATA_DESC);
+		}
+		else if (FreeFile(fd) < 0)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					 errmsg("could not close statistics file \"%s\": %m",
+							PGSTAT_CUSTOM_EXTRA_DATA_DESC)));
+			unlink(PGSTAT_CUSTOM_EXTRA_DATA_DESC);
+		}
+	}
+	/* Handle cleanup after reading statistics */
+	else if (status == STATS_READ)
+	{
+		if (!fd)
+			return;
+
+		FreeFile(fd);
+
+		/* Remove the temporary statistics file after reading */
+		elog(DEBUG2, "removing statistics file \"%s\"", PGSTAT_CUSTOM_EXTRA_DATA_DESC);
+		unlink(PGSTAT_CUSTOM_EXTRA_DATA_DESC);
+	}
+	/* Handle other cleanup operations (reset, etc.) */
+	else
+	{
+		int			ret;
+
+		/* Attempt to remove the statistics file */
+		ret = unlink(PGSTAT_CUSTOM_EXTRA_DATA_DESC);
+		if (ret != 0)
+		{
+			if (errno == ENOENT)
+				elog(LOG,
+					 "didn't need to unlink permanent stats file \"%s\" - didn't exist",
+					 PGSTAT_CUSTOM_EXTRA_DATA_DESC);
+			else
+				ereport(LOG,
+						(errcode_for_file_access(),
+						 errmsg("could not unlink permanent statistics file \"%s\": %m",
+								PGSTAT_CUSTOM_EXTRA_DATA_DESC)));
+		}
+		else
+		{
+			ereport(LOG,
+					(errmsg_internal("unlinked permanent statistics file \"%s\"",
+									 PGSTAT_CUSTOM_EXTRA_DATA_DESC)));
+		}
+	}
+
+	fd = NULL;
+}
+
+/*--------------------------------------------------------------------------
+ * SQL-callable functions
+ *--------------------------------------------------------------------------
+ */
+
+/*
+ * pgstat_create_custom_stat() -
+ *
+ * SQL-callable function to create a new custom statistics entry.
+ * Initializes shared memory structure for tracking statistics with
+ * a name and optional description.
+ */
+PG_FUNCTION_INFO_V1(pgstat_create_custom_stat);
+Datum
+pgstat_create_custom_stat(PG_FUNCTION_ARGS)
+{
+	PgStat_EntryRef *entry_ref;
+	bool		found;
+	char	   *desc_copy;
+	size_t		len;
+	PgStatShared_CustomEntry *shstatent;
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	char	   *description = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+	if (!pgstat_custom_kind_loaded)
+		PG_RETURN_VOID();
+
+	entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_TEST_CUSTOM_STATKIND, InvalidOid,
+											PGSTAT_CUSTOM_STAT_IDX(name), true);
+
+	if (!entry_ref)
+		PG_RETURN_VOID();
+
+	shstatent = (PgStatShared_CustomEntry *) entry_ref->shared_stats;
+
+	/* Initialize shared memory statistics data to zero */
+	memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+
+	/* Validate and store the statistic name */
+	if (strlen(name) >= NAMEDATALEN)
+		ereport(ERROR,
+				(errcode(ERRCODE_NAME_TOO_LONG),
+				 errmsg("custom statistic name \"%s\" is too long", name),
+				 errdetail("Name must be less than %d characters.", NAMEDATALEN)));
+
+	strcpy(shstatent->name, name);
+
+	/* Store description in DSA if provided */
+	if (description)
+	{
+		len = strlen(description) + 1;
+
+		/* Initialize DSA for descriptions if not already done */
+		if (!custom_stats_description_dsa)
+			custom_stats_description_dsa = GetNamedDSA("pgstat_custom_stat_dsa", &found);
+
+		if (custom_stats_description_dsa)
+		{
+			/* Allocate space in DSA and copy description */
+			shstatent->description = dsa_allocate(custom_stats_description_dsa, len);
+			if (!DsaPointerIsValid(shstatent->description))
+			{
+				/* DSA allocation failed, continue without description */
+				shstatent->description = InvalidDsaPointer;
+			}
+			else
+			{
+				desc_copy = dsa_get_address(custom_stats_description_dsa,
+											shstatent->description);
+				memcpy(desc_copy, description, len);
+			}
+		}
+	}
+
+	pgstat_unlock_entry(entry_ref);
+
+	PG_RETURN_VOID();
+}
+
+/*
+ * pgstat_call_custom_stat() -
+ *
+ * SQL-callable function to increment the call count for a custom statistic.
+ * This is typically called from user code to track usage or events.
+ */
+PG_FUNCTION_INFO_V1(pgstat_call_custom_stat);
+Datum
+pgstat_call_custom_stat(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	PgStat_EntryRef *entry_ref;
+	PgStat_StatCustomEntry *pending;
+
+	if (!pgstat_custom_kind_loaded)
+		PG_RETURN_VOID();
+
+	/* Get the pending statistics entry for this custom stat */
+	entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_TEST_CUSTOM_STATKIND, InvalidOid,
+										  PGSTAT_CUSTOM_STAT_IDX(name), NULL);
+
+	pending = (PgStat_StatCustomEntry *) entry_ref->pending;
+
+	/* Increment the call counter in local pending stats */
+	pending->numcalls++;
+
+	PG_RETURN_VOID();
+}
+
+/*
+ * pgstat_report_custom_stat() -
+ *
+ * SQL-callable function to retrieve statistics for a custom statistic.
+ * Returns a composite type containing the name, description, and call count.
+ */
+PG_FUNCTION_INFO_V1(pgstat_report_custom_stat);
+Datum
+pgstat_report_custom_stat(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	PgStat_EntryRef *entry_ref;
+	PgStatShared_CustomEntry *shfuncent;
+	Datum		values[3];
+	bool		nulls[3] = {false, false, false};
+	TupleDesc	tupdesc;
+	TupleDesc	expected;
+	bool		found;
+
+	if (!pgstat_custom_kind_loaded)
+		PG_RETURN_NULL();
+
+	/* Initialize DSA for descriptions if needed */
+	if (!custom_stats_description_dsa)
+		custom_stats_description_dsa =
+			GetNamedDSA("pgstat_custom_stat_dsa", &found);
+
+	/* Return NULL if DSA is not available */
+	if (!custom_stats_description_dsa)
+		PG_RETURN_NULL();
+
+	/* Get the return tuple descriptor from pg_proc */
+	if (get_call_result_type(fcinfo, NULL, &expected) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, "pgstat_report_custom_stat: return type is not composite");
+
+	tupdesc = BlessTupleDesc(expected);
+
+	/* Look up the statistics entry in shared memory */
+	entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_TEST_CUSTOM_STATKIND,
+											InvalidOid,
+											PGSTAT_CUSTOM_STAT_IDX(name),
+											true);
+
+	if (entry_ref)
+	{
+		char	   *desc_copy = NULL;
+
+		shfuncent = (PgStatShared_CustomEntry *) entry_ref->shared_stats;
+
+		/* Build the return tuple with name, description, and call count */
+		values[0] = CStringGetTextDatum(name);	/* name */
+		if (DsaPointerIsValid(shfuncent->description))
+		{
+			/* Get description from DSA */
+			desc_copy = dsa_get_address(custom_stats_description_dsa,
+										shfuncent->description);
+
+			values[1] = CStringGetTextDatum(desc_copy); /* description */
+		}
+		else
+			nulls[1] = true;	/* no description available */
+
+		values[2] = Int64GetDatum(shfuncent->stats.numcalls);	/* calls */
+	}
+
+	pgstat_unlock_entry(entry_ref);
+
+	PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls)));
+}
diff --git a/src/test/modules/test_custom_statkind/test_custom_statkind.control b/src/test/modules/test_custom_statkind/test_custom_statkind.control
new file mode 100644
index 00000000000..7ce22cc195f
--- /dev/null
+++ b/src/test/modules/test_custom_statkind/test_custom_statkind.control
@@ -0,0 +1,4 @@
+comment = 'Test code for custom stat kinds'
+default_version = '1.0'
+module_pathname = '$libdir/test_custom_statkind'
+relocatable = true
-- 
2.43.0



  [application/octet-stream] v2-0001-pgstat-support-custom-serialization-files-and-cal.patch (7.3K, ../CAA5RZ0ufds2r3H7QUro_yJDrnUmsQ4ocufqgcsRGoZvxQKeMrA@mail.gmail.com/3-v2-0001-pgstat-support-custom-serialization-files-and-cal.patch)
  download | inline diff:
From b569f8b0a3a886cf759e9ac8d2376eadb45b6c29 Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Mon, 10 Nov 2025 00:03:41 -0600
Subject: [PATCH v2 1/2] pgstat: support custom serialization files and
 callbacks

Allow custom statistics kinds to serialize and deserialize extra
per-entry data, supporting kinds with variable auxiliary data that
cannot fit in shared-memory.

To allow this, 3 callbacks are provided to serialize, deserialize
and to clean-up resources when writing, reading and discarding
entries. The latter is required for crash recovery scenarios.
---
 src/backend/utils/activity/pgstat.c | 61 ++++++++++++++++++++++++++++-
 src/include/utils/pgstat_internal.h | 33 ++++++++++++++++
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 8713c7a0483..117728db016 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -523,6 +523,7 @@ pgstat_discard_stats(void)
 
 	/* NB: this needs to be done even in single user mode */
 
+	/* First, cleanup the core stats file */
 	ret = unlink(PGSTAT_STAT_PERMANENT_FILENAME);
 	if (ret != 0)
 	{
@@ -544,6 +545,15 @@ pgstat_discard_stats(void)
 								 PGSTAT_STAT_PERMANENT_FILENAME)));
 	}
 
+	/* Now, cleanup every custom kinds extra stats files */
+	for (PgStat_Kind kind = PGSTAT_KIND_CUSTOM_MIN; kind <= PGSTAT_KIND_CUSTOM_MAX; kind++)
+	{
+		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
+
+		if (kind_info && kind_info->end_extra_stats)
+			kind_info->end_extra_stats(STATS_DISCARD);
+	}
+
 	/*
 	 * Reset stats contents. This will set reset timestamps of fixed-numbered
 	 * stats to the current time (no variable stats exist).
@@ -1463,6 +1473,7 @@ pgstat_get_kind_info(PgStat_Kind kind)
 void
 pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info)
 {
+	bool		has_extra = false;
 	uint32		idx = kind - PGSTAT_KIND_CUSTOM_MIN;
 
 	if (kind_info->name == NULL || strlen(kind_info->name) == 0)
@@ -1525,6 +1536,26 @@ pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info)
 					 errdetail("Existing cumulative statistics with ID %u has the same name.", existing_kind)));
 	}
 
+	/*
+	 * Ensure that to_serialized_extra_stats, from_serialized_extra_stats and
+	 * end_extra_stats are registered together or not at all.
+	 */
+	has_extra =
+		kind_info->to_serialized_extra_stats ||
+		kind_info->from_serialized_extra_stats ||
+		kind_info->end_extra_stats;
+
+	if (has_extra &&
+		(!kind_info->to_serialized_extra_stats ||
+		 !kind_info->from_serialized_extra_stats ||
+		 !kind_info->end_extra_stats))
+	{
+		ereport(ERROR,
+				(errmsg("could not register custom cumulative statistics \"%s\" with ID %u",
+						kind_info->name, kind),
+				 errdetail("callbacks to_serialized_extra, from_serialized_extra, and end_extra_stats must all be provided together.")));
+	}
+
 	/* Register it */
 	pgstat_kind_custom_infos[idx] = kind_info;
 	ereport(LOG,
@@ -1702,6 +1733,9 @@ pgstat_write_statsfile(void)
 		pgstat_write_chunk(fpout,
 						   pgstat_get_entry_data(ps->key.kind, shstats),
 						   pgstat_get_entry_len(ps->key.kind));
+
+		if (pgstat_is_kind_custom(ps->key.kind) && kind_info->to_serialized_extra_stats)
+			kind_info->to_serialized_extra_stats(&ps->key, shstats, fpout);
 	}
 	dshash_seq_term(&hstat);
 
@@ -1734,6 +1768,15 @@ pgstat_write_statsfile(void)
 		/* durable_rename already emitted log message */
 		unlink(tmpfile);
 	}
+
+	/* Now, allow the extension to finalize the writes for the extra files */
+	for (PgStat_Kind kind = PGSTAT_KIND_CUSTOM_MIN; kind <= PGSTAT_KIND_CUSTOM_MAX; kind++)
+	{
+		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
+
+		if (kind_info && kind_info->end_extra_stats)
+			kind_info->end_extra_stats(STATS_WRITE);
+	}
 }
 
 /* helper for pgstat_read_statsfile() */
@@ -1871,6 +1914,7 @@ pgstat_read_statsfile(void)
 					PgStat_HashKey key;
 					PgStatShared_HashEntry *p;
 					PgStatShared_Common *header;
+					const PgStat_KindInfo *kind_info = NULL;
 
 					CHECK_FOR_INTERRUPTS();
 
@@ -1891,7 +1935,8 @@ pgstat_read_statsfile(void)
 							goto error;
 						}
 
-						if (!pgstat_get_kind_info(key.kind))
+						kind_info = pgstat_get_kind_info(key.kind);
+						if (!kind_info)
 						{
 							elog(WARNING, "could not find information of kind for entry %u/%u/%" PRIu64 " of type %c",
 								 key.kind, key.dboid,
@@ -1902,7 +1947,6 @@ pgstat_read_statsfile(void)
 					else
 					{
 						/* stats entry identified by name on disk (e.g. slots) */
-						const PgStat_KindInfo *kind_info = NULL;
 						PgStat_Kind kind;
 						NameData	name;
 
@@ -1996,6 +2040,9 @@ pgstat_read_statsfile(void)
 						goto error;
 					}
 
+					if (pgstat_is_kind_custom(key.kind) && kind_info->from_serialized_extra_stats)
+						kind_info->from_serialized_extra_stats(&key, header, fpin);
+
 					break;
 				}
 			case PGSTAT_FILE_ENTRY_END:
@@ -2019,11 +2066,21 @@ pgstat_read_statsfile(void)
 	}
 
 done:
+	/* first, cleanup the core stats file */
 	FreeFile(fpin);
 
 	elog(DEBUG2, "removing permanent stats file \"%s\"", statfile);
 	unlink(statfile);
 
+	/* Now, cleanup every custom kinds extra stats files */
+	for (PgStat_Kind kind = PGSTAT_KIND_CUSTOM_MIN; kind <= PGSTAT_KIND_CUSTOM_MAX; kind++)
+	{
+		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
+
+		if (kind_info && kind_info->end_extra_stats)
+			kind_info->end_extra_stats(STATS_READ);
+	}
+
 	return;
 
 error:
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index ca1ba6420ca..25bba6e98d4 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -63,6 +63,20 @@ typedef struct PgStat_HashKey
 								 * identifier. */
 } PgStat_HashKey;
 
+/*
+ * Tracks if the stats file is being read, written or discarded.
+ *
+ * These states allow plugins that create extra statistics files
+ * to determine the current operation and perform any necessary
+ * file cleanup.
+ */
+typedef enum PgStat_StatsFileOp
+{
+	STATS_WRITE,
+	STATS_READ,
+	STATS_DISCARD,
+}			PgStat_StatsFileOp;
+
 /*
  * PgStat_HashKey should not have any padding.  Checking that the structure
  * size matches with the sum of each field is a check simple enough to
@@ -303,6 +317,25 @@ typedef struct PgStat_KindInfo
 									   const PgStatShared_Common *header, NameData *name);
 	bool		(*from_serialized_name) (const NameData *name, PgStat_HashKey *key);
 
+	/*
+	 * Optional callbacks for kinds that write additional per-entry data to
+	 * the stats file.  If any of these callbacks are provided, all three must
+	 * be provided to ensure that the reader consumes exactly the data written
+	 * by the writer.
+	 *
+	 * to_serialized_extra_stats: write extra data for an entry.
+	 *
+	 * from_serialized_extra_stats: read the extra data for an entry.
+	 *
+	 * end_extra_stats: invoked once per operation (read, write, discard)
+	 * after all entries of this kind have been processed.
+	 */
+	void		(*to_serialized_extra_stats) (const PgStat_HashKey *key,
+											  const PgStatShared_Common *header, FILE *statfile);
+	void		(*from_serialized_extra_stats) (const PgStat_HashKey *key,
+												const PgStatShared_Common *header, FILE *statfile);
+	void		(*end_extra_stats) (PgStat_StatsFileOp status);
+
 	/*
 	 * For fixed-numbered statistics: Initialize shared memory state.
 	 *
-- 
2.43.0



view thread (47+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [Proposal] Adding callback support for custom statistics kinds
  In-Reply-To: <CAA5RZ0ufds2r3H7QUro_yJDrnUmsQ4ocufqgcsRGoZvxQKeMrA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox