Re: Autovacuum fails to keep visibility map up-to-date in mostly-insert-only-tables

On 2014-10-18 21:36:48 -0500, Jim Nasby wrote:
> On 10/9/14, 4:19 PM, Andres Freund wrote:
> >On 2014-10-09 18:16:46 -0300, Alvaro Herrera wrote:
> >>>Andres Freund wrote:
> >>>> >On 2014-10-09 18:03:00 -0300, Alvaro Herrera wrote:
> >>>>> > >Bruce Momjian wrote:
> >>>>> > >
> >>>>>> > > >I agree this is a serious problem.  We have discussed various options,
> >>>>>> > > >but have not decided on anything.  The TODO list has:
> >>>>>> > > >
> >>>>>> > > >    https://wiki.postgresql.org/wiki/Todo
> >>>>>> > > >
> >>>>>> > > >    Improve setting of visibility map bits for read-only and insert-only
> >>>>>> > > >    workloads
> >>>>>> > > >    
> >>>>>> > > >    http://www.postgresql.org/message-id/20130906001437.GA29264@momjian.us
> >>>>> > >
> >>>>> > >I hate to repeat myself, but I think autovacuum could be modified to run
> >>>>> > >actions other than vacuum and analyze.  In this specific case we could
> >>>>> > >be running a table scan that checks only pages that don't have the
> >>>>> > >all-visible bit set, and see if it can be set.
> >>>> >
> >>>> >Isn't that*precisely*  what a plain vacuum run does?
> >>>
> >>>Well, it also scans for dead tuples, removes them, and needs to go
> >>>through indexes to remove their references.

> >IIRC it doesn't do most of that if that there's no need. And if it's a
> >insert only table without rollbacks. I*do*  think there's some
> >optimizations we could make in general.
> 
> No, it always attempts dead tuple removal.

I said some steps, not all steps. Check it out:
   /* If any tuples need to be deleted, perform final vacuum cycle */   /* XXX put a threshold on min number of tuples
here?*/   if (vacrelstats->num_dead_tuples > 0)   {       /* Log cleanup info before we touch indexes */
vacuum_log_cleanup_info(onerel,vacrelstats);
 
       /* Remove index entries */       for (i = 0; i < nindexes; i++)           lazy_vacuum_index(Irel[i],
               &indstats[i],                             vacrelstats);       /* Remove tuples from heap */
lazy_vacuum_heap(onerel,vacrelstats);       vacrelstats->num_index_scans++;   }
 

There's rub here though. We unconditionally do:/* Do post-vacuum cleanup and statistics update for each index */for (i
=0; i < nindexes; i++)    lazy_cleanup_index(Irel[i], indstats[i], vacrelstats);
 

and that's not particularly cheap. Maybe we should make that conditional
when there's been no lazy_vacuum_index/heap calls at all?

> The "weird" part is that if it's not doing a freeze it will just punt
> on a page if it can't get the cleanup lock.

I don't think that's particularly wierd. Otherwise vacuum can get stuck
behind a single very hot page - leading to much, much more bloat.

> I have to believe that could seriously screw up autovacuum scheduling.

Why?

> Now that we have forks, I'm wondering if it would be best to come up
> with a per-page system that could be used to determine when a table
> needs background work to be done. The visibility map could serve a lot
> of this purpose, but I'm not sure if it would work for getting hint
> bits set in the background.

It would. Per definition, all tuples that are 'all visible' need to be
fully hint bitted.

> I think it would also be a win if we had a way to advance relfrozenxid
> and relminmxid. Perhaps something that simply remembered the last XID
> that touched each page...

Not sure what you're getting at here?

I think the big missing piece lest something like Heikki's xid lsn
ranges thing gets finished is a freeze map.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services