Tom Lane wrote:
> Sorry, I got a bit confused there. The vacuum's intentional pruning
> will use its own OldestXmin variable, which is known current at the
> start of the vacuum (and I think there were even proposals to advance
> it more frequently than that). However, a vacuum could require some
> incidental system catalog fetches, which I think could result in
> prune operations based on RecentGlobalXmin on the catalog pages
> (cf index_getnext).
Hmm, right, and what Heikki said too.
> Anyway I think we are on the same page about the rest of the issues.
> Did you want to work on fixing them, or shall I?
Is this more or less what you had in mind?
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/access/heap/heapam.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.262
diff -c -p -r1.262 heapam.c
*** src/backend/access/heap/heapam.c 11 Aug 2008 11:05:10 -0000 1.262
--- src/backend/access/heap/heapam.c 10 Sep 2008 19:50:36 -0000
*************** heapgetpage(HeapScanDesc scan, BlockNumb
*** 219,224 ****
--- 219,225 ----
/*
* Prune and repair fragmentation for the whole page, if possible.
*/
+ Assert(TransactionIdIsValid(RecentGlobalXmin));
heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin);
/*
*************** heap_hot_search_buffer(ItemPointer tid,
*** 1469,1474 ****
--- 1470,1477 ----
if (all_dead)
*all_dead = true;
+ Assert(TransactionIdIsValid(RecentGlobalXmin));
+
Assert(ItemPointerGetBlockNumber(tid) == BufferGetBlockNumber(buffer));
offnum = ItemPointerGetOffsetNumber(tid);
at_chain_start = true;
Index: src/backend/access/index/indexam.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/index/indexam.c,v
retrieving revision 1.109
diff -c -p -r1.109 indexam.c
*** src/backend/access/index/indexam.c 19 Jun 2008 00:46:03 -0000 1.109
--- src/backend/access/index/indexam.c 10 Sep 2008 19:51:38 -0000
*************** index_getnext(IndexScanDesc scan, ScanDi
*** 419,424 ****
--- 419,426 ----
SCAN_CHECKS;
GET_SCAN_PROCEDURE(amgettuple);
+ Assert(TransactionIdIsValid(RecentGlobalXmin));
+
/*
* We always reset xs_hot_dead; if we are here then either we are just
* starting the scan, or we previously returned a visible tuple, and in
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.376
diff -c -p -r1.376 vacuum.c
*** src/backend/commands/vacuum.c 13 Aug 2008 00:07:50 -0000 1.376
--- src/backend/commands/vacuum.c 10 Sep 2008 20:17:32 -0000
*************** vac_update_datfrozenxid(void)
*** 790,803 ****
bool dirty = false;
/*
! * Initialize the "min" calculation with RecentGlobalXmin. Any
! * not-yet-committed pg_class entries for new tables must have
! * relfrozenxid at least this high, because any other open xact must have
! * RecentXmin >= its PGPROC.xmin >= our RecentGlobalXmin; see
! * AddNewRelationTuple(). So we cannot produce a wrong minimum by
! * starting with this.
*/
! newFrozenXid = RecentGlobalXmin;
/*
* We must seqscan pg_class to find the minimum Xid, because there is no
--- 790,801 ----
bool dirty = false;
/*
! * Initialize the "min" calculation with GetOldestXmin, which is a
! * reasonable approximation to the minimum relfrozenxid for not-yet-
! * committed pg_class entries for new tables; see AddNewRelationTuple().
! * Se we cannot produce a wrong minimum by starting with this.
*/
! newFrozenXid = GetOldestXmin(true, true);
/*
* We must seqscan pg_class to find the minimum Xid, because there is no
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 990,1007 ****
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
! if (vacstmt->full)
! {
! /* functions in indexes may want a snapshot set */
! PushActiveSnapshot(GetTransactionSnapshot());
! }
! else
{
/*
! * During a lazy VACUUM we do not run any user-supplied functions, and
! * so it should be safe to not create a transaction snapshot.
! *
! * We can furthermore set the PROC_IN_VACUUM flag, which lets other
* concurrent VACUUMs know that they can ignore this one while
* determining their OldestXmin. (The reason we don't set it during a
* full VACUUM is exactly that we may have to run user- defined
--- 988,1003 ----
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
! /*
! * Functions in indexes may want a snapshot set. Also, setting
! * a snapshot ensures that RecentGlobalXmin is kept truly recent.
! */
! PushActiveSnapshot(GetTransactionSnapshot());
!
! if (!vacstmt->full)
{
/*
! * In lazy vacuum, we can set the PROC_IN_VACUUM flag, which lets other
* concurrent VACUUMs know that they can ignore this one while
* determining their OldestXmin. (The reason we don't set it during a
* full VACUUM is exactly that we may have to run user- defined
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1050,1057 ****
if (!onerel)
{
! if (vacstmt->full)
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
--- 1046,1052 ----
if (!onerel)
{
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1082,1089 ****
(errmsg("skipping \"%s\" --- only table or database owner can vacuum it",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
! if (vacstmt->full)
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
--- 1077,1083 ----
(errmsg("skipping \"%s\" --- only table or database owner can vacuum it",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1099,1106 ****
(errmsg("skipping \"%s\" --- cannot vacuum indexes, views, or special system tables",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
! if (vacstmt->full)
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
--- 1093,1099 ----
(errmsg("skipping \"%s\" --- cannot vacuum indexes, views, or special system tables",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1115,1122 ****
if (isOtherTempNamespace(RelationGetNamespace(onerel)))
{
relation_close(onerel, lmode);
! if (vacstmt->full)
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
--- 1108,1114 ----
if (isOtherTempNamespace(RelationGetNamespace(onerel)))
{
relation_close(onerel, lmode);
! PopActiveSnapshot();
CommitTransactionCommand();
return;
}
*************** vacuum_rel(Oid relid, VacuumStmt *vacstm
*** 1168,1175 ****
/*
* Complete the transaction and free all temporary memory used.
*/
! if (vacstmt->full)
! PopActiveSnapshot();
CommitTransactionCommand();
/*
--- 1160,1166 ----
/*
* Complete the transaction and free all temporary memory used.
*/
! PopActiveSnapshot();
CommitTransactionCommand();
/*
Index: src/backend/executor/nodeBitmapHeapscan.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/executor/nodeBitmapHeapscan.c,v
retrieving revision 1.29
diff -c -p -r1.29 nodeBitmapHeapscan.c
*** src/backend/executor/nodeBitmapHeapscan.c 19 Jun 2008 00:46:04 -0000 1.29
--- src/backend/executor/nodeBitmapHeapscan.c 10 Sep 2008 20:18:52 -0000
***************
*** 37,42 ****
--- 37,43 ----
#include "access/heapam.h"
#include "access/relscan.h"
+ #include "access/transam.h"
#include "executor/execdebug.h"
#include "executor/nodeBitmapHeapscan.h"
#include "pgstat.h"
*************** bitgetpage(HeapScanDesc scan, TBMIterate
*** 262,267 ****
--- 263,269 ----
/*
* Prune and repair fragmentation for the whole page, if possible.
*/
+ Assert(TransactionIdIsValid(RecentGlobalXmin));
heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin);
/*
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.184
diff -c -p -r1.184 postinit.c
*** src/backend/utils/init/postinit.c 12 May 2008 00:00:52 -0000 1.184
--- src/backend/utils/init/postinit.c 10 Sep 2008 20:19:56 -0000
***************
*** 47,52 ****
--- 47,53 ----
#include "utils/plancache.h"
#include "utils/portal.h"
#include "utils/relcache.h"
+ #include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
*************** InitPostgres(const char *in_dbname, Oid
*** 461,470 ****
on_shmem_exit(ShutdownPostgres, 0);
/*
! * Start a new transaction here before first access to db
*/
if (!bootstrap)
StartTransactionCommand();
/*
* Now that we have a transaction, we can take locks. Take a writer's
--- 462,476 ----
on_shmem_exit(ShutdownPostgres, 0);
/*
! * Start a new transaction here before first access to db, and get a
! * snapshot. We don't have a use for the snapshot itself, but we're
! * interested in the secondary effect that it sets RecentGlobalXmin.
*/
if (!bootstrap)
+ {
StartTransactionCommand();
+ (void) GetTransactionSnapshot();
+ }
/*
* Now that we have a transaction, we can take locks. Take a writer's
Index: src/backend/utils/time/snapmgr.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/time/snapmgr.c,v
retrieving revision 1.4
diff -c -p -r1.4 snapmgr.c
*** src/backend/utils/time/snapmgr.c 11 Jul 2008 02:10:14 -0000 1.4
--- src/backend/utils/time/snapmgr.c 10 Sep 2008 19:49:09 -0000
*************** static Snapshot SecondarySnapshot = NULL
*** 59,68 ****
* These are updated by GetSnapshotData. We initialize them this way
* for the convenience of TransactionIdIsInProgress: even in bootstrap
* mode, we don't want it to say that BootstrapTransactionId is in progress.
*/
TransactionId TransactionXmin = FirstNormalTransactionId;
TransactionId RecentXmin = FirstNormalTransactionId;
! TransactionId RecentGlobalXmin = FirstNormalTransactionId;
/*
* Elements of the list of registered snapshots.
--- 59,72 ----
* These are updated by GetSnapshotData. We initialize them this way
* for the convenience of TransactionIdIsInProgress: even in bootstrap
* mode, we don't want it to say that BootstrapTransactionId is in progress.
+ *
+ * RecentGlobalXmin is initialized to InvalidTransactionId, to ensure that no
+ * one tries to use a stale value. Readers should ensure that it has been set
+ * to something else before using it.
*/
TransactionId TransactionXmin = FirstNormalTransactionId;
TransactionId RecentXmin = FirstNormalTransactionId;
! TransactionId RecentGlobalXmin = InvalidTransactionId;
/*
* Elements of the list of registered snapshots.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
