Re: Re[2]: [PATCH] Add last_executed timestamp to pg_stat_statements

From: Pavlo Golub <pavlo(dot)golub(at)cybertec(dot)at>
To: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: Sami Imseih <samimseih(at)gmail(dot)com>, Christoph Berg <myon(at)debian(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Re[2]: [PATCH] Add last_executed timestamp to pg_stat_statements
Date: 2026-02-05 17:35:27
Message-ID: CAK7ymc+E00zKC9Da+BK2se=O2RiOUkdW5dz4iAynD1Vj=Pwexg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

Thank you to Sami, Christoph, and Bertrand for the thorough review and valuable
feedback on v1. I've prepared a v2 patch that addresses all the concerns raised.

You're absolutely right that calling `GetCurrentTimestamp()` while holding a
spinlock is unacceptable.

I've adopted Sami's suggestion to use `GetCurrentStatementStartTimestamp()`
instead. While Christoph correctly points out that this means we're capturing
statement start time rather than end time, I believe this is an acceptable
trade-off for several reasons:
- the vast majority of statements complete quickly (well under typical polling
intervals of 1-5 minutes);
- long-running queries will appear in the next poll anyway;
- deferring timestamp updates adds significant complexity;
- and most importantly zero performance overhead vs a kernel call per execution.

For the monitoring use case, what matters is knowing has this statement been
active since my last poll? The start time serves this purpose effectively.

I've renamed the column to `stats_last_updated` as Christoph suggested. This
matches the existing "stats_since" column for consistency. Following Christoph's
suggestion, I've also moved it to the end of the view.

Per Sami's feedback, I've moved the timestamp from the `Counters`
struct directly
to `pgssEntry`. This is more semantically correct since it's metadata about the
entry rather than a statistical counter.

I've updated `PGSS_FILE_HEADER` to 0x20260205.

Among other changes:
- fixed all whitespace errors
- moved tests to `entry_timestamp.sql` as suggested
- added documentation notice about nested statement behavior
- the timestamp is now set outside the spinlock-protected section

Sami mentioned this was discussed in 2017 [0]. Looking at that thread, the main
concerns were overhead and use cases. I believe we've addressed both:
- zero overhead by using existing statement start timestamp
- clear, documented use case from production monitoring tools (pgwatch, etc.)

[0] https://www.postgresql.org/message-id/flat/CA+TgmoZgZMeuN8t9pawSt6M=mvxKiAZ4CvPofBWwwVWeZwHe4w(at)mail(dot)gmail(dot)com

I've attached the v2 patch. All existing tests pass.

I'd appreciate further review and feedback.

Thanks,
Pavlo Golub

Attachment Content-Type Size
v2-0001-pg_stat_statements-Add-stats_last_updated-column.patch application/octet-stream 17.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2026-02-05 17:36:00 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible
Previous Message Jacob Champion 2026-02-05 17:26:34 Re: client_connection_check_interval default value