pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
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: Mon, 15 Apr 2019 15:46:58 -0400
Message-ID: <23885.1555357618@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+TgmoYJpnbk3XP2CHkzco95a-0XfpqUeCYm74mUMh2NZX5AxA@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>
	<15751.1555256860@sss.pgh.pa.us>
	<CAD21AoCZpFLC-KZywkC-K7OVp0PJKey6znaukTdTv4sEdJWQNQ@mail.gmail.com>
	<CAD21AoAjw_wTZA5vxjBM7BaOTaAHdUdCSui-MtcDwYsY1p25HA@mail.gmail.com>
	<16814.1555348381@sss.pgh.pa.us>
	<CA+TgmoYJpnbk3XP2CHkzco95a-0XfpqUeCYm74mUMh2NZX5AxA@mail.gmail.com>

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Apr 15, 2019 at 1:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I have a very strong feeling that this patch was not fully baked.

> I think you're right, but I don't understand the comment in the
> preceding paragraph.  How does this problem prevent tupgone from
> getting set?

My point is that I suspect that tupgone *shouldn't* get set.
It's not (going to be) gone.

> It looks to me like nleft_dead_tuples should be ripped out.

That was pretty much what I was thinking too.  It makes more sense
just to treat this case identically to dead-but-not-yet-removable.
I have substantial doubts about nleft_dead_itemids being worth
anything, as well.

> We should do something like:
> if (params->index_cleanup == VACOPT_TERNARY_DISABLED)
> {
>     nkeep += tups_vacuumed;
>     tups_vacuumed = 0;
> }

No.  I'm thinking there should be exactly one test of index_cleanup
in this logic, and what it would be is along the lines of

                    if (HeapTupleIsHotUpdated(&tuple) ||
                        HeapTupleIsHeapOnly(&tuple) ||
+                       params->index_cleanup == VACOPT_TERNARY_DISABLED)
                        nkeep += 1;
                    else

In general, this thing has a strong whiff of "large patch
with a small patch struggling to get out".

			regards, tom lane





view thread (118+ messages)  latest in thread

Message-ID: <23885.1555357618@sss.pgh.pa.us>
Permalink:  ../23885.1555357618@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/23885.1555357618@sss.pgh.pa.us

 · 

reply
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: <23885.1555357618@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