I've also added GiST vacuum to the patchset.
> On 24 Oct 2024, at 01:04, Melanie Plageman <melanieplage...@gmail.com> wrote:
>
> On Wed, Oct 23, 2024 at 4:29 PM Andrey M. Borodin <x4...@yandex-team.ru>
> wrote:
>>
>>> On 23 Oct 2024, at 20:57, Andrey M. Borodin <x4...@yandex-team.ru> wrote:
>>>
>>> I'll think how to restructure flow there...
>>
>> OK, I've understood how it should be structured. PFA v5. Sorry for the noise.
>
> I think this would be a bit nicer:
>
> while (BufferIsValid(buf = read_stream_next_buffer(stream, NULL)))
> {
> block = btvacuumpage(&vstate, buf);
> if (info->report_progress)
> pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE, block);
> }OK. > Maybe change btvacuumpage() to return the block number to avoid the > extra BufferGetBlockNumber() calls (those add up). Makes sense... now we have non-obvious function prototype, but I think it worth it. > While looking at this, I started to wonder if it isn't incorrect that > we are not calling pgstat_progress_update_param() for the blocks that > we backtrack and read in btvacuumpage() too (on master as well). > btvacuumpage() may actually have scanned more than one block, so... It's correct, user wants to know progress and backtracks do not count towards progress. Though, it must be relatevely infrequent case. > Unrelated to code review, but btree index vacuum has the same issues > that kept us from committing streaming heap vacuum last release -- > interactions between the buffer access strategy ring buffer size and > the larger reads -- one of which is an increase in the number of WAL > syncs and writes required. Thomas mentions it here [1] and here [2] is > the thread where he proposes adding vectored writeback to address some > of these issues. Do I need to do anything about it? Thank you! Best regards, Andrey Borodin.
v6-0001-Prototype-B-tree-vacuum-streamlineing.patch
Description: Binary data
v6-0002-Use-read_stream-in-GiST-vacuum.patch
Description: Binary data
