Re: Reviewing freeze map code

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(at)leadboat(dot)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.