| From: | Sami Imseih <samimseih(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Lukas Fittl <lukas(at)fittl(dot)com> |
| Subject: | Re: pgstat: Flush some statistics within running transactions, take 2 |
| Date: | 2026-07-30 19:41:58 |
| Message-ID: | CAA5RZ0u84eMFeFWMaEo0D84ed3jF_RY0=RWD8tDwQWsNsU1qvA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
The discussion for mid-transaction flushes ended up taking place in
the pg_stat_statements scalability thread [1], but I rather not have
that thread become the place to review prerequisite patches. To keep
things easier to follow, I am moving this patch back to its dedicated
thread. The pg_stat_statements
scalability work will follow as a patch set on top of this once this
settles.
Attaching v2 of this patch. It incorporates the feedback from the
discussion in the scalability thread [1].
Most of the structural changes here come from Kyotaro's review.
In v1 I pushed back on the context flag and the return value
semantics, but after sitting with it I think he was right on both
counts. The two points I adopted:
- The flush_pending_cb signature now takes an explicit xact_boundary
flag. As Bertrand argued, this makes the changed contract visible
to extensions implementing custom stats kinds, rather than relying
on the callback to infer it from internal state.
- The callback returns a PgStat_FlushResult enum (DONE,
LOCK_CONFLICT, PARTIAL) instead of giving the return bool multiple
meanings. The caller combines this with the flush context to decide
whether to remove the entry from the pending list. This separates
"intentionally partial flush due to mid-transaction flushes" from
"partially flushed due to lock conflict." Currently both cases will
require the same action to be taken, namely to retry the flush,
but keeping them separate makes it straightforward to handle them
differently in the future if needed.
Also changed from v1, the existing pg_stat_force_next_flush() is
extended instead of introducing pg_stat_report_anytime(). When called
in-transaction, it calls pgstat_report_stat(true) directly.
As before, only non-transactional counters are flushed
mid-transaction when a relation has active transaction state. In v2
however, the decision is based on both the xact_boundary flag and
lstats->trans != NULL.
lastscan uses GetCurrentStatementStartTimestamp() for in-transaction
flushes since GetCurrentTransactionStopTimestamp() is not available
in that context. v2 updates the docs for last_seq_scan/last_idx_scan
to reflect this.
Also v2 adds more comprehensive testing, including for ROLLBACK and
TRUNCATE.
As a follow-up item, I am also attaching an example that shows how
the flush API can be used to automatically flush non-transactional
stats at statement boundaries during long-running transactions,
throttled to once every 10 seconds. This is not part of the main
proposal, just a demonstration of what becomes possible on top of it.
--
Sami Imseih
Amazon Web Services (AWS)
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-pgstat-Allow-pg_stat_force_next_flush-to-work-in-.patch | application/octet-stream | 44.3 KB |
| nocfbot-0001-pgstat-Flush-non-transactional-stats-at-statement-bo.patch | application/octet-stream | 2.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-07-30 19:56:30 | Re: Improve pg_stat_statements scalability |
| Previous Message | Bharath Rupireddy | 2026-07-30 19:12:00 | Re: [PATCH] Release replication slot on error in SQL-callable slot functions |