Re: "38.10.10. Shared Memory and LWLocks" may require a clarification

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
Date: 2023-10-30 13:03:45
Message-ID: CAJ7c6TMC2TWeez++so3QuLjW0rzjt-Z7KZtyaYSfAWSM7wa8=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael,

On Fri, Oct 27, 2023 at 9:52 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Tue, May 23, 2023 at 01:47:52PM +0300, Aleksander Alekseev wrote:
> > That's me still talking to myself :)
>
> Let's be two then.

Many thanks for your feedback.

> + <para>
> + It is convenient to use <literal>shmem_startup_hook</literal> which allows
> + placing all the code responsible for initializing shared memory in one place.
> + When using <literal>shmem_startup_hook</literal> the extension still needs
> + to acquire <function>AddinShmemInitLock</function> in order to work properly
> + on all the supported platforms including Windows.
>
> Yeah, AddinShmemInitLock is useful because extensions have no base
> point outside that, and they may want to update their local variables.
> Still, this is not completely exact because EXEC_BACKEND on
> non-Windows platform would still need it, so this should be mentioned.
> Another thing is that extensions may do like autoprewarm.c, where
> the shmem area is not initialized in the startup shmem hook. This is
> a bit cheating because this is a scenario where shmem_request_hook is
> not requested, so shmem needs overallocation, but you'd also need a
> LWLock in this case, even for non-WIN32.

Got it. Let's simply remove the "including Windows" part then.

>
> + on all the supported platforms including Windows. This is also the reason
> + why the return value of <function>GetNamedLWLockTranche</function> is
> + conventionally stored in shared memory instead of local process memory.
> + </para>
>
> Not sure to follow this sentence, the result of GetNamedLWLockTranche
> is the lock, so this sentence does not seem really necessary?

To be honest, by now I don't remember what was meant here, so I
removed the sentence.

> While we're on it, why not improving this part of the documentation more
> modern? We don't mention LWLockNewTrancheId() and
> LWLockRegisterTranche() at all, so do you think that it would be worth
> adding a sample of code with that, mentioning autoprewarm.c as
> example?

Agree, these functions deserve to be mentioned in this section.

PFA patch v4.

--
Best regards,
Aleksander Alekseev

Attachment Content-Type Size
v4-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch application/octet-stream 2.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2023-10-30 13:10:17 Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
Previous Message Matthias van de Meent 2023-10-30 12:31:18 Re: DRAFT GIST support for ORDER BY