On 11/27/13 01:21, Andres Freund wrote:
On 2013-11-26 13:32:44 +0100, Andres Freund wrote:
This seems to be the case since
b4b6923e03f4d29636a94f6f4cc2f5cf6298b8c8. I suggest we go back to using
scan_all to determine whether we can set new_frozen_xid. That's a slight
pessimization when we scan a relation fully without explicitly scanning
it in its entirety, but given this isn't the first bug around
scanned_pages/rel_pages I'd rather go that way. The aforementioned
commit wasn't primarily concerned with that.
Alternatively we could just compute new_frozen_xid et al before the
lazy_truncate_heap.
I've gone for the latter in this preliminary patch. Not increasing
relfrozenxid after an initial data load seems like a bit of a shame.
I wonder if we should just do scan_all || vacrelstats->scanned_pages <
vacrelstats->rel_pages?
Hmm, you did (scan_all || vacrelstats->scanned_pages <
vacrelstats->rel_pages) for relminmxid, and just
(vacrelstats->scanned_pages < vacrelstats->rel_pages) for relfrozenxid.
That was probably not what you meant to do, the thing you did for
relfrozenxid looks good to me.
Does the attached look correct to you?
- Heikki
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 6778c7d..6688ab3 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -178,7 +178,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
int usecs;
double read_rate,
write_rate;
- bool scan_all;
+ bool scan_all; /* should we scan all pages? */
+ bool scanned_all; /* did we actually scan all pages? */
TransactionId freezeTableLimit;
BlockNumber new_rel_pages;
double new_rel_tuples;
@@ -226,6 +227,21 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
vac_close_indexes(nindexes, Irel, NoLock);
/*
+ * Compute whether we actually scanned the whole relation. If we did, we
+ * can adjust relfrozenxid and relminmxid.
+ *
+ * NB: We need to check this before truncating the relation, because that
+ * will change ->rel_pages.
+ */
+ if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
+ {
+ Assert(!scan_all);
+ scanned_all = false;
+ }
+ else
+ scanned_all = true;
+
+ /*
* Optionally truncate the relation.
*
* Don't even think about it unless we have a shot at releasing a goodly
@@ -254,8 +270,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
* is all-visible we'd definitely like to know that. But clamp the value
* to be not more than what we're setting relpages to.
*
- * 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.
+ * Also, don't change relfrozenxid/relminmxid 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;
@@ -269,13 +285,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
if (new_rel_allvisible > new_rel_pages)
new_rel_allvisible = new_rel_pages;
- new_frozen_xid = FreezeLimit;
- if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
- new_frozen_xid = InvalidTransactionId;
-
- new_min_multi = MultiXactCutoff;
- if (vacrelstats->scanned_pages < vacrelstats->rel_pages)
- new_min_multi = InvalidMultiXactId;
+ new_frozen_xid = scanned_all ? FreezeLimit : InvalidTransactionId;
+ new_min_multi = scanned_all ? MultiXactCutoff : InvalidMultiXactId;
vac_update_relstats(onerel,
new_rel_pages,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
