Hi,

On 2023-04-05 16:17:20 -0400, Melanie Plageman wrote:
> On Wed, Apr 5, 2023 at 1:05 PM Andres Freund <and...@anarazel.de> wrote:
> > Perhaps the best solution for autovac vs interactive vacuum issue would be 
> > to
> > move the allocation of the bstrategy to ExecVacuum()?
> 
> So, I started looking into allocating the bstrategy in ExecVacuum().
> 
> While doing so, I was trying to understand if the "sanity checking" in
> vacuum() could possibly apply to autovacuum, and I don't really see how.
> 
> AFAICT, autovacuum does not ever set VACOPT_DISABLE_PAGE_SKIPPING or
> VACOPT_FULL or VACOPT_ONLY_DATABASE_STATS.
> 
> We could move those sanity checks up into ExecVacuum().

Would make sense.

ISTM that eventually most of what currently happens in vacuum() should be in
ExecVacuum(). There's a lot of stuff that can't happen for autovacuum. So it
just seems to make more sense to move those parts to ExecVacuum().


> I also noticed that we make the vac_context in vacuum() which says it is
> for "cross-transaction storage". We use it for the buffer access
> strategy and for the newrels relation list created in vacuum(). Then we
> delete it at the end of vacuum().

> Autovacuum workers already make a similar kind of memory context called
> AutovacMemCxt in do_autovacuum() which the comment says is for the list
> of relations to vacuum/analyze across transactions.

AutovacMemCxt seems to be a bit longer lived / cover more than the context
created in vacuum(). It's where all the hash tables etc live that
do_autovacuum() uses to determine what to vacuum.

Note that do_autovacuum() also creates:

        /*
         * create a memory context to act as fake PortalContext, so that the
         * contexts created in the vacuum code are cleaned up for each table.
         */
        PortalContext = AllocSetContextCreate(AutovacMemCxt,
                                                                                
  "Autovacuum Portal",
                                                                                
  ALLOCSET_DEFAULT_SIZES);

which is then what vacuum() creates the "Vacuum" context in.


> What if we made ExecVacuum() make its own memory context and both it and
> do_autovacuum() pass that memory context (along with the buffer access
> strategy they make) to vacuum(), which then uses the memory context in
> the same way it does now?

Maybe? It's not clear to me why it'd be a win.


> It simplifies the buffer usage limit patchset and also seems a bit more
> clear than what is there now?

I don't really see what it'd make simpler? The context in vacuum() is used for
just that vacuum - we couldn't just use AutovacMemCxt, as that'd live much
longer (for all the tables a autovac worker processes).

Greetings,

Andres Freund


Reply via email to