git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Fix a missed case in code for "moving average" estimate of reltuples.

2011年8月30日 18:49:45 +0000 (14:49 -0400)

2011年8月30日 18:51:38 +0000 (14:51 -0400)

It is possible for VACUUM to scan no pages at all, if the visibility map
shows that all pages are all-visible. In this situation VACUUM has no new
information to report about the relation's tuple density, so it wasn't
changing pg_class.reltuples ... but it updated pg_class.relpages anyway.
That's wrong in general, since there is no evidence to justify changing the
density ratio reltuples/relpages, but it's particularly bad if the previous
state was relpages=reltuples=0, which means "unknown tuple density".
We just replaced "unknown" with "zero". ANALYZE would eventually recover
from this, but it could take a lot of repetitions of ANALYZE to do so if
the relation size is much larger than the maximum number of pages ANALYZE
will scan, because of the moving-average behavior introduced by commit
b4b6923e03f4d29636a94f6f4cc2f5cf6298b8c8.

The only known situation where we could have relpages=reltuples=0 and yet
the visibility map asserts everything's visible is immediately following
a pg_upgrade. It might be advisable for pg_upgrade to try to preserve the
relpages/reltuples statistics; but in any case this code is wrong on its
own terms, so fix it. Per report from Sergey Koposov.

Back-patch to 8.4, where the visibility map was introduced, same as the
previous change.

src/backend/commands/vacuum.c patch | blob | blame | history

src/backend/commands/vacuumlazy.c patch | blob | blame | history

src/backend/utils/cache/relcache.c patch | blob | blame | history

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c

index 889737e26e6610b3e0904bb36b7240817895ccca..7fe787ecb74f72caac9f6e4fa515ba9a0000d8a6 100644 (file)

--- a/src/backend/commands/vacuum.c

+++ b/src/backend/commands/vacuum.c

@@ -493,7 +493,9 @@ vac_estimate_reltuples(Relation relation, bool is_analyze,

* If scanned_pages is zero but total_pages isn't, keep the existing value

- * of reltuples.

+ * of reltuples. (Note: callers should avoid updating the pg_class

+ * statistics in this situation, since no new information has been

+ * provided.)

if (scanned_pages == 0)

return old_rel_tuples;

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c

index b5547c5e757cd5e2fa3ccdf54d559db3d6ecff8a..a2420a81313a3c8c353e3c02abcc7e0e33196f1e 100644 (file)

--- a/src/backend/commands/vacuumlazy.c

+++ b/src/backend/commands/vacuumlazy.c

@@ -84,6 +84,7 @@ typedef struct LVRelStats

/* hasindex = true means two-pass strategy; false means one-pass */

bool hasindex;

/* Overall statistics about rel */

+ BlockNumber old_rel_pages; /* previous value of pg_class.relpages */

BlockNumber rel_pages; /* total number of pages */

BlockNumber scanned_pages; /* number of pages we examined */

double scanned_tuples; /* counts only tuples on scanned pages */

@@ -154,6 +155,9 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,

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)

@@ -178,6 +182,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,

vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));

+ vacrelstats->old_rel_pages = onerel->rd_rel->relpages;

vacrelstats->old_rel_tuples = onerel->rd_rel->reltuples;

vacrelstats->num_index_scans = 0;

@@ -207,20 +212,39 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,

FreeSpaceMapVacuum(onerel);

- * Update statistics in pg_class. But don't change relfrozenxid if we

- * skipped any pages.

+ * Update statistics in pg_class.

+ *

+ * A corner case here is that if we scanned no pages at all because every

+ * page is all-visible, we should not update relpages/reltuples, because

+ * we have no new information to contribute. In particular this keeps

+ * us from replacing relpages=reltuples=0 (which means "unknown tuple

+ * density") with nonzero relpages and reltuples=0 (which means "zero

+ * tuple density") unless there's some actual evidence for the latter.

+ *

+ * Also, don't change relfrozenxid if we skipped any pages, since then

+ * we don't know for certain that all tuples have a newer xmin.

+ new_rel_pages = vacrelstats->rel_pages;

+ new_rel_tuples = vacrelstats->new_rel_tuples;

+ if (vacrelstats->scanned_pages == 0 && new_rel_pages > 0)

+ {

+ new_rel_pages = vacrelstats->old_rel_pages;

+ new_rel_tuples = vacrelstats->old_rel_tuples;

+ }

+ new_frozen_xid = FreezeLimit;

+ if (vacrelstats->scanned_pages < vacrelstats->rel_pages)

+ new_frozen_xid = InvalidTransactionId;

vac_update_relstats(onerel,

- vacrelstats->rel_pages, vacrelstats->new_rel_tuples,

+ new_rel_pages, new_rel_tuples,

vacrelstats->hasindex,

- (vacrelstats->scanned_pages < vacrelstats->rel_pages) ?

- InvalidTransactionId :

- FreezeLimit);

+ new_frozen_xid);

/* report results to the stats collector, too */

pgstat_report_vacuum(RelationGetRelid(onerel),

onerel->rd_rel->relisshared,

- vacrelstats->new_rel_tuples);

+ new_rel_tuples);

/* and log the action if appropriate */

if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)

@@ -240,7 +264,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,

vacrelstats->pages_removed,

vacrelstats->rel_pages,

vacrelstats->tuples_deleted,

- vacrelstats->new_rel_tuples,

+ new_rel_tuples,

pg_rusage_show(&ru0))));

}

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c

index d0234a26d03056338d6614836a2284ecff0be236..d969510cab31b944eb7c925a04165266a420234c 100644 (file)

--- a/src/backend/utils/cache/relcache.c

+++ b/src/backend/utils/cache/relcache.c

@@ -1415,8 +1415,8 @@ formrdesc(const char *relationName, Oid relationReltype,

/* formrdesc is used only for permanent relations */

relation->rd_rel->relpersistence = RELPERSISTENCE_PERMANENT;

- relation->rd_rel->relpages = 1;

- relation->rd_rel->reltuples = 1;

+ relation->rd_rel->relpages = 0;

+ relation->rd_rel->reltuples = 0;

relation->rd_rel->relkind = RELKIND_RELATION;

relation->rd_rel->relhasoids = hasoids;

relation->rd_rel->relnatts = (int16) natts;

This is the main PostgreSQL git repository.

RSS Atom