database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-07 00:36:50 +03:00
Code Activity

Disallow the combination VACUUM FULL FREEZE for safety's sake, for the

Browse Source 
reasons I outlined in pghackers a few days ago.

Also, undo someone's overly optimistic decision to reduce tuple state
checks from if (...) elog() to Asserts.  If I trusted this code more,
I might think it was a good idea to disable these checks in production
installations.  But I don't.
This commit is contained in:
Tom Lane
2004-12-02 19:28:49 +00:00
parent bbf29949c3
commit e9c03c3b1b
3 changed files with 95 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
doc/src/sgml/manage-ag.sgml
View File

@ -1,5 +1,5 @@
<!-- <!--
$PostgreSQL: pgsql/doc/src/sgml/manage-ag.sgml,v 2.36 2004/11/05 19:15:49 tgl Exp $ $PostgreSQL: pgsql/doc/src/sgml/manage-ag.sgml,v 2.37 2004/12/02 19:28:48 tgl Exp $
--> -->
<chapter id="managing-databases"> <chapter id="managing-databases">
@ -240,8 +240,7 @@ createdb -T template0 <replaceable>dbname</>
<para> <para>
After preparing a template database, or making any changes to one, After preparing a template database, or making any changes to one,
it is a good idea to perform it is a good idea to perform <command>VACUUM FREEZE</> in that
<command>VACUUM FREEZE</> or <command>VACUUM FULL FREEZE</> in that
database. If this is done when there are no other open transactions database. If this is done when there are no other open transactions
in the same database, then it is guaranteed that all rows in the in the same database, then it is guaranteed that all rows in the
database are <quote>frozen</> and will not be subject to transaction database are <quote>frozen</> and will not be subject to transaction

6
doc/src/sgml/ref/vacuum.sgml
View File

@ -1,5 +1,5 @@
<!-- <!--
$PostgreSQL: pgsql/doc/src/sgml/ref/vacuum.sgml,v 1.35 2004/03/23 13:21:41 neilc Exp $ $PostgreSQL: pgsql/doc/src/sgml/ref/vacuum.sgml,v 1.36 2004/12/02 19:28:48 tgl Exp $
PostgreSQL documentation PostgreSQL documentation
--> -->
@ -20,8 +20,8 @@ PostgreSQL documentation
<refsynopsisdiv> <refsynopsisdiv>
<synopsis> <synopsis>
VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ <replaceable class="PARAMETER">table</replaceable> ] VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ <replaceable class="PARAMETER">table</replaceable> ]
VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">table</replaceable> [ (<replaceable class="PARAMETER">column</replaceable> [, ...] ) ] ] VACUUM [ FULL | FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">table</replaceable> [ (<replaceable class="PARAMETER">column</replaceable> [, ...] ) ] ]
</synopsis> </synopsis>
</refsynopsisdiv> </refsynopsisdiv>

139
src/backend/commands/vacuum.c
View File

