Re: New gist vacuum.

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: New gist vacuum.
Date: 2018-03-02 05:33:40
Message-ID: 009039A3-0EB6-49FD-8E59-B2B15775B73D@yandex-team.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 1 марта 2018 г., в 22:44, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> написал(а):
>
> Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com> writes:
>> I have added small change to patch to allow it be compiled using msvc (uint64_t -> uint64).
>> Everything seems to work, check-world is passing.
>
>> Actually patch fixes two issues:
>> 1) Partial GIST indexes now have corrent tuples count estimation.
>> 2) Now subsequent calls to VACUUM on GIST index (like "VACCUM table_name") do not change tuples count to estimated number of tuples in table (which is changed even without any updates in table due current implementation).
>
>> I think it is fine to commit.
>
> I took a quick look at this. I wonder what is the point of making
> the counting conditional. Since the function is visiting every
> index page anyway, why not just always count and unconditionally
> provide an exact answer? The number of cycles saved by skipping
> "tuplesCount += PageGetMaxOffsetNumber(page)" on each leaf page
> is surely trivial.

Thanks for looking into the patch, Tom!

I thought that it's a good idea to optimize out as many cycles as possible.
But, indeed, there are some reasons in favor of unconditional counting:
1. Code is cleaner, and this is not hot path
2. If we choose unconditional counting in gistvacuumcleanup() I'll remove those counting cycles in gistbulkdelete() in main vacuum patch (for v12). Both functions will have less code.

So, I agree, unconditional counting is a good idea. Here's the v3 patch.

Best regards, Andrey Borodin.

Attachment Content-Type Size
0001-Fix-GiST-stats-for-partial-indexes-v3.patch application/octet-stream 2.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2018-03-02 05:58:43 Re: [WIP PATCH] Index scan offset optimisation using visibility map
Previous Message Masahiko Sawada 2018-03-02 05:20:03 Re: Failed to request an autovacuum work-item in silence