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>
Subject: Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
Date: 2023-05-21 10:10:49
Message-ID: CAJ7c6TPE45uVCsXKfpPKgpg7cC5qQPjWxsz1i3cMitSWAgi6oA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

> However it's not clear when a race-condition may happen. The rest of
> the text gives an overall impression that the shmem_startup_hook will
> be called by postmaster once (unless an extension places several hooks
> in series). Thus there is no real need to ackquire AddinShmemInitLock
> and it should be safe to store LWLock* in local process memory. This
> memory will be inherited from postmaster by child processes and the
> overall memory usage is going to be the same due to copy-on-write.

I added some logs and comments to my toy extension [1] to demonstrate
this. Additionally I added a sleep() call to the shmem_startup_hook to
make sure there are no concurrent processes at the moment when the
hook is called (this change is not committed to the GitHub
repository):

```
@@ -35,6 +35,9 @@ experiment_shmem_request(void)
RequestNamedLWLockTranche("experiment", 1);
}

+#include <stdio.h>
+#include <unistd.h>
+
static void
experiment_shmem_startup(void)
{
@@ -43,6 +46,8 @@ experiment_shmem_startup(void)
elog(LOG, "experiment_shmem_startup(): pid = %d, postmaster = %d\n",
MyProcPid, !IsUnderPostmaster);

+ sleep(30);
+
if(prev_shmem_startup_hook)
prev_shmem_startup_hook();
```

If we do `make && make install && make installcheck` and examine
.//tmp_check/log/001_basic_main.log we will see:

```
[6288] LOG: _PG_init(): pid = 6288, postmaster = 1
[6288] LOG: experiment_shmem_request(): pid = 6288, postmaster = 1
[6288] LOG: experiment_shmem_startup(): pid = 6288, postmaster = 1
```

Also we can make sure that there is only one process running when
shmem_startup_hook is called.

So it looks like acquiring AddinShmemInitLock in the hook is redundant
and also placing LWLock* in local process memory instead of shared
memory is safe.

Unless I missed something, I suggest updating the documentation and
pg_stat_statements.c accordingly.

[1]: https://github.com/afiskon/postgresql-extensions/blob/main/006-shared-memory/experiment.c#L38

--
Best regards,
Aleksander Alekseev

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ian Lawrence Barwick 2023-05-21 12:30:01 Re: PG 16 draft release notes ready
Previous Message Alena Rybakina 2023-05-21 09:23:14 Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)