Re: WIP: parallel GiST index builds

On 7/26/24 14:13, Andrey M. Borodin wrote:
>
>
>> On 26 Jul 2024, at 14:30, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
>>
>> I feel the tricky part about doing that is that we need to make sure the fake LSNs are all less than the current real LSN when the index build completes and while that normally should be the case we will have a almost never exercised code path for when the fake LSN becomes bigger than the real LSN which may contain bugs. Is that really worth it to optimize.
>>
>> But if we are going to use fake LSN: since the index being built is not visible to any scans we do not have to use GetFakeLSNForUnloggedRel() but could use an own counter in shared memory in the GISTShared struct for this specific index which starts at FirstNormalUnloggedLSN. This would give us slightly less contention plus decrease the risk (for good and bad) of the fake LSN being larger than the real LSN.
>
> +1 for atomic counter in GISTShared.

I tried implementing this, see the attached 0002 patch that replaces the
fake LSN with an atomic counter in shared memory. It seems to work (more
testing needed), but I can't say I'm very happy with the code :-(

The way it passes the shared counter to places that actually need it is
pretty ugly. The thing is - the counter needs to be in shared memory,
but places like gistplacetopage() have no idea/need of that. I chose to
simply pass a pg_atomic_uint64 pointer, but that's ... not pretty. Is
there's a better way to do this?

I thought maybe we could simply increment the counter before each call
and pass the LSN value - 64bits should be enough, not sure about the
overhead. But gistplacetopage() also uses the LSN twice, and I'm not
sure it'd be legal to use the same value twice.

Any better ideas?

> BTW we can just reset LSNs to GistBuildLSN just before doing log_newpage_range().
>

Why would the reset be necessary? Doesn't log_newpage_range() set page
LSN to current insert LSN? So why would reset that?

I'm not sure about the discussion about NSN and the need to handle the
case when NSN / fake LSN values get ahead of LSN. Is that really a
problem? If the values generated from the counter are private to the
index build, and log_newpage_range() replaces them with current LSN, do
we still need to worry about it?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company