On E, 2005-07-04 at 10:24 +0300, Hannu Krosing wrote:
> On P, 2005-07-03 at 12:19 -0400, Tom Lane wrote:
> > Hannu Krosing <[EMAIL PROTECTED]> writes:
> > > Ok, this is a new version of the vacuum patch with the following changes
> > > following some suggestions in this thread.
> >
> > The more I look at this, the uglier it looks ... and I still haven't
> > seen any convincing demonstration that it *works*, ie doesn't have
> > bad side-effects on the transaction-is-in-progress logic.
Ok, I changed GetOldestXmin() to use proc->inVacuum only when
determining the oldest visible xid for vacuum and index (i.e. which
tuples are safe to delete and which tuples there is no need to index).
The third use on GetOldestXmin() in xlog.c is changed to use old
GetOldestXmin() logic.
My reasoning for why the patch should work is as follows:
1) the only transaction during which inVacuum is set is the 2nd
transaction (of originally 3, now 4) of lazy VACUUM, which does simple
heap scanning and old tuple removal (lazy_vacuum_rel()), and does no
externally visible changes to the data. It only removes tuples which are
already invisible to all running transactions.
2) That transaction never deletes, updates or inserts any tuples on it
own.
3) As it can't add any tuples or change any existing tuples to have its
xid as either xmin or xmax, it already does run logically "outside of
transactions".
4) The only use made of of proc->inVacuum is when determining which
tuples are safe to remove (in vacuum.c) or not worth indexing (in
index.c) and thus can't affect anything else.
I can easily demonstrate that it "works" in the sense that it allows
several concurrent vacuums to clean out old tuples, and I have thus far
been unable to construct the counterexample where it does anything bad.
Could you tell me which part of my reasoning is wrong or what else do I
overlook.
--
Hannu Krosing <[EMAIL PROTECTED]>
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.209
diff -c -r1.209 xact.c
*** src/backend/access/transam/xact.c 29 Jun 2005 22:51:53 -0000 1.209
--- src/backend/access/transam/xact.c 4 Jul 2005 10:57:06 -0000
***************
*** 1402,1407 ****
--- 1402,1416 ----
AfterTriggerBeginXact();
/*
+ * mark the transaction as not VACUUM (vacuum_rel will set isVacuum
+ * to true directly after calling BeginTransactionCommand() )
+ *
+ * this can probably be moved to be done only once when establishing
+ * connection as this is now quaranteedto be reset to false in vacuum.c
+ */
+ MyProc->inVacuum = false;
+
+ /*
* done with start processing, set current transaction state to "in
* progress"
*/
Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.205
diff -c -r1.205 xlog.c
*** src/backend/access/transam/xlog.c 30 Jun 2005 00:00:50 -0000 1.205
--- src/backend/access/transam/xlog.c 4 Jul 2005 10:57:07 -0000
***************
*** 5161,5167 ****
* mustn't do this because StartupSUBTRANS hasn't been called yet.
*/
if (!InRecovery)
! TruncateSUBTRANS(GetOldestXmin(true));
if (!shutdown)
ereport(DEBUG2,
--- 5161,5167 ----
* mustn't do this because StartupSUBTRANS hasn't been called yet.
*/
if (!InRecovery)
! TruncateSUBTRANS(GetOldestXmin(true, false));
if (!shutdown)
ereport(DEBUG2,
Index: src/backend/catalog/index.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.258
diff -c -r1.258 index.c
*** src/backend/catalog/index.c 25 Jun 2005 16:53:49 -0000 1.258
--- src/backend/catalog/index.c 4 Jul 2005 10:57:07 -0000
***************
*** 1420,1426 ****
else
{
snapshot = SnapshotAny;
! OldestXmin = GetOldestXmin(heapRelation->rd_rel->relisshared);
}
scan = heap_beginscan(heapRelation, /* relation */
--- 1420,1426 ----
else
{
snapshot = SnapshotAny;
! OldestXmin = GetOldestXmin(heapRelation->rd_rel->relisshared, true);
}
scan = heap_beginscan(heapRelation, /* relation */
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.310
diff -c -r1.310 vacuum.c
*** src/backend/commands/vacuum.c 14 Jun 2005 22:15:32 -0000 1.310
--- src/backend/commands/vacuum.c 4 Jul 2005 10:57:08 -0000
***************
*** 37,42 ****
--- 37,43 ----
#include "miscadmin.h"
#include "storage/freespace.h"
#include "storage/procarray.h"
+ #include "storage/proc.h"
#include "storage/smgr.h"
#include "tcop/pquery.h"
#include "utils/acl.h"
***************
*** 571,577 ****
{
TransactionId limit;
! *oldestXmin = GetOldestXmin(sharedRel);
Assert(TransactionIdIsNormal(*oldestXmin));
--- 572,578 ----
{
TransactionId limit;
! *oldestXmin = GetOldestXmin(sharedRel, true);
Assert(TransactionIdIsNormal(*oldestXmin));
***************
*** 903,908 ****
--- 904,913 ----
Oid toast_relid;
bool result;
+ BlockNumber stats_rel_pages=0;
+ double stats_rel_tuples=0;
+ bool stats_hasindex=false;
+
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
/* functions in indexes may want a snapshot set */
***************
*** 1016,1039 ****
*/
toast_relid = onerel->rd_rel->reltoastrelid;
! /*
! * Do the actual work --- either FULL or "lazy" vacuum
! */
! if (vacstmt->full)
! full_vacuum_rel(onerel, vacstmt);
! else
! lazy_vacuum_rel(onerel, vacstmt);
! result = true; /* did the vacuum */
! /* all done with this class, but hold lock until commit */
! relation_close(onerel, NoLock);
! /*
! * Complete the transaction and free all temporary memory used.
! */
! StrategyHintVacuum(false);
! CommitTransactionCommand();
/*
* If the relation has a secondary toast rel, vacuum that too while we
--- 1021,1071 ----
*/
toast_relid = onerel->rd_rel->reltoastrelid;
! PG_TRY();
! {
! /*
! * Do the actual work --- either FULL or "lazy" vacuum
! */
! if (vacstmt->full)
! full_vacuum_rel(onerel, vacstmt);
! else
! /* mark this transaction as being a lazy vacuum */
! MyProc->inVacuum = true;
! lazy_vacuum_rel(onerel, vacstmt, &stats_rel_pages, &stats_rel_tuples, &stats_hasindex);
! result = true; /* did the vacuum */
! /* all done with this class, but hold lock until commit */
! relation_close(onerel, NoLock);
!
! /*
! * Complete the transaction and free all temporary memory used.
! */
! StrategyHintVacuum(false);
! CommitTransactionCommand();
! }
! PG_CATCH();
! {
! /* make sure in-vacuum flag is cleared is cleared */
! MyProc->inVacuum = false;
! PG_RE_THROW();
! }
! PG_END_TRY();
! MyProc->inVacuum = false;
!
!
! /* use yet another transaction for saving stats from lazy_vacuum_rel into pg_class */
! if (stats_rel_pages > 0) {
! StartTransactionCommand();
! ActiveSnapshot = CopySnapshot(GetTransactionSnapshot()); /* is this needed ? */
! /* Update statistics in pg_class */
! vac_update_relstats(RelationGetRelid(onerel),
! stats_rel_pages,
! stats_rel_tuples,
! stats_hasindex);
! CommitTransactionCommand();
! }
/*
* If the relation has a secondary toast rel, vacuum that too while we
Index: src/backend/commands/vacuumlazy.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v
retrieving revision 1.54
diff -c -r1.54 vacuumlazy.c
*** src/backend/commands/vacuumlazy.c 19 May 2005 21:35:46 -0000 1.54
--- src/backend/commands/vacuumlazy.c 4 Jul 2005 10:57:08 -0000
***************
*** 128,134 ****
* and locked the relation.
*/
void
! lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
{
LVRelStats *vacrelstats;
Relation *Irel;
--- 128,134 ----
* and locked the relation.
*/
void
! lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, BlockNumber *stats_rel_pages, double *stats_rel_tuples, bool *stats_hasindex)
{
LVRelStats *vacrelstats;
Relation *Irel;
***************
*** 175,184 ****
--- 175,190 ----
lazy_update_fsm(onerel, vacrelstats);
/* Update statistics in pg_class */
+ /*
vac_update_relstats(RelationGetRelid(onerel),
vacrelstats->rel_pages,
vacrelstats->rel_tuples,
hasindex);
+ */
+ /* return values for setting relstats in another transaction */
+ *stats_rel_pages = vacrelstats->rel_pages;
+ *stats_rel_tuples = vacrelstats->rel_tuples;
+ *stats_hasindex = hasindex;
}
Index: src/backend/storage/ipc/procarray.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/ipc/procarray.c,v
retrieving revision 1.3
diff -c -r1.3 procarray.c
*** src/backend/storage/ipc/procarray.c 17 Jun 2005 22:32:45 -0000 1.3
--- src/backend/storage/ipc/procarray.c 4 Jul 2005 10:57:08 -0000
***************
*** 399,405 ****
* when it does set the snapshot it cannot set xmin less than what we compute.
*/
TransactionId
! GetOldestXmin(bool allDbs)
{
ProcArrayStruct *arrayP = procArray;
TransactionId result;
--- 399,405 ----
* when it does set the snapshot it cannot set xmin less than what we compute.
*/
TransactionId
! GetOldestXmin(bool allDbs, bool dataChangingTrxOnly)
{
ProcArrayStruct *arrayP = procArray;
TransactionId result;
***************
*** 423,429 ****
{
PGPROC *proc = arrayP->procs[index];
! if (allDbs || proc->databaseId == MyDatabaseId)
{
/* Fetch xid just once - see GetNewTransactionId */
TransactionId xid = proc->xid;
--- 423,432 ----
{
PGPROC *proc = arrayP->procs[index];
! /* if dataChangingTrxOnly==true then don't use xid of transactions
! * running lazy vacuum */
! if ( (dataChangingTrxOnly && (proc->inVacuum == false))
! && (allDbs || proc->databaseId == MyDatabaseId))
{
/* Fetch xid just once - see GetNewTransactionId */
TransactionId xid = proc->xid;
Index: src/include/commands/vacuum.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/commands/vacuum.h,v
retrieving revision 1.59
diff -c -r1.59 vacuum.h
*** src/include/commands/vacuum.h 31 Dec 2004 22:03:28 -0000 1.59
--- src/include/commands/vacuum.h 4 Jul 2005 10:57:08 -0000
***************
*** 142,148 ****
extern void vacuum_delay_point(void);
/* in commands/vacuumlazy.c */
! extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt);
/* in commands/analyze.c */
extern void analyze_rel(Oid relid, VacuumStmt *vacstmt);
--- 142,151 ----
extern void vacuum_delay_point(void);
/* in commands/vacuumlazy.c */
! extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
! BlockNumber *stats_rel_pages,
! double *stats_rel_tuples,
! bool *stats_hasindex);
/* in commands/analyze.c */
extern void analyze_rel(Oid relid, VacuumStmt *vacstmt);
Index: src/include/storage/proc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/storage/proc.h,v
retrieving revision 1.79
diff -c -r1.79 proc.h
*** src/include/storage/proc.h 17 Jun 2005 22:32:50 -0000 1.79
--- src/include/storage/proc.h 4 Jul 2005 10:57:08 -0000
***************
*** 69,74 ****
--- 69,80 ----
* were starting our xact: vacuum must not
* remove tuples deleted by xid >= xmin ! */
+ bool inVacuum; /* true if current command is vacuum.
+ * used by other vacuum commands
+ * as xid or xmin of this cmd need not be used
+ * when finding global xmin for removing
+ * tuples */
+
int pid; /* This backend's process id, or 0 */
Oid databaseId; /* OID of database this backend is using */
Index: src/include/storage/procarray.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/storage/procarray.h,v
retrieving revision 1.2
diff -c -r1.2 procarray.h
*** src/include/storage/procarray.h 17 Jun 2005 22:32:50 -0000 1.2
--- src/include/storage/procarray.h 4 Jul 2005 10:57:08 -0000
***************
*** 24,30 ****
extern bool TransactionIdIsInProgress(TransactionId xid);
extern bool TransactionIdIsActive(TransactionId xid);
! extern TransactionId GetOldestXmin(bool allDbs);
extern PGPROC *BackendPidGetProc(int pid);
extern bool IsBackendPid(int pid);
--- 24,30 ----
extern bool TransactionIdIsInProgress(TransactionId xid);
extern bool TransactionIdIsActive(TransactionId xid);
! extern TransactionId GetOldestXmin(bool allDbs, bool dataChangingTrxOnly);
extern PGPROC *BackendPidGetProc(int pid);
extern bool IsBackendPid(int pid);
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
