On Tue, Feb 24, 2026 at 12:01:30PM +0000, Bertrand Drouvot wrote: > Though I don't think that adresses Michael's concern: "main worries are > mainly around 1), I guess, with the new SIGALRM handler requirements for all > auxiliary processes" in [1].
FWIW, I am still concerned about that, and I have pondered about what we could do here. While reviewing the existing code, one thing that I have noticed we could do is rely on the existing interface of pgstat_report_stat() without changing the existing callers, and not touching at all the flush callbacks. If we begin to require the "force" mode when the routine the called inside a transaction block, things seem to work pretty smoothly in combination with a stats kind property that allows the stats data to be flushed if we are inside a transaction while a report happens. And it is possible to enforce checks inside pgstat_report_stat() as well. So please find attached my shot at that: - Introduction of a new system function called pg_stat_report(), based on a procsignal that gives a way to signal backends for a stats update, reusing the existing code where we only do flushes when idle and not in a transaction. - Property that tracks under which contexts the reports are allowed. Here I have decided to stick with simple, as in only allowing IO and WAL stats to be flushed if we are inside a transaction. Using that, I have done a few tests with three backends: - One with a long-running transaction. - One that periodically triggers the reports. - One that looks at IO and WAL stat. And the third session is able to get refreshes for both of these stats kinds, while the other stats remain the same. Note that this is a WIP, which is check-world stable. One thing that sticks a bit in mind now is that perhaps we should not allow the function for auxiliary processes at all. A second thing is the requirement of allowing partial flushes at the end of the report path, which is OK because the variable-sized stats can have pending data. Perhaps we should just have pgstat_flush_pending_entries() provide a correct status in line with the property set in a stats kind when we try a flush while in a transaction. Thoughts or tomatoes? -- Michael
From c5aeb083265efbd6041ed3868b669997d4430760 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Mon, 16 Mar 2026 15:12:24 +0900 Subject: [PATCH] Add support for transient stats updates This introduces a function able to push stats updates, with a new stats kind property to allow stats to be updated while in a transaction. --- src/include/catalog/pg_proc.dat | 7 +++ src/include/miscadmin.h | 1 + src/include/storage/procsignal.h | 1 + src/include/utils/pgstat_internal.h | 16 ++++++ src/backend/storage/ipc/procsignal.c | 16 ++++++ src/backend/tcop/postgres.c | 11 +++- src/backend/utils/activity/pgstat.c | 42 ++++++++++++-- src/backend/utils/adt/pgstatfuncs.c | 58 ++++++++++++++++++++ src/backend/utils/init/globals.c | 1 + src/test/regress/expected/misc_functions.out | 52 ++++++++++++++++++ src/test/regress/sql/misc_functions.sql | 29 ++++++++++ doc/src/sgml/func/func-admin.sgml | 16 ++++++ 12 files changed, 242 insertions(+), 8 deletions(-) diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 361e2cfffebe..85869154657a 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -8692,6 +8692,13 @@ prosrc => 'pg_log_backend_memory_contexts', proacl => '{POSTGRES=X}' }, +# request an update of statistics +{ oid => '8789', descr => 'have the specified backend push a pgstats update', + proname => 'pg_stat_report', provolatile => 'v', + prorettype => 'bool', proargtypes => 'int4', + prosrc => 'pg_stat_report', + proacl => '{POSTGRES=X}' }, + # non-persistent series generator { oid => '1066', descr => 'non-persistent series generator', proname => 'generate_series', prorows => '1000', diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index f16f35659b9b..a245cdd79d17 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -94,6 +94,7 @@ extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending; extern PGDLLIMPORT volatile sig_atomic_t TransactionTimeoutPending; extern PGDLLIMPORT volatile sig_atomic_t IdleSessionTimeoutPending; extern PGDLLIMPORT volatile sig_atomic_t ProcSignalBarrierPending; +extern PGDLLIMPORT volatile sig_atomic_t ProcSignalStatsUpdatePending; extern PGDLLIMPORT volatile sig_atomic_t LogMemoryContextPending; extern PGDLLIMPORT volatile sig_atomic_t IdleStatsUpdateTimeoutPending; diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h index 348fba53a931..d2e60403ad52 100644 --- a/src/include/storage/procsignal.h +++ b/src/include/storage/procsignal.h @@ -34,6 +34,7 @@ typedef enum PROCSIG_PARALLEL_MESSAGE, /* message from cooperating parallel backend */ PROCSIG_WALSND_INIT_STOPPING, /* ask walsenders to prepare for shutdown */ PROCSIG_BARRIER, /* global barrier interrupt */ + PROCSIG_STATS_UPDATE, /* pgstats update */ PROCSIG_LOG_MEMORY_CONTEXT, /* ask backend to log the memory contexts */ PROCSIG_PARALLEL_APPLY_MESSAGE, /* Message from parallel apply workers */ PROCSIG_RECOVERY_CONFLICT, /* backend is blocking recovery, check diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 9b8fbae00ed5..fa4cc4fe9c60 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -224,6 +224,17 @@ typedef struct PgStat_SubXactStatus PgStat_TableXactStatus *first; /* head of list for this subxact */ } PgStat_SubXactStatus; +/* + * Contexts related to the report of the statistics, defined as properties + * of PgStat_KindInfo.report_context. These define when a stats report is + * allowed depending on the stats kind and the context where + * pgstat_report_stat() is called. + */ + +/* report allowed while idle, outside a transaction (default) */ +#define PGSTAT_REPORT_IDLE 0x00 +/* report of stats data allowed within a transaction */ +#define PGSTAT_REPORT_TRANSACTION 0x01 /* * Metadata for a specific kind of statistics. @@ -251,6 +262,11 @@ typedef struct PgStat_KindInfo */ bool track_entry_count:1; + /* + * Contexts allowed for the report of this stats kind data. + */ + bits32 report_context; + /* * The size of an entry in the shared stats hash table (pointed to by * PgStatShared_HashEntry->body). For fixed-numbered statistics, this is diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index 7e017c8d53b5..e1bff2185933 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -490,6 +490,19 @@ HandleProcSignalBarrierInterrupt(void) /* latch will be set by procsignal_sigusr1_handler */ } +/* + * Handle receipt of an interrupt indicating that a stats update has been + * requested. This routine only gets called when PROCSIG_STATS_UPDATE is + * sent. + */ +static void +HandleProcSignalStatsUpdateInterrupt(void) +{ + InterruptPending = true; + ProcSignalStatsUpdatePending = true; + /* latch will be set by procsignal_sigusr1_handler */ +} + /* * Perform global barrier related interrupt checking. * @@ -694,6 +707,9 @@ procsignal_sigusr1_handler(SIGNAL_ARGS) if (CheckProcSignal(PROCSIG_BARRIER)) HandleProcSignalBarrierInterrupt(); + if (CheckProcSignal(PROCSIG_STATS_UPDATE)) + HandleProcSignalStatsUpdateInterrupt(); + if (CheckProcSignal(PROCSIG_LOG_MEMORY_CONTEXT)) HandleLogMemoryContextInterrupt(); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index d01a09dd0c41..ddd57dfea780 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3555,12 +3555,17 @@ ProcessInterrupts(void) /* * If there are pending stats updates and we currently are truly idle - * (matching the conditions in PostgresMain(), report stats now. + * (matching the conditions in PostgresMain(), or if a status update has + * been requested, report stats now. */ if (IdleStatsUpdateTimeoutPending && - DoingCommandRead && !IsTransactionOrTransactionBlock()) + DoingCommandRead && !IsTransactionOrTransactionBlock() || + ProcSignalStatsUpdatePending) { - IdleStatsUpdateTimeoutPending = false; + if (IdleStatsUpdateTimeoutPending) + IdleStatsUpdateTimeoutPending = false; + if (ProcSignalStatsUpdatePending) + ProcSignalStatsUpdatePending = false; pgstat_report_stat(true); } diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 11bb71cad5ad..5521e96d0cae 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -436,6 +436,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .fixed_amount = true, .write_to_file = true, + .report_context = PGSTAT_REPORT_TRANSACTION, .snapshot_ctl_off = offsetof(PgStat_Snapshot, io), .shared_ctl_off = offsetof(PgStat_ShmemControl, io), @@ -470,6 +471,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .fixed_amount = true, .write_to_file = true, + .report_context = PGSTAT_REPORT_TRANSACTION, .snapshot_ctl_off = offsetof(PgStat_Snapshot, wal), .shared_ctl_off = offsetof(PgStat_ShmemControl, wal), @@ -698,8 +700,9 @@ pgstat_initialize(void) * a timeout after which to call pgstat_report_stat(true), but are not * required to do so. * - * Note that this is called only when not within a transaction, so it is fair - * to use transaction stop time as an approximation of current time. + * Note that when this is called only when not within a transaction, we use + * the transaction stop time as an approximation of current time. "force" + * is required when this is called within a transaction. */ long pgstat_report_stat(bool force) @@ -709,9 +712,14 @@ pgstat_report_stat(bool force) bool partial_flush; TimestampTz now; bool nowait; + bool is_xact = IsTransactionOrTransactionBlock(); pgstat_assert_is_up(); - Assert(!IsTransactionOrTransactionBlock()); + /* + * "force" is required if this routine is called inside a transaction + * block. + */ + Assert(!is_xact || force); /* "absorb" the forced flush even if there's nothing to flush */ if (pgStatForceNextFlush) @@ -789,6 +797,11 @@ pgstat_report_stat(bool force) if (!kind_info->flush_static_cb) continue; + /* Skip if this stats kind cannot be flushed in a transaction */ + if (is_xact && + (kind_info->report_context & PGSTAT_REPORT_TRANSACTION) == 0) + continue; + partial_flush |= kind_info->flush_static_cb(nowait); } } @@ -801,8 +814,11 @@ pgstat_report_stat(bool force) */ if (partial_flush) { - /* force should have prevented us from getting here */ - Assert(!force); + /* + * force should have prevented us from getting here, and partial + * flushes are accepted inside a transaction. + */ + Assert(!force || is_xact); /* remember since when stats have been pending */ if (pending_since == 0) @@ -1351,6 +1367,7 @@ pgstat_flush_pending_entries(bool nowait) { bool have_pending = false; dlist_node *cur = NULL; + bool is_xact = IsTransactionOrTransactionBlock(); /* * Need to be a bit careful iterating over the list of pending entries. @@ -1377,6 +1394,21 @@ pgstat_flush_pending_entries(bool nowait) Assert(!kind_info->fixed_amount); Assert(kind_info->flush_pending_cb != NULL); + /* Skip if this stats kind cannot be flushed while in a transaction */ + if (is_xact && + (kind_info->report_context & PGSTAT_REPORT_TRANSACTION) == 0) + { + have_pending = true; + + if (dlist_has_next(&pgStatPending, cur)) + next = dlist_next_node(&pgStatPending, cur); + else + next = NULL; + + cur = next; + continue; + } + /* flush the stats, if possible */ did_flush = kind_info->flush_pending_cb(entry_ref, nowait); diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index bad5642d9c90..b27a5eb58e18 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -28,6 +28,7 @@ #include "replication/logicallauncher.h" #include "storage/proc.h" #include "storage/procarray.h" +#include "storage/procsignal.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/timestamp.h" @@ -2325,3 +2326,60 @@ pg_stat_have_stats(PG_FUNCTION_ARGS) PG_RETURN_BOOL(pgstat_have_entry(kind, dboid, objid)); } + +/* + * pg_stat_report + * Signal a backend or an auxiliary process to have it push an update + * of its statistics data. + * + * By default, only superusers are allowed to signal to log the memory + * contexts because allowing any users to issue this request at an unbounded + * rate would cause lots of log messages and which can lead to denial of + * service. Additional roles can be permitted with GRANT. + * + * On receipt of this signal, a backend or an auxiliary process sets the flag + * in the signal handler, which causes the next CHECK_FOR_INTERRUPTS() + * or process-specific interrupt handler to update their statistics. + */ +Datum +pg_stat_report(PG_FUNCTION_ARGS) +{ + int pid = PG_GETARG_INT32(0); + PGPROC *proc; + ProcNumber procNumber = INVALID_PROC_NUMBER; + + /* + * See if the process with given pid is a backend or an auxiliary process. + */ + proc = BackendPidGetProc(pid); + if (proc == NULL) + proc = AuxiliaryPidGetProc(pid); + + /* + * BackendPidGetProc() and AuxiliaryPidGetProc() return NULL if the pid + * isn't valid; but by the time we reach kill(), a process for which we + * get a valid proc here might have terminated on its own. This is OK, + * as at shutdown processes flush their stats. + */ + if (proc == NULL) + { + /* + * This is just a warning so a loop-through-resultset will not abort + * if one backend terminated on its own during the run. + */ + ereport(WARNING, + (errmsg("PID %d is not a PostgreSQL server process", pid))); + PG_RETURN_BOOL(false); + } + + procNumber = GetNumberFromPGProc(proc); + if (SendProcSignal(pid, PROCSIG_STATS_UPDATE, procNumber) < 0) + { + /* Again, just a warning to allow loops */ + ereport(WARNING, + (errmsg("could not send signal to process %d: %m", pid))); + PG_RETURN_BOOL(false); + } + + PG_RETURN_BOOL(true); +} diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 36ad708b3602..22960ee3b27b 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -38,6 +38,7 @@ volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false; volatile sig_atomic_t TransactionTimeoutPending = false; volatile sig_atomic_t IdleSessionTimeoutPending = false; volatile sig_atomic_t ProcSignalBarrierPending = false; +volatile sig_atomic_t ProcSignalStatsUpdatePending = false; volatile sig_atomic_t LogMemoryContextPending = false; volatile sig_atomic_t IdleStatsUpdateTimeoutPending = false; volatile uint32 InterruptHoldoffCount = 0; diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index 6c03b1a79d75..68a0e1e02bcc 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -397,6 +397,58 @@ REVOKE EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) FROM regress_log_memory; DROP ROLE regress_log_memory; -- +-- pg_stat_report +-- +-- check execution +SELECT pg_stat_report(pg_backend_pid()); + pg_stat_report +---------------- + t +(1 row) + +SELECT pg_stat_report(pid) FROM pg_stat_activity + WHERE backend_type = 'checkpointer'; + pg_stat_report +---------------- + t +(1 row) + +-- Check privileges +CREATE ROLE regress_stat_report; +SELECT has_function_privilege('regress_stat_report', + 'pg_stat_report(integer)', 'EXECUTE'); -- no + has_function_privilege +------------------------ + f +(1 row) + +-- Fails +SET ROLE regress_stat_report; +SELECT pg_stat_report(pg_backend_pid()); +ERROR: permission denied for function pg_stat_report +RESET ROLE; +-- Access granted, then function works +GRANT EXECUTE ON FUNCTION pg_stat_report(integer) + TO regress_stat_report; +SELECT has_function_privilege('regress_stat_report', + 'pg_stat_report(integer)', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + +SET ROLE regress_stat_report; +SELECT pg_stat_report(pg_backend_pid()); + pg_stat_report +---------------- + t +(1 row) + +RESET ROLE; +REVOKE EXECUTE ON FUNCTION pg_stat_report(integer) + FROM regress_stat_report; +DROP ROLE regress_stat_report; +-- -- Test some built-in SRFs -- -- The outputs of these are variable, so we can't just print their results diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index 35b7983996c4..fe366904c9e8 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -154,6 +154,35 @@ REVOKE EXECUTE ON FUNCTION pg_log_backend_memory_contexts(integer) DROP ROLE regress_log_memory; +-- +-- pg_stat_report +-- + +-- check execution +SELECT pg_stat_report(pg_backend_pid()); +SELECT pg_stat_report(pid) FROM pg_stat_activity + WHERE backend_type = 'checkpointer'; + +-- Check privileges +CREATE ROLE regress_stat_report; +SELECT has_function_privilege('regress_stat_report', + 'pg_stat_report(integer)', 'EXECUTE'); -- no +-- Fails +SET ROLE regress_stat_report; +SELECT pg_stat_report(pg_backend_pid()); +RESET ROLE; +-- Access granted, then function works +GRANT EXECUTE ON FUNCTION pg_stat_report(integer) + TO regress_stat_report; +SELECT has_function_privilege('regress_stat_report', + 'pg_stat_report(integer)', 'EXECUTE'); -- yes +SET ROLE regress_stat_report; +SELECT pg_stat_report(pg_backend_pid()); +RESET ROLE; +REVOKE EXECUTE ON FUNCTION pg_stat_report(integer) + FROM regress_stat_report; +DROP ROLE regress_stat_report; + -- -- Test some built-in SRFs -- diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml index 210b1118bdf7..114202c4fc19 100644 --- a/doc/src/sgml/func/func-admin.sgml +++ b/doc/src/sgml/func/func-admin.sgml @@ -220,6 +220,22 @@ </para></entry> </row> + <row> + <entry role="func_table_entry"><para role="func_signature"> + <indexterm> + <primary>pg_stat_report</primary> + </indexterm> + <function>pg_log_backend_memory_contexts</function> ( <parameter>pid</parameter> <type>integer</type> ) + <returnvalue>boolean</returnvalue> + </para> + <para> + Requests to update the statistics computed by the backend with the + specified process ID. This function can send the request to + backends and auxiliary processes except logger, pushing an + update of the statistics data. + </para></entry> + </row> + <row> <entry role="func_table_entry"><para role="func_signature"> <indexterm> -- 2.53.0
signature.asc
Description: PGP signature
