From: "Wong, Yi Wen" <yiwong@amazon.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Michael Paquier
	<michael.paquier@gmail.com>
CC: Peter Geoghegan <pg@bowt.ie>, "Wood, Dan" <hexpert@amazon.com>,
        "pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>,
        Andres Freund
	<andres@anarazel.de>
Subject: Re: Old row version in hot chain become visible after a freeze
Thread-Topic: [BUGS] Old row version in hot chain become visible after a
 freeze
Thread-Index: AQHTJ3fzEH9T5bMCDUufad1KxDA7S6Kvvd+AgAEpfwCAAAoegIAAp59A
Date: Tue, 12 Sep 2017 18:38:02 +0000
Message-ID: <1505241452245.71279@amazon.com>
References: 
 <CAB7nPqTc0-imDj-5URYW9a5A+yijFv_UUXucDE=3TLyUmZg3AA@mail.gmail.com>,<20170912082203.gq5hutuveqlxpvuc@alvherre.pgsql>
In-Reply-To: <20170912082203.gq5hutuveqlxpvuc@alvherre.pgsql>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org


> > The
> > resulting patch, which I like better than the previously proposed idea
> > of skipping the freeze, takes the approach of handling freeze correctly
> > for the cases where the tuple still exists after pruning.
>
> That's also something I was wondering when looking at the first patch.
> I am unfortunately not as skilled as you are with this area of the
> code (this thread has brought its quantity of study!), so I was not
> able to draw a clear line with what needs to be done. But I am clearly
> +1 with this approach.

I have tried out the new patch and it works great as well! +1 from me.

> > I also tweaked lazy_record_dead_tuple to fail with ERROR if the tuple
> > cannot be recorded, as observed by Yi Wen.  AFAICS that's not reachable
> > because of the way the array is allocated, so an elog(ERROR) is
> > sufficient.

I agree the fail is rare (and probably doesn't happen in real cases, althou=
gh the comment
does imply with a sufficiently low working_memory it might?). However, I'd =
dispute that ERROR
is sufficient --PANIC is probably appropriate here because FREEZE is not WA=
L-logged until
the end of the page; so we'd end up with unfrozen Xids hanging around with =
an appropriately
timed crash. I wouldn't worry much about the PANIC tripping because like yo=
u said,
this seems unreachable normally.

The errmsg should come with a errhint saying "Increase maintenance_work_mem=
".

I've done some code inspection to try to figure out if crashing at any poin=
t around before FREEZE and
the later lazy_vacuum_page phase is safe (i.e. older Xids might be left beh=
ind on restart/crash); it seems to me it
shouldn't be a problem until new_min_xid and new_min_multi isn't updated un=
til later for the truncate.

Yi Wen

--=20
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs