Re: Refactoring

[Sorry, Neil, for abusing your thread. Moving this discussion back to
where it belongs.]

On Tue, 18 Jan 2005 13:17:17 -0300, Alvaro Herrera
<alvherre(at)dcc(dot)uchile(dot)cl> wrote:
>Hmm. I think this is a good idea on principle, but what happens in case
>a previous vacuum was interrupted? Is there a possibility that tuples
>belonging to that vacuum are still marked MOVED_OFF but are not in
>vacpage->offsets, for example?

These bits are handled by an earlier phase of VACUUM, by
HeapTupleSatisfiesVacuum() in scan_heap(). My first vacuum.c
refactoring patch, rev 1.281 2004-06-08, added these comments in
repair_frag():

/*
* VACUUM FULL has an exclusive lock on the relation. So
* normally no other transaction can have pending INSERTs or
* DELETEs in this relation. A tuple is either
* (a) a tuple in a system catalog, inserted or deleted by
* a not yet committed transaction or
* (b) dead (XMIN_INVALID or XMAX_COMMITTED) or
* (c) inserted by a committed xact (XMIN_COMMITTED) or
* (d) moved by the currently running VACUUM.
* In case (a) we wouldn't be in repair_frag() at all.
* In case (b) we cannot be here, because scan_heap() has
* already marked the item as unused, see continue above.
* Case (c) is what normally is to be expected.
* Case (d) is only possible, if a whole tuple chain has been
* moved while processing this or a higher numbered block.
*/

/*
* There cannot be another concurrently running VACUUM. If
* the tuple had been moved in by a previous VACUUM, the
* visibility check would have set XMIN_COMMITTED. If the
* tuple had been moved in by the currently running VACUUM,
* the loop would have been terminated. We had
* elog(ERROR, ...) here, but as we are testing for a
* can't-happen condition, Assert() seems more appropriate.
*/

/*
* MOVED_OFF by another VACUUM would have caused the
* visibility check to set XMIN_COMMITTED or XMIN_INVALID.
*/

This should answer your question, unless the comments are wrong. (BTW
parts of that patch have been backed out by someone, so you won't find
the second comment in current sources.)

As for the problem whether the two code paths deal with the same set of
tuples, read
http://archives.postgresql.org/pgsql-hackers/2004-06/msg00410.php:

| [...] These assignments are
| preceded either by code that sets the appropriate infomask bits or by
| assertions that the bits are already set appropriately.

Servus
Manfred