@ -13,7 +13,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.296 2004/12/01 19:00:41 tgl Exp $ * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.297 2004/12/02 19:28:49 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -265,6 +265,27 @@ vacuum(VacuumStmt *vacstmt)
else else
in_outer_xact = IsInTransactionChain((void *) vacstmt); in_outer_xact = IsInTransactionChain((void *) vacstmt);
/*
* Disallow the combination VACUUM FULL FREEZE; although it would mostly
* work, VACUUM FULL's ability to move tuples around means that it is
* injecting its own XID into tuple visibility checks. We'd have to
* guarantee that every moved tuple is properly marked XMIN_COMMITTED or
* XMIN_INVALID before the end of the operation. There are corner cases
* where this does not happen, and getting rid of them all seems hard
* (not to mention fragile to maintain). On the whole it's not worth it
* compared to telling people to use two operations. See pgsql-hackers
* discussion of 27-Nov-2004, and comments below for update_hint_bits().
*
* Note: this is enforced here, and not in the grammar, since (a) we can
* give a better error message, and (b) we might want to allow it again
* someday.
*/
if (vacstmt->vacuum && vacstmt->full && vacstmt->freeze)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("VACUUM FULL FREEZE is not supported"),
errhint("Use VACUUM FULL, then VACUUM FREEZE.")));
/* /*
* Send info about dead objects to the statistics collector * Send info about dead objects to the statistics collector
*/ */
@ -1346,8 +1367,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
do_shrinking = false; do_shrinking = false;
break; break;
default: default:
/* unexpected HeapTupleSatisfiesVacuum result */ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
Assert(false);
break; break;
} }
@ -1530,9 +1550,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
VacPageList vacuum_pages, VacPageList fraged_pages, VacPageList vacuum_pages, VacPageList fraged_pages,
int nindexes, Relation *Irel) int nindexes, Relation *Irel)
{ {
#ifdef USE_ASSERT_CHECKING
TransactionId myXID = GetCurrentTransactionId(); TransactionId myXID = GetCurrentTransactionId();
#endif
Buffer dst_buffer = InvalidBuffer; Buffer dst_buffer = InvalidBuffer;
BlockNumber nblocks, BlockNumber nblocks,
blkno; blkno;
@ -1702,36 +1720,30 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
*/ */
if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED)) if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
{ {
if (tuple.t_data->t_infomask & HEAP_MOVED_IN)
elog(ERROR, "HEAP_MOVED_IN was not expected");
if (!(tuple.t_data->t_infomask & HEAP_MOVED_OFF))
elog(ERROR, "HEAP_MOVED_OFF was expected");
/* /*
* There cannot be another concurrently running VACUUM. If * MOVED_OFF by another VACUUM would have caused the
* the tuple had been moved in by a previous VACUUM, the * visibility check to set XMIN_COMMITTED or XMIN_INVALID.
* 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.
*/ */
Assert(!(tuple.t_data->t_infomask & HEAP_MOVED_IN)); if (HeapTupleHeaderGetXvac(tuple.t_data) != myXID)
elog(ERROR, "invalid XVAC in tuple header");
/* /*
* If this (chain) tuple is moved by me already then I * If this (chain) tuple is moved by me already then I
* have to check is it in vacpage or not - i.e. is it * have to check is it in vacpage or not - i.e. is it
* moved while cleaning this page or some previous one. * moved while cleaning this page or some previous one.
*/ */
Assert(tuple.t_data->t_infomask & HEAP_MOVED_OFF);
/*
* MOVED_OFF by another VACUUM would have caused the
* visibility check to set XMIN_COMMITTED or XMIN_INVALID.
*/
Assert(HeapTupleHeaderGetXvac(tuple.t_data) == myXID);
/* Can't we Assert(keep_tuples > 0) here? */ /* Can't we Assert(keep_tuples > 0) here? */
if (keep_tuples == 0) if (keep_tuples == 0)
continue; continue;
if (chain_tuple_moved) /* some chains was moved while */ if (chain_tuple_moved)
{ /* cleaning this page */ {
/* some chains were moved while cleaning this page */
Assert(vacpage->offsets_free > 0); Assert(vacpage->offsets_free > 0);
for (i = 0; i < vacpage->offsets_free; i++) for (i = 0; i < vacpage->offsets_free; i++)
{ {
@ -2133,15 +2145,19 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
continue; continue;
/* /*
* * See comments in the walk-along-page loop above, why * See comments in the walk-along-page loop above about
* we * have Asserts here instead of if (...) elog(ERROR). * why only MOVED_OFF tuples should be found here.
*/ */
Assert(!(htup->t_infomask & HEAP_MOVED_IN)); if (htup->t_infomask & HEAP_MOVED_IN)
Assert(htup->t_infomask & HEAP_MOVED_OFF); elog(ERROR, "HEAP_MOVED_IN was not expected");
Assert(HeapTupleHeaderGetXvac(htup) == myXID); if (!(htup->t_infomask & HEAP_MOVED_OFF))
elog(ERROR, "HEAP_MOVED_OFF was expected");
if (HeapTupleHeaderGetXvac(htup) != myXID)
elog(ERROR, "invalid XVAC in tuple header");
if (chain_tuple_moved) if (chain_tuple_moved)
{ {
/* some chains was moved while cleaning this page */ /* some chains were moved while cleaning this page */
Assert(vacpage->offsets_free > 0); Assert(vacpage->offsets_free > 0);
for (i = 0; i < vacpage->offsets_free; i++) for (i = 0; i < vacpage->offsets_free; i++)
{ {
@ -2294,7 +2310,13 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
vacrelstats->rel_tuples, keep_tuples); vacrelstats->rel_tuples, keep_tuples);
} }
/* clean moved tuples from last page in Nvacpagelist list */ /*
* Clean moved-off tuples from last page in Nvacpagelist list.
*
* We need only do this in this one page, because higher-numbered
* pages are going to be truncated from the relation entirely.
* But see comments for update_hint_bits().
*/
if (vacpage->blkno == (blkno - 1) && if (vacpage->blkno == (blkno - 1) &&
vacpage->offsets_free > 0) vacpage->offsets_free > 0)
{ {
@ -2324,16 +2346,18 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
continue; continue;
/* /*
* * See comments in the walk-along-page loop above, why * See comments in the walk-along-page loop above about
* we * have Asserts here instead of if (...) elog(ERROR). * why only MOVED_OFF tuples should be found here.
*/ */
Assert(!(htup->t_infomask & HEAP_MOVED_IN)); if (htup->t_infomask & HEAP_MOVED_IN)
Assert(htup->t_infomask & HEAP_MOVED_OFF); elog(ERROR, "HEAP_MOVED_IN was not expected");
Assert(HeapTupleHeaderGetXvac(htup) == myXID); if (!(htup->t_infomask & HEAP_MOVED_OFF))
elog(ERROR, "HEAP_MOVED_OFF was expected");
if (HeapTupleHeaderGetXvac(htup) != myXID)
elog(ERROR, "invalid XVAC in tuple header");
itemid->lp_flags &= ~LP_USED; itemid->lp_flags &= ~LP_USED;
num_tuples++; num_tuples++;
} }
Assert(vacpage->offsets_free == num_tuples); Assert(vacpage->offsets_free == num_tuples);
@ -2644,20 +2668,36 @@ move_plain_tuple(Relation rel,
/* /*
* update_hint_bits() -- update hint bits in destination pages * update_hint_bits() -- update hint bits in destination pages
* *
* Scan all the pages that we moved tuples onto and update tuple * Scan all the pages that we moved tuples onto and update tuple status bits.
* status bits. This is not really necessary, but will save time for * This is normally not really necessary, but it will save time for future
* future transactions examining these tuples. * transactions examining these tuples.
* *
* XXX NOTICE that this code fails to clear HEAP_MOVED_OFF tuples from * This pass guarantees that all HEAP_MOVED_IN tuples are marked as
* pages that were move source pages but not move dest pages. One * XMIN_COMMITTED, so that future tqual tests won't need to check their XVAC.
* also wonders whether it wouldn't be better to skip this step and *
* let the tuple status updates happen someplace that's not holding an * BUT NOTICE that this code fails to clear HEAP_MOVED_OFF tuples from
* exclusive lock on the relation. * pages that were move source pages but not move dest pages. The bulk
* of the move source pages will be physically truncated from the relation,
* and the last page remaining in the rel will be fixed separately in
* repair_frag(), so the only cases where a MOVED_OFF tuple won't get its
* hint bits updated are tuples that are moved as part of a chain and were
* on pages that were not either move destinations nor at the end of the rel.
* To completely ensure that no MOVED_OFF tuples remain unmarked, we'd have
* to remember and revisit those pages too.
*
* Because of this omission, VACUUM FULL FREEZE is not a safe combination;
* it's possible that the VACUUM's own XID remains exposed as something that
* tqual tests would need to check.
*
* For the non-freeze case, one wonders whether it wouldn't be better to skip
* this work entirely, and let the tuple status updates happen someplace
* that's not holding an exclusive lock on the relation.
*/ */
static void static void
update_hint_bits(Relation rel, VacPageList fraged_pages, int num_fraged_pages, update_hint_bits(Relation rel, VacPageList fraged_pages, int num_fraged_pages,
BlockNumber last_move_dest_block, int num_moved) BlockNumber last_move_dest_block, int num_moved)
{ {
TransactionId myXID = GetCurrentTransactionId();
int checked_moved = 0; int checked_moved = 0;
int i; int i;
VacPage *curpage; VacPage *curpage;
@ -2696,12 +2736,13 @@ update_hint_bits(Relation rel, VacPageList fraged_pages, int num_fraged_pages,
continue; continue;
/* /*
* See comments in the walk-along-page loop above, why we have * Here we may see either MOVED_OFF or MOVED_IN tuples.
* Asserts here instead of if (...) elog(ERROR). The
* difference here is that we may see MOVED_IN.
*/ */
Assert(htup->t_infomask & HEAP_MOVED); if (!(htup->t_infomask & HEAP_MOVED))
Assert(HeapTupleHeaderGetXvac(htup) == GetCurrentTransactionId()); elog(ERROR, "HEAP_MOVED_OFF/HEAP_MOVED_IN was expected");
if (HeapTupleHeaderGetXvac(htup) != myXID)
elog(ERROR, "invalid XVAC in tuple header");
if (htup->t_infomask & HEAP_MOVED_IN) if (htup->t_infomask & HEAP_MOVED_IN)
{ {
htup->t_infomask |= HEAP_XMIN_COMMITTED; htup->t_infomask |= HEAP_XMIN_COMMITTED;