From 47e81c9b6001f93041c52708b3c2c0a444194c41 Mon Sep 17 00:00:00 2001
From: Greg Lamberson <greg@lamco.io>
Date: Fri, 10 Apr 2026 07:27:44 -0500
Subject: [PATCH v1 2/2] Add test module for sync handler registration

Adds src/test/modules/test_sync_handler, a minimal extension that
exercises the register_sync_handler() API introduced in the previous
commit. The module:

- Registers a trivial SyncOps via register_sync_handler() at
  _PG_init() time and stores the returned handler ID.
- Exposes SQL-callable test_sync_handler_register(seg bigint) that
  queues a FileTag via RegisterSyncRequest(SYNC_REQUEST).
- Tracks the number of sync_syncfiletag callback invocations in a
  shared-memory counter (via GetNamedDSMSegment) so that the
  checkpointer's increments are visible to the backend that calls
  test_sync_handler_count().

The TAP test in t/001_basic.pl verifies:

- The registered handler ID is >= SYNC_HANDLER_FIRST_DYNAMIC, proving
  that built-in handlers still occupy IDs 0..4.
- Queuing 5 distinct FileTags produces 5 callback invocations after
  CHECKPOINT, confirming that dispatch flows through the new dynamic
  table for extension-registered handlers.
- Queuing 10 identical FileTags produces only 1 additional callback
  invocation, confirming that HASH_BLOBS coalescing still works
  correctly with extension-assigned handler IDs.
- An idle CHECKPOINT (with no new queued entries) does not re-invoke
  the callback, confirming cycle_ctr skip semantics.

Module layout follows the pattern established by
src/test/modules/test_slru. The test sets fsync=on in its TAP cluster
config (overriding the default fsync=off that TAP clusters use for
speed) because ProcessSyncRequests skips dispatch entirely when
fsync is off.

This mirrors how fsync_checker is structured in CF 5616 v5+: a
minimal reference consumer that demonstrates the API surface, lives
in src/test/modules so installcheck users don't need to preload it,
and has a focused TAP test.

Signed-off-by: Greg Lamberson <greg@lamco.io>
---
 src/test/modules/Makefile                     |   1 +
 src/test/modules/meson.build                  |   1 +
 src/test/modules/test_sync_handler/.gitignore |   4 +
 src/test/modules/test_sync_handler/Makefile   |  27 +++
 .../modules/test_sync_handler/meson.build     |  33 ++++
 .../modules/test_sync_handler/t/001_basic.pl  |  96 +++++++++
 .../test_sync_handler--1.0.sql                |  13 ++
 .../test_sync_handler/test_sync_handler.c     | 187 ++++++++++++++++++
 .../test_sync_handler.control                 |   4 +
 9 files changed, 366 insertions(+)
 create mode 100644 src/test/modules/test_sync_handler/.gitignore
 create mode 100644 src/test/modules/test_sync_handler/Makefile
 create mode 100644 src/test/modules/test_sync_handler/meson.build
 create mode 100644 src/test/modules/test_sync_handler/t/001_basic.pl
 create mode 100644 src/test/modules/test_sync_handler/test_sync_handler--1.0.sql
 create mode 100644 src/test/modules/test_sync_handler/test_sync_handler.c
 create mode 100644 src/test/modules/test_sync_handler/test_sync_handler.control

diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 0a74ab5c86f..2a3334d7508 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -52,6 +52,7 @@ SUBDIRS = \
 		  test_shmem \
 		  test_shm_mq \
 		  test_slru \
+		  test_sync_handler \
 		  test_tidstore \
 		  unsafe_tests \
 		  worker_spi \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 4bca42bb370..00bc7454cc8 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -53,6 +53,7 @@ subdir('test_saslprep')
 subdir('test_shmem')
 subdir('test_shm_mq')
 subdir('test_slru')
+subdir('test_sync_handler')
 subdir('test_tidstore')
 subdir('typcache')
 subdir('unsafe_tests')
