Re: New criteria for autovacuum - Mailing list pgsql-hackers

On Fri, Apr 4, 2025 at 3:27 PM Konstantin Knizhnik <knizhnik@garret.ru> wrote:

From logical point of view I agree with you: taken in account number of inserted tuples makes sense if it allows to mark page as all-visible.
So `ins_since_vacuum` should be better renamed to `ins_all_visible_since_vacuum` and count only all-visible tuples. If newly inserted tuple is not visible to all, then it should not be accounted in statistic and trigger autovacuum. But I have completely no idea of how to efficiently maintain such counter: we should keep track of xids of all recently inserted tuples and on each transaction commit determine which one of them become all-visible.

And your suggestion just to not reset `ins_since_vacuum` until all of them becomes all-visible may be easier to implement, but under permanent workload it can lead to situation when `ins_since_vacuum` is never reset and at each vacuum iteration cause vacuuming of the table. Which may cause significant degrade of performance. Even without long-living transactions.

Right, you definitely can't just never reset it to 0. As it is
currently defined, the behavior is correct, that is how many tuples
were inserted since the last vacuum.

As for your proposed patch, I agree with Robert that triggering
vacuums using the stat you proposed just adds to the problem we know
we already have with wasted vacuuming work due to long-running
transactions.

If we already have a problem with wasted vacuuming then IMHO it should be addressed (for example by keeping last_autovacuum_xid_horizon). I do no think that the proposed patch adds something new to the problem. Yes it will trigger autovacuum after each N visibility checks. And if there is still some long running transaction, then this vacuum is useless. But the same is true for criteria based on number of dead tuples.

And from my point of view additional autovacuum runs are less critical than missed autovacuum runs (when table contains garbage or not updarted VM but VM is not launched).

A more targeted solution to your specific problem would be to update
the visibility map on access. Then, the first time you have to fetch
that heap page, you could mark it all-visible (assuming the long
running transaction has ended) and then the next index-only scan
wouldn't have to do the same heap fetch. It doesn't add any overhead
in the case that the long running transaction has not ended, unlike
trying to trigger another autovacuum.

I really considered this alternative when thinking about the solution of the problem. It is more consistent with hint bit approach.
I declined it in favor of this solution because of the following reasons:

1. Index-only scan holds read-only lock on heap page. In order to update it, we need to upgrade this lock to exclusive.
2. We need to check visibility for all elements on the page (actually do something like `heap_page_is_all_visible`) but if there is large number elements at the page it can be quite expensive. And I afraid that it can slowdown speed of index-only scan. Yes, only in "slow case" - when it has to access heap to perform visibility check. But still it may be not acceptable. Also it is not clear how to mark page as already checked. Otherwise we will have to repeat this check for all tids referring this page.
3. `heap_page_is_all_visible` is local to lazyvaccum.c. So  to use it in index-only scan we either have to make it global, either cut&paste it's code. Just removing "static" is not possible, because it is using local `LVRelState`, so some refactoring is needed in any case.
4. We need to wal-log VM page and heap pages in case of setting all-visible bit. It is quite expensive operation. Doing it inside index-only scan can significantly increase time of select. Certainly Postgres is not a real-time DBMS. But still it is better to provide some predictable query execution time. This is why I think that it is better to do such workt in background (in vaccum).