After reviewing the threads and thinking about the various
historical behaviors, I'm suggesting that we apply the attached,
back-patched to 9.0, to fix the unintended changes from historical
behavior related to lack of statistics generation in some cases
where statistics were generated before
b19e4250b45e91c9cbdd18d35ea6391ab5961c8d, and to generate them in
some cases where they were historically suppressed.
Prior behavior was different between a VACUUM command and
autovacuum when there was sufficient empty space at the end of a
table to trigger an attempt to truncate the table:
* For a VACUUM command, statistics were always generated on a
VACUUM ANALYZE, and truncation might or might not occur, depending
on whether it was able to immediately get an AccessExclusiveLock on
the table when it got to this phase of VACUUM processing. If it
was able to get the lock, it would hold it for as long as the
truncation took, blocking any other access to the table --
potentially for a very long time. If it was not immediately able
to get the lock, it would not attempt any truncation, so truncation
was not deterministic before the recent patch.
* For autovacuum, if it was initially unable to acquire the
AccessExclusiveLock on the table when it got to this phase no
truncation was attempted and statistics were generated. If it
acquired the lock and blocked any other process for the duration
set by deadlock_timeout all work toward truncation was discarded
and no statistics were generated. If it was able to complete the
truncation, statistics were generated. So before the recent patch
neither truncation nor statistics generation were deterministic.
Current behavior for both the VACUUM command and autovacuum is to
avoid any prolonged holding of AccessExclusiveLock on the table
when there is contention for the lock, with limited retries to
acquire or reacquire the lock to make incremental progress on
truncation. Any progress on truncating is not lost if the lock is
relinquished to allow other tasks to proceed. I'm proposing that
we don't change that part of it.
The problem is that current behavior is to skip statistics
generation if the truncation attempt is terminated due to
contention for the table lock; whereas historically that was never
skipped for the VACUUM ANALYZE command, and only sometimes skipped
when autovacuum was intending to analyze the table but was unable
to complete the truncation. The attached will not skip the analyze
step where it had historically run, and will actually allow
autovacuum to run it when the truncation attempt was started but
not able to complete. The old mechanism for terminating the
truncation attempt (a cancel signal from the blocked process) did
not allow this.
The attached is along the lines of what Tom suggested was the
minimal fix, and less drastic than what I was initially proposing
-- which was to also restore historical behavior for the VACUUM
command. After seeing how unpredictable that behavior was
regarding truncation, it doesn't seem wise to complicate the code
to try to go back to that.
I also think that the new LOG level entry about giving up on the
truncate attempt is too chatty, and we've gotten questions from
users who were somewhat alarmed by it, so I toned it down. I'm
still not sure that the logging is quite optimal yet, so any
suggestions are welcome.
The only change outside of local naming, white space, comments,
messages, and moving a couple variables into a more local scope is
this:
- if (!vacrelstats->lock_waiter_detected)
- pgstat_report_vacuum(RelationGetRelid(onerel),
- onerel->rd_rel->relisshared,
- new_rel_tuples);
- else
- vacstmt->options &= ~VACOPT_ANALYZE;
+ pgstat_report_vacuum(RelationGetRelid(onerel),
+ onerel->rd_rel->relisshared,
+ new_rel_tuples);
... which simply reverts this part to match older code.
This is being presented for discussion; I have not finished testing
it.
Comments?
--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index d392698..8a1ffcf 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -78,9 +78,9 @@
* that the potential for improvement was great enough to merit the cost of
* supporting them.
*/
-#define AUTOVACUUM_TRUNCATE_LOCK_CHECK_INTERVAL 20 /* ms */
-#define AUTOVACUUM_TRUNCATE_LOCK_WAIT_INTERVAL 50 /* ms */
-#define AUTOVACUUM_TRUNCATE_LOCK_TIMEOUT 5000 /* ms */
+#define VACUUM_TRUNCATE_LOCK_CHECK_INTERVAL 20 /* ms */
+#define VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL 50 /* ms */
+#define VACUUM_TRUNCATE_LOCK_TIMEOUT 5000 /* ms */
/*
* Guesstimation of number of dead tuples per page. This is used to
@@ -285,17 +285,10 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
new_frozen_xid,
new_min_multi);
- /*
- * Report results to the stats collector, too. An early terminated
- * lazy_truncate_heap attempt suppresses the message and also cancels the
- * execution of ANALYZE, if that was ordered.
- */
- if (!vacrelstats->lock_waiter_detected)
- pgstat_report_vacuum(RelationGetRelid(onerel),
- onerel->rd_rel->relisshared,
- new_rel_tuples);
- else
- vacstmt->options &= ~VACOPT_ANALYZE;
+ /* report results to the stats collector, too */
+ pgstat_report_vacuum(RelationGetRelid(onerel),
+ onerel->rd_rel->relisshared,
+ new_rel_tuples);
/* and log the action if appropriate */
if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
@@ -1347,28 +1340,21 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
*/
CHECK_FOR_INTERRUPTS();
- if (++lock_retry > (AUTOVACUUM_TRUNCATE_LOCK_TIMEOUT /
- AUTOVACUUM_TRUNCATE_LOCK_WAIT_INTERVAL))
+ if (++lock_retry > (VACUUM_TRUNCATE_LOCK_TIMEOUT /
+ VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL))
{
/*
* We failed to establish the lock in the specified number of
- * retries. This means we give up truncating. Suppress the
- * ANALYZE step. Doing an ANALYZE at this point will reset the
- * dead_tuple_count in the stats collector, so we will not get
- * called by the autovacuum launcher again to do the truncate.
+ * retries. This means we give up truncating.
*/
vacrelstats->lock_waiter_detected = true;
- ereport(LOG,
- (errmsg("automatic vacuum of table \"%s.%s.%s\": "
- "could not (re)acquire exclusive "
- "lock for truncate scan",
- get_database_name(MyDatabaseId),
- get_namespace_name(RelationGetNamespace(onerel)),
+ ereport(elevel,
+ (errmsg("\"%s\": stopping truncate due to conflicting lock request",
RelationGetRelationName(onerel))));
return;
}
- pg_usleep(AUTOVACUUM_TRUNCATE_LOCK_WAIT_INTERVAL);
+ pg_usleep(VACUUM_TRUNCATE_LOCK_WAIT_INTERVAL);
}
/*
@@ -1448,8 +1434,6 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
{
BlockNumber blkno;
instr_time starttime;
- instr_time currenttime;
- instr_time elapsed;
/* Initialize the starttime if we check for conflicting lock requests */
INSTR_TIME_SET_CURRENT(starttime);
@@ -1467,24 +1451,26 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
/*
* Check if another process requests a lock on our relation. We are
* holding an AccessExclusiveLock here, so they will be waiting. We
- * only do this in autovacuum_truncate_lock_check millisecond
- * intervals, and we only check if that interval has elapsed once
- * every 32 blocks to keep the number of system calls and actual
- * shared lock table lookups to a minimum.
+ * only do this once per VACUUM_TRUNCATE_LOCK_CHECK_INTERVAL, and we
+ * only check if that interval has elapsed once every 32 blocks to
+ * keep the number of system calls and actual shared lock table
+ * lookups to a minimum.
*/
if ((blkno % 32) == 0)
{
+ instr_time currenttime;
+ instr_time elapsed;
+
INSTR_TIME_SET_CURRENT(currenttime);
elapsed = currenttime;
INSTR_TIME_SUBTRACT(elapsed, starttime);
if ((INSTR_TIME_GET_MICROSEC(elapsed) / 1000)
- >= AUTOVACUUM_TRUNCATE_LOCK_CHECK_INTERVAL)
+ >= VACUUM_TRUNCATE_LOCK_CHECK_INTERVAL)
{
if (LockHasWaitersRelation(onerel, AccessExclusiveLock))
{
ereport(elevel,
- (errmsg("\"%s\": suspending truncate "
- "due to conflicting lock request",
+ (errmsg("\"%s\": suspending truncate due to conflicting lock request",
RelationGetRelationName(onerel))));
vacrelstats->lock_waiter_detected = true;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
