On Sat, Jun 4, 2016 at 1:46 PM, Masahiko Sawada <sawada.m...@gmail.com> wrote:
> On Sat, Jun 4, 2016 at 12:59 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> On Fri, Jun 3, 2016 at 11:21 AM, Masahiko Sawada <sawada.m...@gmail.com>
>> wrote:
>>>> Can you submit that part as a separate patch?
>>>
>>> Attached.
>>
>> Thanks, committed.
>>
>>>>> I'm address the review comment of 7087166 commit, and will post the patch.
>>>>
>>>> When?
>>>
>>> On Saturday.
>>
>> Great. Will that address everything for this open item, then?
>>
>
> Attached patch for commit 7087166 on another mail.
> I think that only the test tool for visibility map is remaining and
> under the discussion.
> Even if we have verification tool or function for visibility map, we
> cannot repair the contents of visibility map if we turned out that
> contents of visibility map is something wrong.
> So I think we should have the way that re-generates the visibility map.
> For this purpose, doing vacuum while ignoring visibility map by a new
> option or new function is one idea.
> But IMHO, it's not good idea to allow a function to do vacuum, and
> expanding the VACUUM syntax might be somewhat overkill.
>
> So other idea is to have GUC parameter, vacuum_even_frozen_page for example.
> If this parameter is set true (false by default), we do vacuum whole
> table forcibly and re-generate visibility map.
> The advantage of this idea is that we don't necessary to expand VACUUM
> syntax and relatively easily can remove this parameter if it's not
> necessary anymore.
>
Attached is a sample patch that controls full page vacuum by new GUC parameter.
Regards,
--
Masahiko Sawada
diff --git a/src/backend/commands/vacuumlazy.c
b/src/backend/commands/vacuumlazy.c
index 784c3e9..03f026d 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -125,6 +125,8 @@ typedef struct LVRelStats
bool lock_waiter_detected;
} LVRelStats;
+/* GUC parameter */
+bool vacuum_even_frozen_page; /* should we scan all pages forcibly? */
/* A few variables that don't seem worth passing around as parameters */
static int elevel = -1;
@@ -138,7 +140,7 @@ static BufferAccessStrategy vac_strategy;
/* non-export function prototypes */
static void lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
- Relation *Irel, int nindexes, bool aggressive);
+ Relation *Irel, int nindexes, bool aggressive, bool
scan_all);
static void lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats);
static bool lazy_check_needs_freeze(Buffer buf, bool *hastup);
static void lazy_vacuum_index(Relation indrel,
@@ -246,7 +248,8 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams
*params,
vacrelstats->hasindex = (nindexes > 0);
/* Do the vacuuming */
- lazy_scan_heap(onerel, vacrelstats, Irel, nindexes, aggressive);
+ lazy_scan_heap(onerel, vacrelstats, Irel, nindexes, aggressive,
+ vacuum_even_frozen_page);
/* Done with indexes */
vac_close_indexes(nindexes, Irel, NoLock);
@@ -261,7 +264,7 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams
*params,
if ((vacrelstats->scanned_pages + vacrelstats->frozenskipped_pages)
< vacrelstats->rel_pages)
{
- Assert(!aggressive);
+ Assert(!aggressive && !vacuum_even_frozen_page);
scanned_all_unfrozen = false;
}
else
@@ -442,7 +445,7 @@ vacuum_log_cleanup_info(Relation rel, LVRelStats
*vacrelstats)
*/
static void
lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
- Relation *Irel, int nindexes, bool aggressive)
+ Relation *Irel, int nindexes, bool aggressive, bool
scan_all)
{
BlockNumber nblocks,
blkno;
@@ -513,6 +516,10 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
* such pages do not need freezing and do not affect the value that we
can
* safely set for relfrozenxid or relminmxid.
*
+ * When scan_all is set, we have to scan all pages forcibly while
ignoring
+ * visibility map status, and then we can safely set for relfrozenxid or
+ * relminmxid.
+ *
* Before entering the main loop, establish the invariant that
* next_unskippable_block is the next block number >= blkno that's not
we
* can't skip based on the visibility map, either all-visible for a
@@ -639,11 +646,12 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
/*
* 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.
+ * scan_all is not specified, 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.
*/
- if (skipping_blocks && !FORCE_CHECK_PAGE())
+ if (skipping_blocks && !scan_all && !FORCE_CHECK_PAGE())
{
/*
* Tricky, tricky. If this is in aggressive
vacuum, the page
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e246a9c..f86fa68 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1662,6 +1662,16 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
+ {
+ {"vacuum_even_frozen_page", PGC_USERSET, DEVELOPER_OPTIONS,
+ gettext_noop("Do vacuum even frozen page while ignoring
visibility map."),
+ NULL
+ },
+ &vacuum_even_frozen_page,
+ false,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 80cd4a8..31916dd 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -154,6 +154,7 @@ extern int vacuum_freeze_min_age;
extern int vacuum_freeze_table_age;
extern int vacuum_multixact_freeze_min_age;
extern int vacuum_multixact_freeze_table_age;
+extern bool vacuum_even_frozen_page;
/* in commands/vacuum.c */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
