Re: New IndexAM API controlling index vacuum strategies

On Thu, Apr 1, 2021 at 9:58 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Wed, Mar 31, 2021 at 4:45 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > Both 0001 and 0002 patch refactors the whole lazy vacuum code. Can we
> > merge them? I basically agree with the refactoring made by 0001 patch
> > but I'm concerned a bit that having such a large refactoring at very
> > close to feature freeze could be risky. We would need more eyes to
> > review during stabilization.
>
> I think that Robert makes some related points about how we might cut
> scope here. So I'll definitely do some of that, maybe all of it.
>
> > I think it's more clear to use this macro. The macro can be like this:
> >
> > ParallelVacuumIsActive(vacrel) (((LVRelState) vacrel)->lps != NULL)
>
> Yes, that might be better. I'll consider it when I get back to the
> patch tomorrow.
>
> > + * LVDeadTuples stores TIDs that are gathered during pruning/the initial heap
> > + * scan.  These get deleted from indexes during index vacuuming.  They're then
> > + * removed from the heap during a second heap pass that performs heap
> > + * vacuuming.
> >   */
> >
> > The second sentence of the removed lines still seems to be useful
> > information for readers?
>
> I don't think that the stuff about shared memory was useful, really.
> If we say something like this then it should be about the LVRelState
> pointer, not the struct.

Understood.

> > ---
> > +               /* Stop applying cost limits from this point on */
> > +               VacuumCostActive = false;
> > +               VacuumCostBalance = 0;
> > +       }
> >
> > I agree with the idea of disabling vacuum delay in emergency cases.
> > But why do we do that only in the case of the table with indexes? I
> > think this optimization is helpful even in the table with no indexes.
> > We can check the XID wraparound emergency by calling
> > vacuum_xid_limit_emergency() at some point to disable vacuum delay?
>
> Hmm. I see your point, but at the same time I think that the risk is
> lower on a table that has no indexes. It may be true that index
> vacuuming doesn't necessarily take the majority of all of the work in
> lots of cases. But I think that it is true that it does when things
> get very bad -- one-pass/no indexes VACUUM does not care about
> maintenance_work_mem, etc.

Agreed.

>
> But let me think about it...I suppose we could do it when one-pass
> VACUUM considers vacuuming a range of FSM pages every
> VACUUM_FSM_EVERY_PAGES. That's kind of similar to index vacuuming, in
> a way -- it wouldn't be too bad to check for emergencies in the same
> way there.

Yeah, I also thought that would be a good place to check for
emergencies. That sounds reasonable.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/