From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: New vacuum option to do only freezing
Date: Sun, 14 Apr 2019 11:47:40 -0400
Message-ID: <15751.1555256860@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+TgmoZOJ=mPf4kedwg7HCHsG4Oimm67_3S9xEdYf=8nzO=a7w@mail.gmail.com>
References: <20190403.131702.71087088.horiguchi.kyotaro@lab.ntt.co.jp>
<CAD21AoBtM=HGLkMKBgch37mf0-epa3_o=Y1PU0_m9r5YmtS-NQ@mail.gmail.com>
<CA+Tgmoas581jpJ0TPaA38OhjXHgbLy8z1fuuHH7CaNkrboZJeA@mail.gmail.com>
<20190404.091743.76975665.horiguchi.kyotaro@lab.ntt.co.jp>
<CAD21AoDu0wHfBmgJSQdH4Yn1vJingqgdEQ4bUioJf8v=RQ9K=g@mail.gmail.com>
<CA+TgmoZOJ=mPf4kedwg7HCHsG4Oimm67_3S9xEdYf=8nzO=a7w@mail.gmail.com>
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++;
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, robertmhaas@gmail.com, sawada.mshk@gmail.com, horiguchi.kyotaro@lab.ntt.co.jp, bossartn@amazon.com, alvherre@2ndquadrant.com
Subject: Re: New vacuum option to do only freezing
In-Reply-To: <15751.1555256860@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox