Re: [PATCH] Make ENOSPC not fatal in semaphore creation

On Mon, Oct 18, 2021 at 10:07:40AM +1300, Thomas Munro wrote:
> On Mon, Oct 18, 2021 at 4:49 AM Mikhail <mp39590(at)gmail(dot)com> wrote:
> > The logic works - the initial call to semget() in
> > InternalIpcSemaphoreCreate returns -1 and errno is set to ENOSPC - I
> > tested the patch on OpenBSD 7.0, it successfully recycles sem's after
> > previous "pkill -6 postgres". Verified it with 'ipcs -s'.
>
> Since you mentioned OpenBSD, what do you think of the idea of making
> named POSIX semas the default on that platform? You can't run out of
> those practically speaking, but then you get lots of little memory
> mappings (from memory, at least it does close the fd for each one,
> unlike some other OSes where we wouldn't want to use this technique).
> Trivial patch:
>
> https://www.postgresql.org/message-id/CA%2BhUKGJVSjiDjbJpHwUrvA1TikFnJRfyJanrHofAWhnqcDJayQ%40mail.gmail.com
>
> No strong opinion on the tradeoffs here, as I'm not an OpenBSD user,
> but it's something I think about whenever testing portability stuff
> there and having to adjust the relevant sysctls.
>
> Note: The best kind would be *unnamed* POSIX semas, where we get to
> control their placement in existing memory; that's what we do on Linux
> and FreeBSD. They weren't supported on OpenBSD last time we checked:
> it rejects requests for shared ones. I wonder if someone could
> implement them with just a few lines of user space code, using atomic
> counters and futex() for waiting.

Hello, sorry for not replying earlier - I was able to think about and
test the patch only on the weekend.

I totally agree with your approach, in conversation with one of the
OpenBSD developers he supported using of sem_open(), because most ports
use it and consistency is desirable across our ports tree. It looks like
PostgreSQL was the only port to use semget().

Switching to sem_open() also looks much safer than patching sysv_sema.c
for corner ENOSPC case as Tom already mentioned.

In your patch I've removed testing for 5.x versions, because official
releases are supported only for one year, no need to worry about them.
The patch is tested with 'make installcheck', also I can confirm that
'ipcs' shows that no semaphores are used, and server starts normally
after 'pkill -6 postgres' with the default semmns sysctl, what was the
original motivation for the work.

diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index d74d1ed7af..2dfea0662b 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -998,21 +998,7 @@ psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed: No such
<para>
The default shared memory settings are usually good enough, unless
you have set <literal>shared_memory_type</literal> to <literal>sysv</literal>.
- You will usually want to
- increase <literal>kern.seminfo.semmni</literal>
- and <literal>kern.seminfo.semmns</literal>,
- as <systemitem class="osname">OpenBSD</systemitem>'s default settings
- for these are uncomfortably small.
- </para>
-
- <para>
- IPC parameters can be adjusted using <command>sysctl</command>,
- for example:
-<screen>
-<prompt>#</prompt> <userinput>sysctl kern.seminfo.semmni=100</userinput>
-</screen>
- To make these settings persist over reboots, modify
- <filename>/etc/sysctl.conf</filename>.
+ System V semaphores are not used on this platform.
</para>

</listitem>
diff --git a/src/template/openbsd b/src/template/openbsd
index 365268c489..41221af382 100644
--- a/src/template/openbsd
+++ b/src/template/openbsd
@@ -2,3 +2,7 @@

# Extra CFLAGS for code that will go into a shared library
CFLAGS_SL="-fPIC -DPIC"
+
+# OpenBSD 5.5 (2014) gained named POSIX semaphores. They work out of the box
+# without changing any sysctl settings, unlike System V semaphores.
+USE_NAMED_POSIX_SEMAPHORES=1