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. [1] https://www.postgresql.org/message-id/flat/CAA5RZ0sQ%2BgDn-J85j1FzOdL1YjVYRegpmQpDiah1%3DREWZSZj%2BQ%40mail.gmail.com -- Sami Imseih Amazon Web Services (AWS)
v2-0001-pgstat-Allow-pg_stat_force_next_flush-to-work-in-.patch
Description: Binary data
nocfbot-0001-pgstat-Flush-non-transactional-stats-at-statement-bo.patch
Description: Binary data
