Date: Mon, 30 Dec 2024 09:44:45 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Autovacuum giving up on tables after crash because of lack of
 stats
Message-ID: <Z3JrjavpckvZ/L02@ip-10-97-1-34.eu-west-3.compute.internal>
References: <Z2tblEmfuOfZy4zx@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Z2tblEmfuOfZy4zx@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/Z3JrjavpckvZ/L02%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Wed, Dec 25, 2024 at 10:10:44AM +0900, Michael Paquier wrote:
> 
> 2) The main issue I am trying to tackle is autovacuum giving up on
> tables if there are no stats entries, so we could add *some* WAL
> logging of the relation stats that are relevant for autovacuum, then
> replay them.  I think that the correct approach here is to introduce
> one new RMGR for pgstats, giving to each stats kind the possibility
> to call a routine able to do WAL logging of *some* of its data (custom
> structure size, custom data size), and insert records associated to
> their stats kind.  We are going to need a new optional callback
> defined by a stats kind to be able to force some actions at replay, so
> as stats kinds can decide what to do with the data in the record.
> Generation of WAL records has to happen pgstat_report_stat() through
> the flush callback of each stats kind when the stats stored locally
> are synced with shared memory.  There is a different reason for that:
> stats are flushed when backends shut down, and we are still able to
> generate some WAL at this stage.  An advantage of this design is to be
> able to decide which portions of which stats kind is worth
> replicating, and we can take a step-by-step approach we what data and
> how much data we want to replay (for example for tables we should not
> care about replicating the number scans).

I think that's a good idea. As you said that would give the ability to discard
some stats from the replication and replicate some of them (n_dead_tup,...).

> Another benefit of this
> design is for custom stats kind: these can call the pgstats RMGR to
> pass down some data and define their own callback to use at replay.
> If we do that, flushing the stats file at each checkpoint is not
> actually mandatory: the most critical stats could be in WAL.

> Among all these ideas, 2) is by far the most relevant approach to me,

+1

> because even if we do not flush pgstats at checkpoint, we can still
> keep around relevant stats when performing crash recovery, while
> copying around some stats on standbys.  It should be possible for a
> given stats kind to do a different action depending on if we're in
> standby mode or just in crash recovery.  And that would take care of
> this autovacuum problem post-crash: we could have stats to help in the
> decision of if a table should be vacuum or not.  Note that the
> implementation can be done in multiple steps, like:
> - Adding the whole WAL pgstats facility and some tests related to it
> (WAL logging with injection points for variable and fixed-numbered
> stats in a custom stats kind). 
> - Deal about the autovacuum and relation stats part.
> - Open the door for more replication of stats data, whatever that may
> be.
> 
> Comments, thoughts or tomatoes?

I think that replicating stats that are used by autovacuum would be an additional
benefit, so +1 for the idea number 2). This is an "issue" that has been raised
multiple times (like in [1]). 

[1]: https://www.postgresql.org/message-id/20240607033806.6gwgolihss72cj6r%40awork3.anarazel.de

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com