On Thu, Aug 25, 2016 at 1:41 AM, Anastasia Lubennikova
<lubennikov...@gmail.com> wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: not tested
> Spec compliant: not tested
> Documentation: not tested
>
> Hi,
> I haven't tested the performance yet, but the patch itself looks pretty good
> and reasonable improvement.
> I have a question about removing the comment. It seems to be really tricky
> moment. How do we know that all-frozen block hasn't changed since the
> moment we checked it?
I think that we don't need to check whether all-frozen block hasn't
changed since we checked it.
Even if the all-frozen block has changed after we saw it as an
all-frozen page, we can update the relfrozenxid.
Because any new XIDs just added to that page are newer than the
GlobalXmin we computed.
Am I missing something?
In this patch, since we count the number of all-frozen page even
during not an aggresive scan, I thought that we don't need to check
whether these blocks were all-frozen pages.
> I'm going to test the performance this week.
> I wonder if you could send a test script or describe the steps to test it?
This optimization reduces the number of scanning visibility map when
table is almost visible or frozen..
So it would be effective for very large table (more than several
hundreds GB) which is almost all-visible or all-frozen pages.
For example,
1. Create very large table.
2. Execute VACUUM FREEZE to freeze all pages of table.
3. Measure the execution time of VACUUM FREEZE.
I hope that the second VACUUM FREEZE become fast a little.
I modified the comment, and attached v2 patch.
Regards,
--
Masahiko Sawada
diff --git a/src/backend/commands/vacuumlazy.c
b/src/backend/commands/vacuumlazy.c
index 231e92d..226dc83 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,32 @@ 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)
{
/*
+ * We know that there are n_skipped pages by
the visibilitymap
+ * scan we did just before.
+ *
* 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.
+ * relminmxid. Even if it's not an aggressive
vacuum, we know
+ * the number of all-frozen pages in this
all-visible blocks,
+ * so we don't need to recheck it.
*/
- 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 +790,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 +822,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 +835,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 +886,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats
*vacrelstats,
UnlockReleaseBuffer(buf);
RecordPageWithFreeSpace(onerel, blkno, freespace);
+ blkno++;
continue;
}
@@ -892,6 +926,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats
*vacrelstats,
UnlockReleaseBuffer(buf);
RecordPageWithFreeSpace(onerel, blkno, freespace);
+ blkno++;
continue;
}
@@ -1239,6 +1274,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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
