From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Fujii Masao <masao.fujii@gmail.com>
To: maxim.boguk@gmail.com
Cc: pgsql-bugs@postgresql.org
Subject: Re: [BUGS] BUG #10123: Weird entries in pg_stat_activity
Date: Thu, 4 Jul 2024 14:54:01 +0900
Message-ID: <2153c106-8979-430d-ba86-34d35e565ab4@oss.nttdata.com> (raw)
In-Reply-To: <CAHGQGwEQWk6J+vqQnnVFFngtMdMuw-AwuEFnq=u3Wb2STXu3TA@mail.gmail.com>
References: <20140424101827.2714.39486@wrigleys.postgresql.org>
<CAHGQGwEQWk6J+vqQnnVFFngtMdMuw-AwuEFnq=u3Wb2STXu3TA@mail.gmail.com>
On 2024/07/04 0:14, Fujii Masao wrote:
> I'm not sure if these are bugs. However, to prevent confusion from
> these odd pg_stat_activity entries, I suggest the following changes:
>
> 1. Modify ProcessCatchupInterrupt() and RemoveTempRelationsCallback()
> to reset stmtStartTimestamp (timestamp of the last query
> executed) to 0
> before starting the transaction. This way, during the transaction,
> records with state="idle" and xact_start=NULL would be observed.
>
> 2. Update exec_describe_statement_message() and
> exec_describe_portal_message()
> to change the state to "active," similar to exec_simple_query().
> This would result in records with state="active" and
> xact_start="current timestamp"
> during those operations.
Attached patch does these things. I will register it into the next CF.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:v1-0001-Prevent-incorrect-entries-in-pg_stat_activity.patch (text/plain, 4.9K) download
From e687ec472c5478716b31189e25e942ff3abf11bf Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Thu, 4 Jul 2024 12:09:39 +0900
Subject: [PATCH v1] Prevent incorrect entries in pg_stat_activity.
In pg_stat_activity, when the backend's state is "idle",
no transaction should be running, and xact_start
(indicating the transaction start time) should be NULL.
Previously, during the processing of "Describe" messages,
removing temp relations at exit, and handling client read interrupts,
the backend did not change its state from "idle" but still set xact_start,
causing incorrect pg_stat_activity entries. Additionally,
during temp relation removal or interrupt processing,
xact_start could incorrectly reflect the timestamp of
the last query executed.
To fix this, the backend now sets its state to "active"
when processing a "Describe" message.
For interrupts and temp relation removal, since these
involve brief transactions, it is unnecessary to treat them
as normal transactions. Instead of changing the state to
"active" and setting xact_start to the current timestamp,
this commit ensures the backend resets xact_start to NULL
in these cases.
---
src/backend/access/transam/xact.c | 9 +++++++++
src/backend/catalog/namespace.c | 10 ++++++++++
src/backend/tcop/postgres.c | 15 +++++++++++++++
src/include/access/xact.h | 1 +
4 files changed, 35 insertions(+)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.cindex d119ab909d..58921bc469 100644--- a/src/backend/access/transam/xact.c+++ b/src/backend/access/transam/xact.c@@ -917,6 +917,15 @@ SetCurrentStatementStartTimestamp(void)
Assert(stmtStartTimestamp != 0);
}
+/*+ * ResetCurrentStatementStartTimestamp+ */+void+ResetCurrentStatementStartTimestamp(void)+{+ stmtStartTimestamp = 0;+}+
/*
* GetCurrentTransactionNestLevel
*
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.cindex a2510cf80c..78d530263d 100644--- a/src/backend/catalog/namespace.c+++ b/src/backend/catalog/namespace.c@@ -4610,6 +4610,16 @@ RemoveTempRelationsCallback(int code, Datum arg)
{
if (OidIsValid(myTempNamespace)) /* should always be true */
{
+ /*+ * Reset statement_timestamp() to 0 so that subsequent+ * StartTransactionCommand() will set pg_stat_activity.xact_start to+ * NULL. Otherwise, xact_start could be set to statement_timestamp()+ * that may indicate the timestamp of the last query executed, while+ * the backend's state remains "idle," leading to incorrect "idle"+ * entries with non-NULL xact_start in pg_stat_activity.+ */+ ResetCurrentStatementStartTimestamp();+
/* Need to ensure we have a usable transaction. */
AbortOutOfAnyTransaction();
StartTransactionCommand();
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.cindex e39c6804a7..04046f41d8 100644--- a/src/backend/tcop/postgres.c+++ b/src/backend/tcop/postgres.c@@ -515,6 +515,17 @@ ProcessClientReadInterrupt(bool blocked)
/* Check for general interrupts that arrived before/while reading */
CHECK_FOR_INTERRUPTS();
+ /*+ * Reset statement_timestamp() to 0 so that ProcessCatchupInterrupt()+ * and ProcessNotifyInterrupt() will set pg_stat_activity.xact_start+ * to NULL when they start new transaction. Otherwise, xact_start+ * could be set to statement_timestamp() that may indicate the+ * timestamp of the last query executed, while the backend's state+ * remains "idle," leading to incorrect "idle" entries with non-NULL+ * xact_start in pg_stat_activity.+ */+ ResetCurrentStatementStartTimestamp();+
/* Process sinval catchup interrupts, if any */
if (catchupInterruptPending)
ProcessCatchupInterrupt();
@@ -2598,6 +2609,8 @@ exec_describe_statement_message(const char *stmt_name)
{
CachedPlanSource *psrc;
+ pgstat_report_activity(STATE_RUNNING, NULL);+
/*
* Start up a transaction command. (Note that this will normally change
* current memory context.) Nothing happens if we are already in one.
@@ -2692,6 +2705,8 @@ exec_describe_portal_message(const char *portal_name)
{
Portal portal;
+ pgstat_report_activity(STATE_RUNNING, NULL);+
/*
* Start up a transaction command. (Note that this will normally change
* current memory context.) Nothing happens if we are already in one.
diff --git a/src/include/access/xact.h b/src/include/access/xact.hindex 6d4439f052..84745575be 100644--- a/src/include/access/xact.h+++ b/src/include/access/xact.h@@ -450,6 +450,7 @@ extern TimestampTz GetCurrentTransactionStartTimestamp(void);
extern TimestampTz GetCurrentStatementStartTimestamp(void);
extern TimestampTz GetCurrentTransactionStopTimestamp(void);
extern void SetCurrentStatementStartTimestamp(void);
+extern void ResetCurrentStatementStartTimestamp(void);
extern int GetCurrentTransactionNestLevel(void);
extern bool TransactionIdIsCurrentTransactionId(TransactionId xid);
extern void CommandCounterIncrement(void);
--
2.45.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: masao.fujii@oss.nttdata.com, masao.fujii@gmail.com, maxim.boguk@gmail.com
Subject: Re: [BUGS] BUG #10123: Weird entries in pg_stat_activity
In-Reply-To: <2153c106-8979-430d-ba86-34d35e565ab4@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox