Re: New vacuum option to do only freezing

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: New vacuum option to do only freezing
Date: 2019-04-14 15:47:40
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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:

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/", 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++;

The poor quality of that comment suggests that maybe the code is just
as confused.

(I also think that that "ternary option" stuff is unreadably overdesigned
notation, which possibly contributed to getting this wrong. If even the
author can't keep it straight, it's unreadable.)

regards, tom lane

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2019-04-14 15:51:47 Identity columns should own only one sequence
Previous Message Tom Lane 2019-04-14 14:38:05 Re: pg_dump is broken for partition tablespaces