From: Andres Freund <andres@anarazel.de>
To: Noah Misch <noah@leadboat.com>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Consistently test for in-use shared memory.
Date: Wed, 3 Apr 2019 19:00:57 -0700
Message-ID: <20190404020057.galelv7by75ekqrh@alap3.anarazel.de> (raw)
In-Reply-To: <E1hBq3r-0002Dz-CX@gemulon.postgresql.org>
References: <E1hBq3r-0002Dz-CX@gemulon.postgresql.org>
Hi,
On 2019-04-04 00:16:55 +0000, Noah Misch wrote:
> Consistently test for in-use shared memory.
>
> postmaster startup scrutinizes any shared memory segment recorded in
> postmaster.pid, exiting if that segment matches the current data
> directory and has an attached process. When the postmaster.pid file was
> missing, a starting postmaster used weaker checks. Change to use the
> same checks in both scenarios. This increases the chance of a startup
> failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
> postmaster.pid` && rm postmaster.pid && pg_ctl -w start". A postmaster
> will no longer recycle segments pertaining to other data directories.
> That's good for production, but it's bad for integration tests that
> crash a postmaster and immediately delete its data directory. Such a
> test now leaks a segment indefinitely. No "make check-world" test does
> that. win32_shmem.c already avoided all these problems. In 9.6 and
> later, enhance PostgresNode to facilitate testing. Back-patch to 9.4
> (all supported versions).
>
> Reviewed by Daniel Gustafsson and Kyotaro HORIGUCHI.
>
> Discussion: https://postgr.es/m/20130911033341.GD225735@tornado.leadboat.com
My compiler now nitpicks:
In file included from /home/andres/src/postgresql/src/include/postgres.h:47,
from pg_shmem.c:20:
pg_shmem.c: In function ‘PGSharedMemoryCreate’:
/home/andres/src/postgresql/src/include/utils/elog.h:122:5: warning: this statement may fall through [-Wimplicit-fallthrough=]
do { \
^
/home/andres/src/postgresql/src/include/utils/elog.h:142:2: note: in expansion of macro ‘ereport_domain’
ereport_domain(elevel, TEXTDOMAIN, rest)
^~~~~~~~~~~~~~
pg_shmem.c:668:5: note: in expansion of macro ‘ereport’
ereport(FATAL,
^~~~~~~
pg_shmem.c:675:4: note: here
case SHMSTATE_ENOENT:
^~~~
All of PostgreSQL successfully made. Ready to install.
I think the SHMSTATE_ATTACHED case simply should grow a (unreachable)
break. But I'd also strongly suggest adding one to SHMSTATE_UNATTACHED -
because that's actually reachable, and just seems like a trap for person
adding another case:.
Greetings,
Andres Freund
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: andres@anarazel.de, noah@leadboat.com, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Consistently test for in-use shared memory.
In-Reply-To: <20190404020057.galelv7by75ekqrh@alap3.anarazel.de>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox