| From: | Alena Rybakina <lena(dot)ribackina(at)yandex(dot)ru> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Vacuum statistics |
| Date: | 2026-09-16 10:57:49 |
| Message-ID: | 6696eb83-e649-4e4c-b1df-c8e72346b63f@yandex.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Alexander, hi Bharath,
Thank you both for the review, and sorry for the late reply. Both
issues are fixed in the attached v43. The rebase and the fixes needed
more work than I expected, so it took a bit longer.
1. Statistics of dropped objects
On Wed, Aug 12, 2026 at 3:10 AM Alexander Korotkov
<aekorotkov(at)gmail(dot)com> wrote:
> 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.
On Fri, Aug 14, 2026 at 1:45 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> Can't the existing object access type (OAT) hooks for relation and
> database drops implemented in an external module take care of this
> problem?
I agree with Bharath, and v43 does this (0005). The extension now uses
object_access_hook, and nothing has to change in core:
* when a table or an index is dropped, its statistics entry is dropped
too;
* when a table or an index is created, an old entry with the same OID
is reset.
This is the same thing core does for the built-in relation statistics.
Dropped databases need nothing extra: when core drops the statistics of
a database, it also removes all other entries of that database,
including the extension's.
Since dropped objects no longer leave entries behind, nothing old gets
into the stats file either, so I kept .write_to_file = true.
One thing to be aware of: if the stats file has entries of the extension
and the server is started even once without the module in
shared_preload_libraries, the server treats the whole file as corrupted
and resets all cumulative statistics, including the built-in ones. This
is how custom stats kinds work today. The documentation now warns about
it and suggests vacuum_statistics.enabled = off instead of removing the
module.
The new TAP test 056_vacuum_stats_gc.pl checks DROP TABLE, a rolled back
DROP, a restart, DROP DATABASE and OID reuse. If the drop part of the
hook is removed, the DROP TABLE check fails; if the reset part is
removed, the OID reuse check fails.
2. The lock in the error callback
On Wed, Aug 12, 2026 at 3:10 AM Alexander Korotkov
<aekorotkov(at)gmail(dot)com> wrote:
> Additionally, pgstat_report_vacuum_error() looks a big dangerous. It
> takes LWLock to update the shared memory inside the error handler.
On Fri, Aug 14, 2026 at 1:45 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> So, +1 to just increment the counter in the callback and report it at
> a later safe point.
Agreed, this is fixed in 0004. The error callback now only increments a
local counter: no locks and no shared memory. At the end of the
transaction, after all locks have been released, the counter is added to
the database statistics and then flushed like any other statistics.
3. Rebase
Some recent commits in master changed the same code, so v43 also has
these changes:
* index statistics now have their own stats kind (72a6dad1c911,
f2ccb8e6c591), so the per-index vacuum times moved there (0002);
* visibilitymap_clear() no longer takes a Relation, so 0009 adds
visibilitymap_clear_rel() for the callers that have one;
* PgStat_TableCounts was split into two parts (3f2f5e7c4cc); the
visibility map counters are in the non-transactional part;
* the test from 0004 is now t/018_vacuum_interrupts.pl, because master
added its own t/015_*.
The structure is the same as before: 0001-0004 are the small core-only
part and are useful on their own; 0005-0008 are the hook and the
extension; 0009 is independent.
--
Regards,
Alena Rybakina
Yandex
| Attachment | Content-Type | Size |
|---|---|---|
| v43-0001-Report-per-index-removed-tuples-in-vacuum-instru.patch | text/plain | 1.4 KB |
| v43-0002-Track-vacuum-times-for-indexes-and-databases-and.patch | text/plain | 36.6 KB |
| v43-0003-Count-wraparound-failsafe-vacuums-in-pg_stat-vie.patch | text/plain | 15.1 KB |
| v43-0004-Count-vacuums-interrupted-by-errors-in-pg_stat_d.patch | text/plain | 14.4 KB |
| v43-0007-ext_vacuum_statistics-WAL-metrics-and-the-per-da.patch | text/plain | 61.1 KB |
| v43-0008-ext_vacuum_statistics-shared-buffer-access-count.patch | text/plain | 24.4 KB |
| v43-0009-Track-table-VM-stability.patch | text/plain | 28.3 KB |
| v43-0005-Vacuum-report-hook-and-the-ext_vacuum_statistics.patch | text/plain | 64.6 KB |
| v43-0006-ext_vacuum_statistics-page-counters-for-tables-a.patch | text/plain | 17.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-09-16 11:00:35 | Re: pgsql: Revert online data checksum transitions |
| Previous Message | Ayush Tiwari | 2026-09-16 10:53:51 | Re: ERROR: no relation entry for relid 3 |