Hi Alena,
Thank you for reworking vacuum_interrupt_count! I tested v44, and the
counter now increases exactly once per failed VACUUM, including failures
originating in a parallel worker. This part looks good to me.
I found two other issues:
1. Parallel VACUUM appears to double-count worker resource usage.
Worker index usage is reported separately, but is also included in the
leader’s table-level usage. For example, with one parallel worker I
observed:
table.total_blks_read = 729
sum(index.total_blks_read) = 729
database.db_blks_read = 1459
The corresponding serial VACUUM didn't show this duplication.
2. The reset functions retain the default EXECUTE privilege for PUBLIC.
After granting a non-superuser USAGE on the extension schema, it could
call all three reset functions and remove the accumulated statistics:
vacuum_statistics_reset()
extvac_reset_entry(oid, oid)
extvac_reset_db_entry(oid)
For comparison, pg_stat_reset* functions are restricted to POSTGRES, and
pg_stat_statements explicitly revokes PUBLIC access to its reset
function. Could the extension similarly restrict these functions?
Regards,
Vlada
On 24.09.2026 18:10, Alena Rybakina wrote:
Hi Karina,
Thank you for the review! v44 is attached.
On Wed, Sep 23, 2026 at 8:57 AM Karina Litskevich
<[email protected]> wrote:
> CI is complaining about more than one definition of
> <table id="extvacuumstatistics-pg-stats-vacuum-indexes-columns">
Fixed, the extra table is removed from 0007.
> If you add a new external function in
> visibilitymap.c, i.e., visibilitymap_clear_rel, you should also
> add it in the INTERFACE ROUTINES list in the beginning of the file.
Done.
> I'd also suggest that the variation of the visibilitymap_clear
> function taking a Relation should be the main one (and be called
> visibilitymap_clear), and the variant taking a RelFileLocator
> should be the additional one for those who don't have a Relation
> (and be called visibilitymap_clear_no_stats or something). I am
> not insisting, though.
I left it as is for now. Master has just changed
visibilitymap_clear() to take a RelFileLocator, and I didn't want to
change it back. I can rename it if others prefer.
I also changed one more thing in 0004, after a question from Vlada
Pogozheskaya <[email protected]>. vacuum_interrupt_count
is now added to the database stats in pgstat_update_dbstats(), the
same way as xact_rollback, instead of in AtEOXact_PgStat_Database().
So nothing is done during transaction abort anymore.
--
Regards,
Alena Rybakina
Yandex
