Re: Freezing without write I/O - Mailing list pgsql-hackers

On 2013-09-25 12:31:20 +0300, Heikki Linnakangas wrote:
> Hmm, some of those are trivial, but others, rewrite_heap_tuple() are
> currently only passed the HeapTuple, with no reference to the page where the
> tuple came from. Instead of changing code to always pass that along with a
> tuple, I think we should add a boolean to HeapTupleData, to indicate if the
> tuple came from a mature page. If the flag is set, the tuple should be
> considered visible to everyone, without looking at the xmin/xmax. This might
> affect extensions, although usually external C code that have to deal with
> HeapTuples will use functions like heap_form_tuple to do so, and won't need
> to deal with low-level stuff or visibility themselves.
> 
> Attached is a new version, which adds that field to HeapTupleData. Most of
> the issues on you listed above have been fixed, plus a bunch of other bugs I
> found myself. The bug that Jeff ran into with his count.pl script has also
> been fixed.

This seems a bit hacky to me. Change a widely used struct because a few
functions don't get passed enough information? Visibilitychecks are
properly done with a buffer passed along; that some places have cut
corners around that doesn't mean we have to continue to do so. The
pullups around rs_pagemature are imo indicative of this (there's even a
bug because a page can be all visible before it's mature, right? That
could then cause an assert somewhere down the line when we check page
and tuple are coherent).

Ok, making another scan through this:
* Why can we do PageSetLSN(page, RangeSwitchLSN) in heap_* when the action doesn't need WAL? And why is that correct? I
cansee doing that in vacuum itself, but doing it when we write *new* data to the page?
 
* heap_freeze_page(): The PageSetLSN(page, RangeSwitchLSN) if there's nothing to log is not WAL logged. Which means
thatif we crash it won't necessarily be set, so the VM and the heap lsn might get out of sync. That probably doesn't
haveany bad effects, but imo deserves a comment.
 
* heap_freeze_page(): PageUpdateNeedsFreezing() can now be true before and after. That's a bit confusing :/
* GetSafeClogTruncationPoint truncates the xid-lsn ranges, but there's also an, uncommented, TruncateXidLSNRanges. At
leashow they work together should be described better.
 
* There's quite some FIXMEs around
* Let's move the xid-lsn ranges code from GetNewTransactionId() into it's own function.
* PageMatureLSN and RangeSwitchLSN should be documented somewhere. They are implicitly, but they are used widely enough
thatthat doesn't seem sufficient.
 
* pg_controldata should output pageMatureLSN

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services