Message-ID: <23a0cec3-6880-4608-b066-c1ebd8dff902@gmail.com>
Date: Fri, 12 Jan 2024 13:02:33 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Content-Language: en-US
To: Melanie Plageman <melanieplageman@gmail.com>,
 Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Andres Freund <andres@anarazel.de>,
 Pg Hackers <pgsql-hackers@postgresql.org>,
 Heikki Linnakangas <hlinnaka@iki.fi>, Thomas Munro <thomas.munro@gmail.com>
References: 
 <CAAKRu_Yf3gvXGcCnqqfoq0Q8LX8UM-e-qbm_B1LeZh60f8WhWA@mail.gmail.com>
 <CAAKRu_aK68L=YbMf4rtksxk-kz4dnmMZmjJwohjVf0FTSpgXdw@mail.gmail.com>
 <20240104202309.77h5llrambkl5a3m@awork3.anarazel.de>
 <de814b4a-c60e-4114-8cc0-46468edb607c@gmail.com>
 <CAN55FZ00tfPb9GxtL-qe8OgpjXOFj9HBGCibGO10DGytxs0GTw@mail.gmail.com>
 <CAAKRu_ZwcVf6cC-+v_mb4cFC_Eigb30M459+VTbrsJW3U75sJA@mail.gmail.com>
From: Jim Nasby <jim.nasby@gmail.com>
In-Reply-To: 
 <CAAKRu_ZwcVf6cC-+v_mb4cFC_Eigb30M459+VTbrsJW3U75sJA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/23a0cec3-6880-4608-b066-c1ebd8dff902%40gmail.com>
Precedence: bulk

On 1/11/24 5:50 PM, Melanie Plageman wrote:
> On Fri, Jan 5, 2024 at 5:51 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>>
>> On Fri, 5 Jan 2024 at 02:25, Jim Nasby <jim.nasby@gmail.com> wrote:
>>>
>>> On 1/4/24 2:23 PM, Andres Freund wrote:
>>>
>>> On 2024-01-02 12:36:18 -0500, Melanie Plageman wrote:
>>>
>>> Subject: [PATCH v2 1/6] lazy_scan_skip remove unnecessary local var rel_pages
>>> Subject: [PATCH v2 2/6] lazy_scan_skip remove unneeded local var
>>>   nskippable_blocks
>>>
>>> I think these may lead to worse code - the compiler has to reload
>>> vacrel->rel_pages/next_unskippable_block for every loop iteration, because it
>>> can't guarantee that they're not changed within one of the external functions
>>> called in the loop body.
>>>
>>> Admittedly I'm not up to speed on recent vacuum changes, but I have to wonder if the concept of skipping should go away in the context of vector IO? Instead of thinking about "we can skip this range of blocks", why not maintain a list of "here's the next X number of blocks that we need to vacuum"?
>>
>> Sorry if I misunderstood. AFAIU, with the help of the vectored IO;
>> "the next X number of blocks that need to be vacuumed" will be
>> prefetched by calculating the unskippable blocks ( using the
>> lazy_scan_skip() function ) and the X will be determined by Postgres
>> itself. Do you have something different in your mind?
> 
> I think you are both right. As we gain more control of readahead from
> within Postgres, we will likely want to revisit this heuristic as it
> may not serve us anymore. But the streaming read interface/vectored
> I/O is also not a drop-in replacement for it. To change anything and
> ensure there is no regression, we will probably have to do
> cross-platform benchmarking, though.
> 
> That being said, I would absolutely love to get rid of the skippable
> ranges because I find them very error-prone and confusing. Hopefully
> now that the skipping logic is isolated to a single function, it will
> be easier not to trip over it when working on lazy_scan_heap().

Yeah, arguably it's just a matter of semantics, but IMO it's a lot 
clearer to simply think in terms of "here's the next blocks we know we 
want to vacuum" instead of "we vacuum everything, but sometimes we skip 
some blocks".
-- 
Jim Nasby, Data Architect, Austin TX