Re: [BUGS] BUG #10123: Weird entries in pg_stat_activity

Hi,

On 2024-07-04 14:54:01 +0900, Fujii Masao wrote:
> @@ -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();

This seems odd to me. ProcessClientReadInterrupt() can be called a lot, why do
we need to reset the statement start timestamp even if nothing is happening here?

> @@ -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.

Calls to pgstat_report_activity() aren't exactly free. This substantially
increases the number of calls to it for common workloads. There got to be a
more targeted way of dealing with this.

Greetings,

Andres Freund