Hi Greg,
(2011/08/28 18:54), Greg Smith wrote:
> Updated patch cleans up two diff mistakes made when backing out the
> progress report feature. The tip-off I screwed up should have been the
> absurdly high write rate shown. The usleep was accidentally deleted, so
> it was running without cost limits even applying. Here's a good one
> instead:
>
> LOG: automatic vacuum of table "pgbench.public.pgbench_accounts": index
> scans: 1
> pages: 0 removed, 163935 remain
> tuples: 2000000 removed, 2928356 remain
> buffer usage: 117393 hits, 123351 misses, 102684 dirtied, 2.168 MiB/s
> write rate
> system usage: CPU 2.54s/6.27u sec elapsed 369.99 sec
I took a look at your patch, and it seems fine about fundamental
functionality, though the patch needed to be rebased against current
HEAD. Please see attached patch which I used for review.
IIUC, this patch provides:
* Three counters, which are used to keep # of buffers which were (1)
Hits: found in shared buffers, (2) Missed: not found in shared buffers,
and (3) Dirtied: marked as dirty, in an autovacuum of a relation.
These counters are used only when cost-based autovacuum is enabled by
setting vacuum_cost_delay to non-zero.
* Capability to report # of buffers above, and buffer write rate
(MiB/sec) in the existing autovacuum logging message, only when actual
duration > autovacuum_min_duration, and cost-based autovacuum is enabled.
I think one concern is the way showing statistics. If showing summary
of statistics (at the end of an autovacuum) in server log is enough,
current implementation is fine. Also showing progress report in server
log would be easy to achieve. In contrast, reporting progress via
another backend would require shared memory or statistics collector,
rather than per-process global variables. Anyway, this patch can be the
base of such enhancement.
There are some trivial comments:
* Local variables added by the patch (secs, usecs, write_rate and
endtime) can be moved into narrower scope.
* Initializing starttime to zero seems unnecessary.
In addition, documents about how to use the statistics would be
necessary, maybe in "23.1.5. The Autovacuum Daemon".
I'll set the status of this patch to waiting-on-author. Would you rebase
the patch and post it again?
Regards,
--
Shigeru Hanada
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7fe787e..768c658 100644
*** a/src/backend/commands/vacuum.c
--- b/src/backend/commands/vacuum.c
*************** vacuum(VacuumStmt *vacstmt, Oid relid, b
*** 214,219 ****
--- 214,222 ----
VacuumCostActive = (VacuumCostDelay > 0);
VacuumCostBalance = 0;
+ VacuumPageHit = 0;
+ VacuumPageMiss = 0;
+ VacuumPageDirty = 0;
/*
* Loop to process each selected relation.
diff --git a/src/backend/commands/vacuumlazy.c
b/src/backend/commands/vacuumlazy.c
index cf8337b..d9bb272 100644
*** a/src/backend/commands/vacuumlazy.c
--- b/src/backend/commands/vacuumlazy.c
*************** lazy_vacuum_rel(Relation onerel, VacuumS
*** 153,170 ****
int nindexes;
BlockNumber possibly_freeable;
PGRUsage ru0;
! TimestampTz starttime = 0;
bool scan_all;
TransactionId freezeTableLimit;
BlockNumber new_rel_pages;
double new_rel_tuples;
TransactionId new_frozen_xid;
/* measure elapsed time iff autovacuum logging requires it */
if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
{
pg_rusage_init(&ru0);
! if (Log_autovacuum_min_duration > 0)
starttime = GetCurrentTimestamp();
}
--- 153,173 ----
int nindexes;
BlockNumber possibly_freeable;
PGRUsage ru0;
! TimestampTz starttime = 0, endtime;
bool scan_all;
TransactionId freezeTableLimit;
BlockNumber new_rel_pages;
double new_rel_tuples;
TransactionId new_frozen_xid;
+ long secs;
+ int usecs;
+ double write_rate;
/* measure elapsed time iff autovacuum logging requires it */
if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
{
pg_rusage_init(&ru0);
! if (Log_autovacuum_min_duration > 0 || VacuumCostActive)
starttime = GetCurrentTimestamp();
}
*************** lazy_vacuum_rel(Relation onerel, VacuumS
*** 250,272 ****
/* and log the action if appropriate */
if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
{
if (Log_autovacuum_min_duration == 0 ||
! TimestampDifferenceExceeds(starttime,
GetCurrentTimestamp(),
Log_autovacuum_min_duration))
! ereport(LOG,
! (errmsg("automatic vacuum of table
\"%s.%s.%s\": index scans: %d\n"
! "pages: %d removed, %d
remain\n"
! "tuples: %.0f removed,
%.0f remain\n"
! "system usage: %s",
!
get_database_name(MyDatabaseId),
!
get_namespace_name(RelationGetNamespace(onerel)),
!
RelationGetRelationName(onerel),
!
vacrelstats->num_index_scans,
!
vacrelstats->pages_removed,
! vacrelstats->rel_pages,
!
vacrelstats->tuples_deleted,
! new_rel_tuples,
! pg_rusage_show(&ru0))));
}
}
--- 253,309 ----
/* and log the action if appropriate */
if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
{
+ endtime = GetCurrentTimestamp();
if (Log_autovacuum_min_duration == 0 ||
! TimestampDifferenceExceeds(starttime, endtime,
Log_autovacuum_min_duration))
! {
! if (VacuumCostActive)
! {
! TimestampDifference(starttime, endtime, &secs,
&usecs);
! write_rate = 0;
! if ((secs > 0) || (usecs > 0))
! write_rate = (double) BLCKSZ *
VacuumPageDirty / (1024 * 1024) /
! (secs + usecs / 1000000.0);
!
! ereport(LOG,
! (errmsg("automatic vacuum of
table \"%s.%s.%s\": index scans: %d\n"
! "pages: %d
removed, %d remain\n"
! "tuples: %.0f
removed, %.0f remain\n"
! "buffer usage:
%d hits, %d misses, %d dirtied, %.3f MiB/s write rate\n"
! "system usage:
%s",
!
get_database_name(MyDatabaseId),
!
get_namespace_name(RelationGetNamespace(onerel)),
!
RelationGetRelationName(onerel),
!
vacrelstats->num_index_scans,
!
vacrelstats->pages_removed,
!
vacrelstats->rel_pages,
!
vacrelstats->tuples_deleted,
! new_rel_tuples,
! VacuumPageHit,
! VacuumPageMiss,
! VacuumPageDirty,
! write_rate,
!
pg_rusage_show(&ru0))));
! }
! else
! {
! ereport(LOG,
! (errmsg("automatic vacuum of
table \"%s.%s.%s\": index scans: %d\n"
! "pages: %d
removed, %d remain\n"
! "tuples: %.0f
removed, %.0f remain\n"
! "system usage:
%s",
!
get_database_name(MyDatabaseId),
!
get_namespace_name(RelationGetNamespace(onerel)),
!
RelationGetRelationName(onerel),
!
vacrelstats->num_index_scans,
!
vacrelstats->pages_removed,
!
vacrelstats->rel_pages,
!
vacrelstats->tuples_deleted,
! new_rel_tuples,
!
pg_rusage_show(&ru0))));
! }
! }
}
}
diff --git a/src/backend/storage/buffer/bufmgr.c
b/src/backend/storage/buffer/bufmgr.c
index 8647edd..1b0415c 100644
*** a/src/backend/storage/buffer/bufmgr.c
--- b/src/backend/storage/buffer/bufmgr.c
*************** ReadBuffer_common(SMgrRelation smgr, cha
*** 342,348 ****
--- 342,351 ----
*hit = true;
if (VacuumCostActive)
+ {
+ VacuumPageHit++;
VacuumCostBalance += VacuumCostPageHit;
+ }
TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
smgr->smgr_rnode.node.spcNode,
*************** ReadBuffer_common(SMgrRelation smgr, cha
*** 472,478 ****
--- 475,484 ----
}
if (VacuumCostActive)
+ {
+ VacuumPageMiss++;
VacuumCostBalance += VacuumCostPageMiss;
+ }
TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum,
smgr->smgr_rnode.node.spcNode,
*************** MarkBufferDirty(Buffer buffer)
*** 975,981 ****
--- 981,990 ----
* If the buffer was not dirty already, do vacuum cost accounting.
*/
if (!(bufHdr->flags & BM_DIRTY) && VacuumCostActive)
+ {
+ VacuumPageDirty++;
VacuumCostBalance += VacuumCostPageDirty;
+ }
bufHdr->flags |= (BM_DIRTY | BM_JUST_DIRTIED);
*************** SetBufferCommitInfoNeedsSave(Buffer buff
*** 2300,2306 ****
--- 2309,2318 ----
LockBufHdr(bufHdr);
Assert(bufHdr->refcount > 0);
if (!(bufHdr->flags & BM_DIRTY) && VacuumCostActive)
+ {
+ VacuumPageDirty++;
VacuumCostBalance += VacuumCostPageDirty;
+ }
bufHdr->flags |= (BM_DIRTY | BM_JUST_DIRTIED);
UnlockBufHdr(bufHdr);
}
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index c4c4154..9ce64e6 100644
*** a/src/backend/utils/init/globals.c
--- b/src/backend/utils/init/globals.c
*************** int VacuumCostPageDirty = 20;
*** 115,120 ****
--- 115,124 ----
int VacuumCostLimit = 200;
int VacuumCostDelay = 0;
+ int VacuumPageHit = 0;
+ int VacuumPageMiss = 0;
+ int VacuumPageDirty = 0;
+
int VacuumCostBalance = 0; /* working state for
vacuum */
bool VacuumCostActive = false;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 9d19417..4ee08fe 100644
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
*************** extern int VacuumCostPageDirty;
*** 230,235 ****
--- 230,239 ----
extern int VacuumCostLimit;
extern int VacuumCostDelay;
+ extern int VacuumPageHit;
+ extern int VacuumPageMiss;
+ extern int VacuumPageDirty;
+
extern int VacuumCostBalance;
extern bool VacuumCostActive;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