diff --git a/src/test/modules/test_sync_handler/.gitignore b/src/test/modules/test_sync_handler/.gitignore
new file mode 100644
index 00000000000..5dcb3ff9723
--- /dev/null
+++ b/src/test/modules/test_sync_handler/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_sync_handler/Makefile b/src/test/modules/test_sync_handler/Makefile
new file mode 100644
index 00000000000..22326a47e9c
--- /dev/null
+++ b/src/test/modules/test_sync_handler/Makefile
@@ -0,0 +1,27 @@
+# src/test/modules/test_sync_handler/Makefile
+
+MODULE_big = test_sync_handler
+OBJS = \
+	$(WIN32RES) \
+	test_sync_handler.o
+PGFILEDESC = "test_sync_handler - test module for sync handler registration"
+
+EXTENSION = test_sync_handler
+DATA = test_sync_handler--1.0.sql
+
+TAP_TESTS = 1
+
+# Tests require shared_preload_libraries=test_sync_handler which typical
+# installcheck users do not have. Match test_slru's convention.
+NO_INSTALLCHECK = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_sync_handler
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_sync_handler/meson.build b/src/test/modules/test_sync_handler/meson.build
new file mode 100644
index 00000000000..e7f03616ba0
--- /dev/null
+++ b/src/test/modules/test_sync_handler/meson.build
@@ -0,0 +1,33 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+test_sync_handler_sources = files(
+  'test_sync_handler.c',
+)
+
+if host_system == 'windows'
+  test_sync_handler_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'test_sync_handler',
+    '--FILEDESC', 'test_sync_handler - test module for sync handler registration',])
+endif
+
+test_sync_handler = shared_module('test_sync_handler',
+  test_sync_handler_sources,
+  kwargs: pg_test_mod_args,
+)
+test_install_libs += test_sync_handler
+
+test_install_data += files(
+  'test_sync_handler.control',
+  'test_sync_handler--1.0.sql',
+)
+
+tests += {
+  'name': 'test_sync_handler',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+    'tests': [
+      't/001_basic.pl',
+    ],
+  },
+}
diff --git a/src/test/modules/test_sync_handler/t/001_basic.pl b/src/test/modules/test_sync_handler/t/001_basic.pl
new file mode 100644
index 00000000000..29c0fc3c61e
--- /dev/null
+++ b/src/test/modules/test_sync_handler/t/001_basic.pl
@@ -0,0 +1,96 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Basic test for register_sync_handler() dispatch.
+#
+# Verifies that a custom sync handler registered via register_sync_handler()
+# in _PG_init() receives callback invocations from ProcessSyncRequests() at
+# CHECKPOINT time, that identical FileTags coalesce via HASH_BLOBS
+# deduplication, that distinct FileTags produce distinct callbacks, and
+# that an idle checkpoint does not re-dispatch entries that were already
+# processed (cycle_ctr skip).
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('sync_handler');
+$node->init;
+$node->append_conf(
+	'postgresql.conf', q{
+shared_preload_libraries = 'test_sync_handler'
+# TAP clusters set fsync = off by default for speed; re-enable here so
+# that ProcessSyncRequests actually dispatches our sync handler callback.
+fsync = on
+});
+$node->start;
+$node->safe_psql('postgres', 'CREATE EXTENSION test_sync_handler');
+
+# The handler ID must be >= SYNC_HANDLER_FIRST_DYNAMIC. Built-ins
+# currently occupy IDs 0..4, so the first extension handler should be
+# at least 5.
+my $id = $node->safe_psql('postgres', 'SELECT test_sync_handler_id()');
+ok($id >= 5,
+	"handler id $id is >= SYNC_HANDLER_FIRST_DYNAMIC (built-ins = 5)")
+  or diag("got id=$id");
+
+# Baseline: no dispatches before we queue anything.
+my $baseline =
+  $node->safe_psql('postgres', 'SELECT test_sync_handler_count()');
+is($baseline, '0', 'baseline dispatch count is zero');
+
+# Queue 5 distinct FileTags (differing in segno only) and checkpoint.
+# Expect 5 callback invocations since they are all distinct hash keys.
+$node->safe_psql(
+	'postgres', q{
+SELECT test_sync_handler_register(1);
+SELECT test_sync_handler_register(2);
+SELECT test_sync_handler_register(3);
+SELECT test_sync_handler_register(4);
+SELECT test_sync_handler_register(5);
+});
+$node->safe_psql('postgres', 'CHECKPOINT');
+my $after_distinct =
+  $node->safe_psql('postgres', 'SELECT test_sync_handler_count()');
+is($after_distinct, '5',
+	'5 distinct FileTags produce 5 sync_syncfiletag callbacks')
+  or diag("got $after_distinct");
+
+# Queue 10 duplicate FileTags (same segno 42) and checkpoint.
+# Expect exactly 1 additional callback because pendingOps uses HASH_BLOBS
+# and collapses identical FileTags into a single hash entry.
+$node->safe_psql(
+	'postgres', q{
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+SELECT test_sync_handler_register(42);
+});
+$node->safe_psql('postgres', 'CHECKPOINT');
+my $after_coalesce =
+  $node->safe_psql('postgres', 'SELECT test_sync_handler_count()');
+is($after_coalesce, '6',
+	'10 duplicate FileTags coalesce via HASH_BLOBS to 1 additional callback')
+  or diag("got $after_coalesce");
+
+# Second CHECKPOINT with no new requests. The count must stay the same:
+# every entry from the previous checkpoint was processed and removed
+# from pendingOps, and no new entries have been queued, so
+# ProcessSyncRequests has nothing to dispatch.
+$node->safe_psql('postgres', 'CHECKPOINT');
+my $after_idle =
+  $node->safe_psql('postgres', 'SELECT test_sync_handler_count()');
+is($after_idle, '6', 'idle checkpoint does not re-dispatch')
+  or diag("got $after_idle");
+
+$node->stop;
+
+done_testing();
diff --git a/src/test/modules/test_sync_handler/test_sync_handler--1.0.sql b/src/test/modules/test_sync_handler/test_sync_handler--1.0.sql
new file mode 100644
index 00000000000..07ea297f15f
--- /dev/null
+++ b/src/test/modules/test_sync_handler/test_sync_handler--1.0.sql
@@ -0,0 +1,13 @@
+/* src/test/modules/test_sync_handler/test_sync_handler--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_sync_handler" to load this file. \quit
+
+CREATE FUNCTION test_sync_handler_id() RETURNS int4
+  AS 'MODULE_PATHNAME', 'test_sync_handler_id' LANGUAGE C STRICT;
+
+CREATE FUNCTION test_sync_handler_register(seg bigint) RETURNS void
+  AS 'MODULE_PATHNAME', 'test_sync_handler_register' LANGUAGE C STRICT;
+
+CREATE FUNCTION test_sync_handler_count() RETURNS bigint
+  AS 'MODULE_PATHNAME', 'test_sync_handler_count' LANGUAGE C STRICT;
diff --git a/src/test/modules/test_sync_handler/test_sync_handler.c b/src/test/modules/test_sync_handler/test_sync_handler.c
new file mode 100644
index 00000000000..055d90b55de
--- /dev/null
+++ b/src/test/modules/test_sync_handler/test_sync_handler.c
@@ -0,0 +1,187 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_sync_handler.c
+ *		Minimal extension exercising register_sync_handler() + dispatch.
+ *
+ * This module demonstrates the sync.c extensibility API by registering a
+ * trivial SyncOps during _PG_init(), exposing SQL-callable helpers to
+ * queue FileTags for the registered handler, and tracking how many times
+ * the handler's sync_syncfiletag callback is invoked.
+ *
+ * Because sync_syncfiletag runs in the checkpointer process but
+ * test_sync_handler_count() runs in a regular backend, the call counter
+ * lives in shared memory via GetNamedDSMSegment().
+ *
+ * The TAP test in t/001_basic.pl uses this module to verify:
+ *   - register_sync_handler() returns an ID >= SYNC_HANDLER_FIRST_DYNAMIC
+ *   - Queued FileTags round-trip through the checkpointer and land in
+ *     the registered sync_syncfiletag callback at CHECKPOINT time
+ *   - Identical FileTags coalesce via HASH_BLOBS deduplication in
+ *     pendingOps (N duplicates -> 1 callback)
+ *   - Distinct FileTags produce distinct callbacks
+ *   - Idle checkpoints do not re-dispatch (cycle_ctr skip)
+ *
+ * Copyright (c) 2026, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		src/test/modules/test_sync_handler/test_sync_handler.c
+ *
+ *--------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "pg_config.h"
+#include "port/atomics.h"
+#include "storage/dsm_registry.h"
+#include "storage/sync.h"
+#include "utils/builtins.h"
+
+PG_MODULE_MAGIC;
+
+void		_PG_init(void);
+
+typedef struct TshSharedState
+{
+	pg_atomic_uint64 call_count;
+} TshSharedState;
+
+static int16 tsh_handler_id = -1;
+static TshSharedState *tsh_shared = NULL;
+
+/*
+ * GetNamedDSMSegment's init_callback signature gained an extra `arg`
+ * parameter in PG 19devel. Provide both shapes so the test module is
+ * buildable across 18 and 19.
+ */
+#if PG_VERSION_NUM >= 190000
+static void
+tsh_init_shmem(void *ptr, void *arg)
+#else
+static void
+tsh_init_shmem(void *ptr)
+#endif
+{
+	TshSharedState *state = (TshSharedState *) ptr;
+
+	pg_atomic_init_u64(&state->call_count, 0);
+}
+
+static void
+tsh_attach_shmem(void)
+{
+	bool		found;
+
+	if (tsh_shared != NULL)
+		return;
+#if PG_VERSION_NUM >= 190000
+	tsh_shared = GetNamedDSMSegment("test_sync_handler",
+									sizeof(TshSharedState),
+									tsh_init_shmem,
+									&found,
+									NULL);
+#else
+	tsh_shared = GetNamedDSMSegment("test_sync_handler",
+									sizeof(TshSharedState),
+									tsh_init_shmem,
+									&found);
+#endif
+}
+
+static int
+test_sync_syncfiletag(const FileTag *ftag, char *path)
+{
+	/*
+	 * This runs in the checkpointer process. Attach to the shared
+	 * memory segment the first time we're called so that counter
+	 * increments are visible to the backend that queries
+	 * test_sync_handler_count().
+	 */
+	tsh_attach_shmem();
+	pg_atomic_fetch_add_u64(&tsh_shared->call_count, 1);
+
+	if (path != NULL)
+		snprintf(path, MAXPGPATH, "test_sync_handler:seg%llu",
+				 (unsigned long long) ftag->segno);
+	return 0;
+}
+
+static int
+test_sync_unlinkfiletag(const FileTag *ftag, char *path)
+{
+	if (path != NULL)
+		snprintf(path, MAXPGPATH, "test_sync_handler:unlink");
+	return 0;
+}
+
+static bool
+test_sync_filetagmatches(const FileTag *ftag, const FileTag *candidate)
+{
+	/*
+	 * Match on dbOid, mirroring md.c's DROP DATABASE semantics. The
+	 * test doesn't exercise the filter path today, but the callback
+	 * is defined so extensions can use this module as a copy-paste
+	 * starting point.
+	 */
+	return ftag->rlocator.dbOid == candidate->rlocator.dbOid;
+}
+
+static const SyncOps test_sync_ops = {
+	.sync_syncfiletag = test_sync_syncfiletag,
+	.sync_unlinkfiletag = test_sync_unlinkfiletag,
+	.sync_filetagmatches = test_sync_filetagmatches,
+};
+
+void
+_PG_init(void)
+{
+	tsh_handler_id = register_sync_handler(&test_sync_ops, "test_sync_handler");
+	elog(LOG, "test_sync_handler: registered as id %d",
+		 (int) tsh_handler_id);
+}
+
+PG_FUNCTION_INFO_V1(test_sync_handler_id);
+Datum
+test_sync_handler_id(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT32((int32) tsh_handler_id);
+}
+
+PG_FUNCTION_INFO_V1(test_sync_handler_register);
+Datum
+test_sync_handler_register(PG_FUNCTION_ARGS)
+{
+	int64		seg = PG_GETARG_INT64(0);
+	FileTag		tag;
+
+	if (tsh_handler_id < 0)
+		ereport(ERROR,
+				(errmsg("test_sync_handler: registration failed at _PG_init")));
+
+	/*
+	 * Mandatory memset: pendingOps uses HASH_BLOBS which hashes every
+	 * byte of the FileTag. Uninitialized padding would break coalescing.
+	 */
+	memset(&tag, 0, sizeof(FileTag));
+	tag.handler = tsh_handler_id;
+	tag.forknum = 0;
+	tag.rlocator.spcOid = 1;
+	tag.rlocator.dbOid = MyDatabaseId;
+	tag.rlocator.relNumber = 1;
+	tag.segno = (uint64) seg;
+
+	if (!RegisterSyncRequest(&tag, SYNC_REQUEST, true /* retryOnError */))
+		ereport(ERROR,
+				(errmsg("test_sync_handler: RegisterSyncRequest failed")));
+
+	PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_sync_handler_count);
+Datum
+test_sync_handler_count(PG_FUNCTION_ARGS)
+{
+	tsh_attach_shmem();
+	PG_RETURN_INT64((int64) pg_atomic_read_u64(&tsh_shared->call_count));
+}
diff --git a/src/test/modules/test_sync_handler/test_sync_handler.control b/src/test/modules/test_sync_handler/test_sync_handler.control
new file mode 100644
index 00000000000..3d528f7a866
--- /dev/null
+++ b/src/test/modules/test_sync_handler/test_sync_handler.control
@@ -0,0 +1,4 @@
+comment = 'Test module for sync handler registration'
+default_version = '1.0'
+module_pathname = '$libdir/test_sync_handler'
+relocatable = true
-- 
2.47.3

