From a9566b9110b53491f792a61c83f5d76b01fb029f Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Fri, 10 Apr 2026 07:27:44 -0500 Subject: [PATCH v2 2/2] Add test module for sync handler registration test_sync_handler exercises register_sync_handler() from _PG_init() and verifies: - The registered handler ID is at least SYNC_HANDLER_FIRST_DYNAMIC. - Distinct FileTags produce distinct sync_syncfiletag callbacks at CHECKPOINT time. - Duplicate FileTags coalesce via HASH_BLOBS to a single dispatch. - Idle checkpoints do not re-dispatch already-processed entries (cycle_ctr skip). Shared state between the backend and the checkpointer uses GetNamedDSMSegment() so the dispatch counter is visible to the backend that queries it. Discussion: https://postgr.es/m/IA1PR07MB983072521EE7FDEE98902534A9592@IA1PR07MB9830.namprd07.prod.outlook.com --- 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..b2cd25cc18d --- /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, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("sync handler was not registered during module initialization"))); + + /* + * 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, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("could not register sync request"))); + + 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