From ae6acc8de61ebd9e1c040b46c859b4d075f70e2a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 8 Jul 2024 13:36:06 +0900 Subject: [PATCH v6 4/5] injection_points: Add statistics for custom points This acts as a template of what can be achieved with the pluggable cumulative stats APIs, while being useful on its own for injection points. Currently, the only data gathered is the number of times an injection point is called. This can be extended as required. All the routines related to the stats are located in their own file, for clarity. A TAP test is included to provide coverage for these new APIs, showing the persistency of the data across restarts. --- src/test/modules/injection_points/Makefile | 11 +- .../injection_points--1.0.sql | 10 + .../injection_points/injection_points.c | 27 +++ .../injection_points/injection_stats.c | 194 ++++++++++++++++++ .../injection_points/injection_stats.h | 23 +++ src/test/modules/injection_points/meson.build | 9 + .../modules/injection_points/t/001_stats.pl | 48 +++++ src/tools/pgindent/typedefs.list | 2 + 8 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 src/test/modules/injection_points/injection_stats.c create mode 100644 src/test/modules/injection_points/injection_stats.h create mode 100644 src/test/modules/injection_points/t/001_stats.pl diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 2ffd2f77ed..7b9cd12a2a 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -1,7 +1,10 @@ # src/test/modules/injection_points/Makefile -MODULES = injection_points - +MODULE_big = injection_points +OBJS = \ + $(WIN32RES) \ + injection_points.o \ + injection_stats.o EXTENSION = injection_points DATA = injection_points--1.0.sql PGFILEDESC = "injection_points - facility for injection points" @@ -11,9 +14,13 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = inplace +TAP_TESTS = 1 + # The injection points are cluster-wide, so disable installcheck NO_INSTALLCHECK = 1 +export enable_injection_points enable_injection_points + ifdef USE_PGXS PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql index e275c2cf5b..747a64e812 100644 --- a/src/test/modules/injection_points/injection_points--1.0.sql +++ b/src/test/modules/injection_points/injection_points--1.0.sql @@ -64,3 +64,13 @@ CREATE FUNCTION injection_points_detach(IN point_name TEXT) RETURNS void AS 'MODULE_PATHNAME', 'injection_points_detach' LANGUAGE C STRICT PARALLEL UNSAFE; + +-- +-- injection_points_stats_numcalls() +-- +-- Reports statistics, if any, related to the given injection point. +-- +CREATE FUNCTION injection_points_stats_numcalls(IN point_name TEXT) +RETURNS bigint +AS 'MODULE_PATHNAME', 'injection_points_stats_numcalls' +LANGUAGE C STRICT; diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index b6c8e89324..eb411b9d44 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -18,6 +18,7 @@ #include "postgres.h" #include "fmgr.h" +#include "injection_stats.h" #include "miscadmin.h" #include "nodes/pg_list.h" #include "nodes/value.h" @@ -170,6 +171,9 @@ injection_points_cleanup(int code, Datum arg) char *name = strVal(lfirst(lc)); (void) InjectionPointDetach(name); + + /* Remove stats entry */ + pgstat_drop_inj(name); } } @@ -182,6 +186,8 @@ injection_error(const char *name, const void *private_data) if (!injection_point_allowed(condition)) return; + pgstat_report_inj(name); + elog(ERROR, "error triggered for injection point %s", name); } @@ -193,6 +199,8 @@ injection_notice(const char *name, const void *private_data) if (!injection_point_allowed(condition)) return; + pgstat_report_inj(name); + elog(NOTICE, "notice triggered for injection point %s", name); } @@ -211,6 +219,8 @@ injection_wait(const char *name, const void *private_data) if (!injection_point_allowed(condition)) return; + pgstat_report_inj(name); + /* * Use the injection point name for this custom wait event. Note that * this custom wait event name is not released, but we don't care much for @@ -299,6 +309,10 @@ injection_points_attach(PG_FUNCTION_ARGS) inj_list_local = lappend(inj_list_local, makeString(pstrdup(name))); MemoryContextSwitchTo(oldctx); } + + /* Add entry for stats */ + pgstat_create_inj(name); + PG_RETURN_VOID(); } @@ -417,5 +431,18 @@ injection_points_detach(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldctx); } + /* Remove stats entry */ + pgstat_drop_inj(name); + PG_RETURN_VOID(); } + + +void +_PG_init(void) +{ + if (!process_shared_preload_libraries_in_progress) + return; + + pgstat_register_inj(); +} diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c new file mode 100644 index 0000000000..c37b0b33d3 --- /dev/null +++ b/src/test/modules/injection_points/injection_stats.c @@ -0,0 +1,194 @@ +/*-------------------------------------------------------------------------- + * + * injection_stats.c + * Code for statistics of injection points. + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/test/modules/injection_points/injection_stats.c + * + * ------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "fmgr.h" + +#include "common/hashfn.h" +#include "injection_stats.h" +#include "pgstat.h" +#include "utils/builtins.h" +#include "utils/pgstat_internal.h" + +/* Structures for statistics of injection points */ +typedef struct PgStat_StatInjEntry +{ + PgStat_Counter numcalls; /* number of times point has been run */ +} PgStat_StatInjEntry; + +typedef struct PgStatShared_InjectionPoint +{ + PgStatShared_Common header; + PgStat_StatInjEntry stats; +} PgStatShared_InjectionPoint; + +static bool injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait); + +static const PgStat_KindInfo injection_stats = { + .name = "injection_points", + .fixed_amount = false, /* Bounded by the number of points */ + + /* Injection points are system-wide */ + .accessed_across_databases = true, + + .shared_size = sizeof(PgStatShared_InjectionPoint), + .shared_data_off = offsetof(PgStatShared_InjectionPoint, stats), + .shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats), + .pending_size = sizeof(PgStat_StatInjEntry), + .flush_pending_cb = injection_stats_flush_cb, +}; + +/* + * Compute stats entry idx from point name with a 4-byte hash. + */ +#define PGSTAT_INJ_IDX(name) hash_bytes((const unsigned char *) name, strlen(name)) + +#define PGSTAT_KIND_INJECTION PGSTAT_KIND_EXPERIMENTAL + +/* Track if stats are loaded */ +static bool inj_stats_loaded = false; + +/* + * Callback for stats handling + */ +static bool +injection_stats_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) +{ + PgStat_StatInjEntry *localent; + PgStatShared_InjectionPoint *shfuncent; + + localent = (PgStat_StatInjEntry *) entry_ref->pending; + shfuncent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; + + if (!pgstat_lock_entry(entry_ref, nowait)) + return false; + + shfuncent->stats.numcalls += localent->numcalls; + return true; +} + +/* + * Support function for the SQL-callable pgstat* functions. Returns + * a pointer to the injection point statistics struct. + */ +static PgStat_StatInjEntry * +pgstat_fetch_stat_injentry(const char *name) +{ + PgStat_StatInjEntry *entry = NULL; + + if (!inj_stats_loaded) + return NULL; + + /* Compile the lookup key as a hash of the point name */ + entry = (PgStat_StatInjEntry *) pgstat_fetch_entry(PGSTAT_KIND_INJECTION, + InvalidOid, + PGSTAT_INJ_IDX(name)); + return entry; +} + +/* + * Workhorse to do the registration work, called in _PG_init(). + */ +void +pgstat_register_inj(void) +{ + pgstat_register_kind(PGSTAT_KIND_INJECTION, &injection_stats); + + /* mark stats as loaded */ + inj_stats_loaded = true; +} + +/* + * Report injection point creation. + */ +void +pgstat_create_inj(const char *name) +{ + PgStat_EntryRef *entry_ref; + PgStatShared_InjectionPoint *shstatent; + + /* leave if disabled */ + if (!inj_stats_loaded) + return; + + entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_INJECTION, InvalidOid, + PGSTAT_INJ_IDX(name), false); + shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; + + /* initialize shared memory data */ + memset(&shstatent->stats, 0, sizeof(shstatent->stats)); + pgstat_unlock_entry(entry_ref); +} + +/* + * Report injection point drop. + */ +void +pgstat_drop_inj(const char *name) +{ + /* leave if disabled */ + if (!inj_stats_loaded) + return; + + if (!pgstat_drop_entry(PGSTAT_KIND_INJECTION, InvalidOid, + PGSTAT_INJ_IDX(name))) + pgstat_request_entry_refs_gc(); +} + +/* + * Report statistics for injection point. + * + * This is simple because the set of stats to report currently is simple: + * track the number of times a point has been run. + */ +void +pgstat_report_inj(const char *name) +{ + PgStat_EntryRef *entry_ref; + PgStatShared_InjectionPoint *shstatent; + PgStat_StatInjEntry *statent; + + /* leave if disabled */ + if (!inj_stats_loaded) + return; + + entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_INJECTION, InvalidOid, + PGSTAT_INJ_IDX(name), false); + + shstatent = (PgStatShared_InjectionPoint *) entry_ref->shared_stats; + statent = &shstatent->stats; + + /* Update the injection point statistics */ + statent->numcalls++; + + pgstat_unlock_entry(entry_ref); +} + +/* + * SQL function returning the number of times an injection point + * has been called. + */ +PG_FUNCTION_INFO_V1(injection_points_stats_numcalls); +Datum +injection_points_stats_numcalls(PG_FUNCTION_ARGS) +{ + char *name = text_to_cstring(PG_GETARG_TEXT_PP(0)); + PgStat_StatInjEntry *entry = pgstat_fetch_stat_injentry(name); + + if (entry == NULL) + PG_RETURN_NULL(); + + PG_RETURN_INT64(entry->numcalls); +} diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h new file mode 100644 index 0000000000..3e99705483 --- /dev/null +++ b/src/test/modules/injection_points/injection_stats.h @@ -0,0 +1,23 @@ +/*-------------------------------------------------------------------------- + * + * injection_stats.h + * Definitions for statistics of injection points. + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/test/modules/injection_points/injection_stats.h + * + * ------------------------------------------------------------------------- + */ + +#ifndef INJECTION_STATS +#define INJECTION_STATS + +extern void pgstat_register_inj(void); +extern void pgstat_create_inj(const char *name); +extern void pgstat_drop_inj(const char *name); +extern void pgstat_report_inj(const char *name); + +#endif diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 3c23c14d81..a52fe5121e 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -6,6 +6,7 @@ endif injection_points_sources = files( 'injection_points.c', + 'injection_stats.c', ) if host_system == 'windows' @@ -42,4 +43,12 @@ tests += { 'inplace', ], }, + 'tap': { + 'env': { + 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no', + }, + 'tests': [ + 't/001_stats.pl', + ], + }, } diff --git a/src/test/modules/injection_points/t/001_stats.pl b/src/test/modules/injection_points/t/001_stats.pl new file mode 100644 index 0000000000..7d5a96e522 --- /dev/null +++ b/src/test/modules/injection_points/t/001_stats.pl @@ -0,0 +1,48 @@ + +# Copyright (c) 2024, PostgreSQL Global Development Group + +use strict; +use warnings FATAL => 'all'; +use locale; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Test persistency of statistics generated for injection points. +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +# Node initialization +my $node = PostgreSQL::Test::Cluster->new('master'); +$node->init; +$node->append_conf('postgresql.conf', + "shared_preload_libraries = 'injection_points'"); +$node->start; +$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;'); + +# This should count for two calls. +$node->safe_psql('postgres', + "SELECT injection_points_attach('stats-notice', 'notice');"); +$node->safe_psql('postgres', "SELECT injection_points_run('stats-notice');"); +$node->safe_psql('postgres', "SELECT injection_points_run('stats-notice');"); +my $numcalls = $node->safe_psql('postgres', + "SELECT injection_points_stats_numcalls('stats-notice');"); +is($numcalls, '2', 'number of stats calls'); + +# Restart the node cleanly, stats should still be around. +$node->restart; +$numcalls = $node->safe_psql('postgres', + "SELECT injection_points_stats_numcalls('stats-notice');"); +is($numcalls, '2', 'number of stats after clean restart'); + +# On crash the stats are gone. +$node->stop('immediate'); +$node->start; +$numcalls = $node->safe_psql('postgres', + "SELECT injection_points_stats_numcalls('stats-notice');"); +is($numcalls, '', 'number of stats after clean restart'); + +done_testing(); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 635e6d6e21..5206029e96 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2118,6 +2118,7 @@ PgStatShared_Common PgStatShared_Database PgStatShared_Function PgStatShared_HashEntry +PgStatShared_InjectionPoint PgStatShared_IO PgStatShared_Relation PgStatShared_ReplSlot @@ -2149,6 +2150,7 @@ PgStat_Snapshot PgStat_SnapshotEntry PgStat_StatDBEntry PgStat_StatFuncEntry +PgStat_StatInjEntry PgStat_StatReplSlotEntry PgStat_StatSubEntry PgStat_StatTabEntry -- 2.45.2