Hi all,
While reviewing freeze map code, Andres pointed out[1] that
lazy_scan_heap could accesses visibility map twice and its logic is
seems a bit tricky.
As discussed before, it's not nice especially when large relation is
entirely frozen.
I posted the patch for that before but since this is an optimization,
not bug fix, I'd like to propose it again.
Please give me feedback.
[1]
https://www.postgresql.org/message-id/20160505000840.epatoq6d2e556446%40alap3.anarazel.de
Regards,
--
Masahiko Sawada
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 231e92d..e7cdd2c 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -471,6 +471,8 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
PROGRESS_VACUUM_MAX_DEAD_TUPLES
};
int64 initprog_val[3];
+ BlockNumber n_skipped;
+ BlockNumber n_all_frozen;
pg_rusage_init(&ru0);
@@ -501,6 +503,9 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
initprog_val[2] = vacrelstats->max_dead_tuples;
pgstat_progress_update_multi_param(3, initprog_index, initprog_val);
+ n_skipped = 0;
+ n_all_frozen = 0;
+
/*
* Except when aggressive is set, we want to skip pages that are
* all-visible according to the visibility map, but only when we can skip
@@ -558,14 +563,20 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
{
if ((vmstatus & VISIBILITYMAP_ALL_FROZEN) == 0)
break;
+ n_all_frozen++;
}
else
{
if ((vmstatus & VISIBILITYMAP_ALL_VISIBLE) == 0)
break;
+
+ /* Count the number of all-frozen pages */
+ if (vmstatus & VISIBILITYMAP_ALL_FROZEN)
+ n_all_frozen++;
}
vacuum_delay_point();
next_unskippable_block++;
+ n_skipped++;
}
}
@@ -574,7 +585,8 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
else
skipping_blocks = false;
- for (blkno = 0; blkno < nblocks; blkno++)
+ blkno = 0;
+ while (blkno < nblocks)
{
Buffer buf;
Page page;
@@ -592,18 +604,22 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
TransactionId visibility_cutoff_xid = InvalidTransactionId;
/* see note above about forcing scanning of last page */
-#define FORCE_CHECK_PAGE() \
- (blkno == nblocks - 1 && should_attempt_truncation(vacrelstats))
+#define FORCE_CHECK_PAGE(blk) \
+ ((blk) == nblocks - 1 && should_attempt_truncation(vacrelstats))
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
if (blkno == next_unskippable_block)
{
+ n_skipped = 0;
+ n_all_frozen = 0;
+
/* Time to advance next_unskippable_block */
next_unskippable_block++;
if ((options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
{
- while (next_unskippable_block < nblocks)
+ while (next_unskippable_block < nblocks &&
+ !FORCE_CHECK_PAGE(next_unskippable_block))
{
uint8 vmskipflags;
@@ -614,14 +630,22 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
{
if ((vmskipflags & VISIBILITYMAP_ALL_FROZEN) == 0)
break;
+
+ /* Count the number of all-frozen pages */
+ n_all_frozen++;
}
else
{
if ((vmskipflags & VISIBILITYMAP_ALL_VISIBLE) == 0)
break;
+
+ /* Count the number of all-frozen pages */
+ if (vmskipflags & VISIBILITYMAP_ALL_FROZEN)
+ n_all_frozen++;
}
vacuum_delay_point();
next_unskippable_block++;
+ n_skipped++;
}
}
@@ -646,26 +670,23 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
else
{
/*
- * The current block is potentially skippable; if we've seen a
- * long enough run of skippable blocks to justify skipping it, and
- * we're not forced to check it, then go ahead and skip.
- * Otherwise, the page must be at least all-visible if not
- * all-frozen, so we can set all_visible_according_to_vm = true.
+ * The current block is the first block of continuous skippable blocks,
+ * and we know that how many blocks we can skip to scan. If we've
+ * seen a long enough run of skippable blocks to justify skipping it,
+ * then go ahead and skip. Otherwise, the page must be at least all-visible
+ * if not all-frozen, so we can set all_visible_according_to_vm = true.
*/
- if (skipping_blocks && !FORCE_CHECK_PAGE())
+ if (skipping_blocks)
{
/*
- * Tricky, tricky. If this is in aggressive vacuum, the page
- * must have been all-frozen at the time we checked whether it
- * was skippable, but it might not be any more. We must be
- * careful to count it as a skipped all-frozen page in that
- * case, or else we'll think we can't update relfrozenxid and
- * relminmxid. If it's not an aggressive vacuum, we don't
- * know whether it was all-frozen, so we have to recheck; but
- * in this case an approximate answer is OK.
+ * We know that there are n_skipped pages by the visibilitymap scan we
+ * did just before.
*/
- if (aggressive || VM_ALL_FROZEN(onerel, blkno, &vmbuffer))
- vacrelstats->frozenskipped_pages++;
+ vacrelstats->frozenskipped_pages += n_all_frozen;
+
+ /* Jump to the next unskippable block directly */
+ blkno += n_skipped;
+
continue;
}
all_visible_according_to_vm = true;
@@ -760,10 +781,11 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
* it's OK to skip vacuuming pages we get a lock conflict on. They
* will be dealt with in some future vacuum.
*/
- if (!aggressive && !FORCE_CHECK_PAGE())
+ if (!aggressive && !FORCE_CHECK_PAGE(blkno))
{
ReleaseBuffer(buf);
vacrelstats->pinskipped_pages++;
+ blkno++;
continue;
}
@@ -791,6 +813,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
vacrelstats->pinskipped_pages++;
if (hastup)
vacrelstats->nonempty_pages = blkno + 1;
+ blkno++;
continue;
}
if (!aggressive)
@@ -803,6 +826,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
vacrelstats->pinskipped_pages++;
if (hastup)
vacrelstats->nonempty_pages = blkno + 1;
+ blkno++;
continue;
}
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
@@ -853,6 +877,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
UnlockReleaseBuffer(buf);
RecordPageWithFreeSpace(onerel, blkno, freespace);
+ blkno++;
continue;
}
@@ -892,6 +917,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
UnlockReleaseBuffer(buf);
RecordPageWithFreeSpace(onerel, blkno, freespace);
+ blkno++;
continue;
}
@@ -1239,6 +1265,8 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
*/
if (vacrelstats->num_dead_tuples == prev_dead_count)
RecordPageWithFreeSpace(onerel, blkno, freespace);
+
+ blkno++;
}
/* report that everything is scanned and vacuumed */
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
