Hello,
Some initial comments on optimize_lazy_scan_heap_v2.patch.
>- while (next_unskippable_block < nblocks)
>+ while (next_unskippable_block < nblocks &&
>+ !FORCE_CHECK_PAGE(next_unskippable_block))
Dont we need similar check of !FORCE_CHECK_PAGE(next_unskippable_block) in
the below code
which appears on line no 556 of vacuumlazy.c ?
next_unskippable_block = 0;
if ((options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
{
while (next_unskippable_block < nblocks)
> {
> 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++;
> }
I think its better to have just one place where we increment n_all_frozen
before if-else block.
> }
> vacuum_delay_point();
> next_unskippable_block++;
>+ n_skipped++;
> }
> }
Also, instead of incrementing n_skipped everytime, it can be set to
'next_unskippable_block' or 'next_unskippable_block -blkno'
once at the end of the loop.
Thank you,
Rahila Syed
On Thu, Aug 25, 2016 at 11:52 AM, Masahiko Sawada <sawada.m...@gmail.com>
wrote:
> 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
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
