Date: Sun, 26 Jun 2016 17:39:08 -0400
From: Noah Misch <noah@leadboat.com>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Andres Freund <andres@anarazel.de>,
	Amit Kapila <amit.kapila16@gmail.com>,
	Stephen Frost <sfrost@snowman.net>,
	Michael Paquier <michael.paquier@gmail.com>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Reviewing freeze map code
Message-ID: <20160626213908.GA1317196@tornado.leadboat.com>
References: 
 <CA+TgmoZY-taFv5MCJoON-NupG9q8dL34stydWieKYbBGb4tcNQ@mail.gmail.com>
 <CAEepm=3fWAbWryVW9swHyLTY4sXVf0xbLvXqOwUoDiNCx9mBjQ@mail.gmail.com>
 <CA+TgmoYOHNANcr503GDoxwyynLHBwgWeB-6sGF2ikBb=sMQLjw@mail.gmail.com>
 <CA+Tgmoan8=YLen3SvJ8Qt51fYALmrkULfvhvPcFvNzrHEMSXAA@mail.gmail.com>
 <CA+Tgmob+jC=3LUSSQ0ojA02naKC6yb8ny-SHZp8rEFi8Y-X=BA@mail.gmail.com>
 <CAEepm=3kJ-fo9UTf6_4HMsKAHgooPXXFDxDF=jQVehiLx-0Bdw@mail.gmail.com>
 <CAEepm=2at9oWWqt-eMxz3rw3stSa+m6LRzeM+J+iXKBkUJx9FQ@mail.gmail.com>
 <CA+TgmoZ-1TXCsxWcp3i-KMo5DNB-6p-odqw7c-N_q3pzZY1TJg@mail.gmail.com>
 <20160617033628.GA1062555@tornado.leadboat.com>
 <CAEepm=1hgCCz1PZyhuPj0=ybsq601QotQbaOA=skqsaMMO5ARQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAEepm=1hgCCz1PZyhuPj0=ybsq601QotQbaOA=skqsaMMO5ARQ@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On Tue, Jun 21, 2016 at 10:59:25AM +1200, Thomas Munro wrote:
> On Fri, Jun 17, 2016 at 3:36 PM, Noah Misch <noah@leadboat.com> wrote:
> > I agree the non-atomic, unlogged change is a problem.  A related threat
> > doesn't require a torn page:
> >
> >   AssignTransactionId() xid=123
> >   heapam.c:3931 HeapTupleHeaderSetXmax(oldtup.t_data, 123);
> >   some ERROR before heap_update() finishes
> >   rollback;  -- xid=123
> >   some backend flushes the modified page
> >   immediate shutdown
> >   AssignTransactionId() xid=123
> >   commit;  -- xid=123
> >
> > If nothing wrote an xlog record that witnesses xid 123, the cluster can
> > reassign it after recovery.  The failed update is now considered a successful
> > update, and the row improperly becomes dead.  That's important.
> 
> I wonder if that was originally supposed to be handled with the
> HEAP_XMAX_UNLOGGED flag which was removed in 11919160.  A comment in
> the heap WAL logging commit f2bfe8a2 said that tqual routines would
> see the HEAP_XMAX_UNLOGGED flag in the event of a crash before logging
> (though I'm not sure if the tqual routines ever actually did that).

HEAP_XMAX_UNLOGGED does appear to have originated in contemplation of this
same hazard.  Looking at the three commits in "git log -S HEAP_XMAX_UNLOGGED"
(f2bfe8a b58c041 1191916), nothing ever completed the implementation by
testing for that flag.


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