Re: New vacuum option to do only freezing

On Mon, Apr 15, 2019 at 12:47 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Wed, Apr 3, 2019 at 10:32 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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. For nleft_dead_itemids we require that index
cleanup is disabled as follows.

{
/*
* Here, we have indexes but index cleanup is disabled.
Instead of
* vacuuming the dead tuples on the heap, we just forget them.
*
* Note that vacrelstats->dead_tuples could have tuples which
* became dead after HOT-pruning but are not marked dead yet.
* We do not process them because it's a very rare condition, and
* the next vacuum will process them anyway.
*/
Assert(params->index_cleanup == VACOPT_TERNARY_DISABLED);
nleft_dead_itemids += vacrelstats->num_dead_tuples;
}

Regards,

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