Re: New vacuum option to do only freezing

On Mon, Apr 15, 2019 at 9:28 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Apr 15, 2019 at 12:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Robert Haas <robertmhaas@gmail.com> writes:
> > > On Wed, Apr 3, 2019 at 10:32 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >> Attached the updated version patch.
> >
> > > Committed with a little bit of documentation tweaking.
> >
> > topminnow just failed an assertion from this patch:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=topminnow&dt=2019-04-14%2011%3A01%3A48
> >
> > The symptoms are:
> >
> > TRAP: FailedAssertion("!((params->index_cleanup == VACOPT_TERNARY_ENABLED && nleft_dead_tuples == 0 && nleft_dead_itemids == 0) || params->index_cleanup == VACOPT_TERNARY_DISABLED)", File: "/home/nm/farm/mipsel_deb8_gcc_32/HEAD/pgsql.build/../pgsql/src/backend/access/heap/vacuumlazy.c", Line: 1404)
> > ...
> > 2019-04-14 14:49:16.328 CEST [15282:5] LOG:  server process (PID 18985) was terminated by signal 6: Aborted
> > 2019-04-14 14:49:16.328 CEST [15282:6] DETAIL:  Failed process was running: autovacuum: VACUUM ANALYZE pg_catalog.pg_depend
> >
> > Just looking at the logic around index_cleanup, I rather think that
> > that assertion is flat out wrong:
> >
> > +    /* No dead tuples should be left if index cleanup is enabled */
> > +    Assert((params->index_cleanup == VACOPT_TERNARY_ENABLED &&
> > +            nleft_dead_tuples == 0 && nleft_dead_itemids == 0) ||
> > +           params->index_cleanup == VACOPT_TERNARY_DISABLED);
> >
> > Either it's wrong, or this is:
> >
> > +                        /*
> > +                         * Since this dead tuple will not be vacuumed and
> > +                         * ignored when index cleanup is disabled we count
> > +                         * count it for reporting.
> > +                         */
> > +                        if (params->index_cleanup == VACOPT_TERNARY_ENABLED)
> > +                            nleft_dead_tuples++;
> >
>
> Ugh, I think the assertion is right but the above condition is
> completely wrong. We should increment nleft_dead_tuples when index
> cleanup is *not* enabled.

Here is a draft patch to fix this issue.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center