Hi, Alena! On Thu, Jul 23, 2026 at 12:20 PM Alena Rybakina <[email protected]> wrote: > Thank you for the detailed review, and sorry for the delay. To explain > how the patch set ended up in core in the first place: vacuum is > performed by a dedicated backend and is a permanent background process, > so it seemed natural that all its metrics should be permanent as well > and, like everything else backends collect, flow into the cumulative > statistics system. Nevertheless, the extension gives much more > flexibility, and adding filters later on will make it possible to reduce > the amount of memory reserved for storing the statistics compared to > what the in-core version claimed. I have actually built it this way > before - see the extension-based version I posted in [0] - so this is a > return to that shape rather than a brand-new design. Until now both > approaches looked self-sufficient and valid to me, each in its own way, > but I now think the hybrid one - where a small part of the statistics > lives in core and the rest does not - is the most complete. So you made > me rethink the shape of the patch set, and I've reworked it around the > approach you (and Amit upthread) suggested. Attached is v42, > restructured as follows; per-patch replies are below.
I've some notes regarding the v42 version of the patch. There are no hooks for pgstat_drop_relation()/pg_stat_drop_database(). And the shared-memory stats subsystem has no periodic reaper for "object gone => drop entry". That could lead to garbage accumulation. Where the garbage physically accumulates: 1. Shared memory: the pgstat shared hash. One PgStatShared_ExtVacEntry per relation plus one per database. Grows without bound under DROP/CREATE churn. 2. The stats file pg_stat/pgstat.stat. Both kinds (relation and database) have .write_to_file = true, so entries are serialised on clean shutdown (pgstat_write_statsfile()) and read back at startup (pgstat_read_statsfile()) with no existence check. Therefore, the garbage survives restarts. Also, previously existed OID could be re-used for another relation/database. That would cause abandoned statistics to be handled with new relation/database. Built-in stats avoid this by dropping corresponding entries explicitly. Additionally, pgstat_report_vacuum_error() looks a big dangerous. It takes LWLock to update the shared memory inside the error handler. Could we better save the fast of the error and process it later after exit from the error handler? ------ Regards, Alexander Korotkov Supabase
