> > This adds a planId to pg_stat_get_activity ( not pg_stat_activity ). > > An extension > > can offer its own view, similar to pg_stat_activity, which can include > > planId. > > > > Note that there are no documentation updates required here as we don't > > have per-field documentation for pg_stat_get_activity. > > > > These 2 patches can probably be combined , but will leave them like > > this for now. > > The split makes more sense to me, and FWIW I see more value in 0001 > that provides an anchor on the backend side for extensions to plug in > a plan ID that can be tracked across multiple layers.
Yes, I think this is a step in the right direction for this functionality. > I am less > convinced about the need for 0002 that adds this information to > pg_stat_activity at this stage, as the getter routine in 0001 is > enough to know what's happening, roughly. I agree. I don't think exposing this in a system view is required at this time. Once extensions start to use it, and it becomes obvious it should be exposed in a system view, that discussion can be had at that time. > Hmm. Shouldn't we document all such expectations, > which are similar to a query ID? I had a lazy comment :) * For the same reasons as the query identifiers (see above), but, I went ahead and commented it similar to how we document pgstat_report_query_id and pgstat_get_my_query_id routines. attached is v2-0001 -- Sami Imseih Amazon Web Services (AWS)
From 53b122db056154a2cd27ebc446545fbb241ba570 Mon Sep 17 00:00:00 2001 From: "Sami Imseih (AWS)" <simseih@dev-dsk-simseih-1d-3940b79e.us-east-1.amazon.com> Date: Wed, 19 Feb 2025 15:56:21 +0000 Subject: [PATCH v2 1/1] Allow plugins to set a 64-bit plan identifier There are extensions that wish to compute a plan identifier to distinguish different plans that a particular query may use. Currently, there is no way for such extensions to make this plan identifier available to the core for reporting and other potentially interesting use cases such as plan management. This patch allows extensions to do this by providing a mechanism to track this identifier in the backend_status as well as in PlannedStmt. The reset of the plan identifier is controlled by core and follows the same code path as the query identifier added in 4f0b0966c8. Discussion: https://www.postgresql.org/message-id/flat/CAA5RZ0vyWd4r35uUBUmhngv8XqeiJUkJDDKkLf5LCoWxv-t_pw%40mail.gmail.com --- src/backend/executor/execParallel.c | 1 + src/backend/tcop/postgres.c | 23 ++++++++ src/backend/utils/activity/backend_status.c | 58 +++++++++++++++++++++ src/include/nodes/plannodes.h | 3 ++ src/include/utils/backend_status.h | 5 ++ 5 files changed, 90 insertions(+) diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index e9337a97d17..39c990ae638 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -175,6 +175,7 @@ ExecSerializePlan(Plan *plan, EState *estate) pstmt = makeNode(PlannedStmt); pstmt->commandType = CMD_SELECT; pstmt->queryId = pgstat_get_my_query_id(); + pstmt->planId = pgstat_get_my_plan_id(); pstmt->hasReturning = false; pstmt->hasModifyingCTE = false; pstmt->canSetTag = true; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 0554a4ae3c7..441a631a887 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1107,6 +1107,7 @@ exec_simple_query(const char *query_string) size_t cmdtaglen; pgstat_report_query_id(0, true); + pgstat_report_plan_id(0, true); /* * Get the command name for use in status display (it also becomes the @@ -2016,6 +2017,17 @@ exec_bind_message(StringInfo input_message) */ cplan = GetCachedPlan(psrc, params, NULL, NULL); + foreach(lc, cplan->stmt_list) + { + PlannedStmt *plan = lfirst_node(PlannedStmt, lc); + + if (plan->planId != UINT64CONST(0)) + { + pgstat_report_plan_id(plan->planId, false); + break; + } + } + /* * Now we can define the portal. * @@ -2170,6 +2182,17 @@ exec_execute_message(const char *portal_name, long max_rows) } } + foreach(lc, portal->stmts) + { + PlannedStmt *stmt = lfirst_node(PlannedStmt, lc); + + if (stmt->planId != UINT64CONST(0)) + { + pgstat_report_plan_id(stmt->planId, false); + break; + } + } + cmdtagname = GetCommandTagNameAndLen(portal->commandTag, &cmdtaglen); set_ps_display_with_len(cmdtagname, cmdtaglen); diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 7681b4ba5a9..8ce743899ce 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -321,6 +321,7 @@ pgstat_bestart_initial(void) lbeentry.st_progress_command = PROGRESS_COMMAND_INVALID; lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = UINT64CONST(0); + lbeentry.st_plan_id = UINT64CONST(0); /* * we don't zero st_progress_param here to save cycles; nobody should @@ -599,6 +600,7 @@ pgstat_report_activity(BackendState state, const char *cmd_str) /* st_xact_start_timestamp and wait_event_info are also disabled */ beentry->st_xact_start_timestamp = 0; beentry->st_query_id = UINT64CONST(0); + beentry->st_plan_id = UINT64CONST(0); proc->wait_event_info = 0; PGSTAT_END_WRITE_ACTIVITY(beentry); } @@ -659,7 +661,10 @@ pgstat_report_activity(BackendState state, const char *cmd_str) * identifier. */ if (state == STATE_RUNNING) + { beentry->st_query_id = UINT64CONST(0); + beentry->st_plan_id = UINT64CONST(0); + } if (cmd_str != NULL) { @@ -710,6 +715,44 @@ pgstat_report_query_id(uint64 query_id, bool force) PGSTAT_END_WRITE_ACTIVITY(beentry); } +/* -------- + * pgstat_report_plan_id() - + * + * Called to update top-level plan identifier. + * -------- + */ +void +pgstat_report_plan_id(uint64 plan_id, bool force) +{ + volatile PgBackendStatus *beentry = MyBEEntry; + + /* + * if track_activities is disabled, st_plan_id should already have been + * reset + */ + if (!beentry || !pgstat_track_activities) + return; + + /* + * We only report the top-level plan identifiers. The stored plan_id is + * reset when a backend calls pgstat_report_activity(STATE_RUNNING), or + * with an explicit call to this function using the force flag. If the + * saved plan identifier is not zero it means that it's not a top-level + * command, so ignore the one provided unless it's an explicit call to + * reset the identifier. + */ + if (beentry->st_plan_id != 0 && !force) + return; + + /* + * Update my status entry, following the protocol of bumping + * st_changecount before and after. We use a volatile pointer here to + * ensure the compiler doesn't try to get cute. + */ + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); + beentry->st_plan_id = plan_id; + PGSTAT_END_WRITE_ACTIVITY(beentry); +} /* ---------- * pgstat_report_appname() - @@ -1106,6 +1149,21 @@ pgstat_get_my_query_id(void) return MyBEEntry->st_query_id; } +/* ---------- + * pgstat_get_my_plan_id() - + * + * Return current backend's plan identifier. Currently, only plugins + * have a use for this routine. + */ +uint64 +pgstat_get_my_plan_id(void) +{ + if (!MyBEEntry) + return 0; + + return MyBEEntry->st_plan_id; +} + /* ---------- * pgstat_get_backend_type_by_proc_number() - * diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index f78bffd90cf..618b9248a01 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -55,6 +55,9 @@ typedef struct PlannedStmt /* query identifier (copied from Query) */ uint64 queryId; + /* plan identifier (set by plugins) */ + uint64 planId; + /* is it insert|update|delete|merge RETURNING? */ bool hasReturning; diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index 1c9b4fe14d0..c11e6835659 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -171,6 +171,9 @@ typedef struct PgBackendStatus /* query identifier, optionally computed using post_parse_analyze_hook */ uint64 st_query_id; + + /* plan identifier, optionally computed by plugins */ + uint64 st_plan_id; } PgBackendStatus; @@ -319,6 +322,7 @@ extern void pgstat_clear_backend_activity_snapshot(void); /* Activity reporting functions */ extern void pgstat_report_activity(BackendState state, const char *cmd_str); extern void pgstat_report_query_id(uint64 query_id, bool force); +extern void pgstat_report_plan_id(uint64 plan_id, bool force); extern void pgstat_report_tempfile(size_t filesize); extern void pgstat_report_appname(const char *appname); extern void pgstat_report_xact_timestamp(TimestampTz tstamp); @@ -326,6 +330,7 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser); extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen); extern uint64 pgstat_get_my_query_id(void); +extern uint64 pgstat_get_my_plan_id(void); extern BackendType pgstat_get_backend_type_by_proc_number(ProcNumber procNumber); -- 2.47.1
