Re: WIP: parallel GiST index builds

x4mmm@yandex-team.ru

From: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
To: Tomas Vondra <tomas@vondra.me>
Cc: Michael Paquier <michael@paquier.xyz>, Andreas Karlsson <andreas@proxel.se>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-10-31T18:05:43Z
Lists: pgsql-hackers

> On 8 Oct 2024, at 17:05, Tomas Vondra <tomas@vondra.me> wrote:
> 
> Here's an updated patch adding the queryid.

I've took another round of looking through the patch.
Everything I see seems correct to me. It just occurred to me that we will have: buffered build, parallel build, sorting build. All 3 aiming to speed things up. I really hope that we will find a way to parallel sorting build, because it will be fastest for sure.


Currently we have some instances of such code...or similar... or related code.

if (is_build)
{
  if (is_parallel)
    recptr = GetFakeLSNForUnloggedRel();
  else
    recptr = GistBuildLSN;
}
else
{
  if (RelationNeedsWAL(rel))
  {
    recptr = actuallyWriteWAL();
  }
  else
    recptr = gistGetFakeLSN(rel);
}
// Use recptr

In previous review I was proponent of not adding arguments to gistGetFakeLSN(). But now I see that now logic of choosing LSN source is sprawling across various places...
Now I do not have a strong point of view on this. Do you think if something like following would be clearer?
if (!is_build && RelationNeedsWAL(rel))
  {
    recptr = actuallyWriteWAL();
  }
  else
    recptr = gistGetFakeLSN(rel, is_build, is_parallel);

Just as an idea.

I'm mostly looking on GiST-specific parts of the patch, while things around entering parallel mode seems much more complicated. But as far as I can see, large portions of this code are taken from B-tree\BRIN.


Best regards, Andrey Borodin.