Re: Unexpected "shared memory block is still in use"

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Fri, May 10, 2019 at 04:46:40PM -0400, Tom Lane wrote:
>> Done now, but while thinking more about the issue, I had an idea: why is
>> it that we base the shmem key on the postmaster's port number, and not
>> on the data directory's inode number? Using the port number not only
>> increases the risk of collisions (though admittedly only in testing
>> situations), but it *decreases* our ability to detect real conflicts.
>> Consider case where DBA wants to change the installation's port number,
>> and he edits postgresql.conf, but then uses "kill -9 && rm postmaster.pid"
>> rather than some saner way of stopping the old postmaster. When he
>> starts the new one, it won't detect any remaining children of the old
>> postmaster because it'll be looking in the wrong range of shmem keys.
>> It seems like something tied to the data directory's identity would
>> be much more trustworthy.

> Good point. Since we now ignore (SHMSTATE_FOREIGN) any segment that bears
> (st_dev,st_ino) not matching $PGDATA, the change you describe couldn't make us
> fail to detect a real conflict or miss a cleanup opportunity. It would reduce
> the ability to test sysv_shmem.c; I suppose one could add a debug GUC to
> override the start of the key space.

Attached is a draft patch to change both shmem and sema key selection
to be based on data directory inode rather than port.

I considered using "st_ino ^ st_dev", or some such, but decided that
that would largely just make it harder to manually correlate IPC
keys with running postmasters. It's generally easy to find out the
data directory inode number with "ls", but the extra work to find out
and XOR in the device number is not so easy, and it's not clear what
it'd buy us in typical scenarios.

The Windows code seems fine as-is: it's already using data directory
name, not port, to set up shmem, and it doesn't need anything for
semaphores.

I'm not quite sure what's going on in src/test/recovery/t/017_shm.pl.
As expected, the test for port number non-collision no longer sees
a failure. After fixing that, the test passes, but it takes a
ridiculously long time (minutes); apparently each postmaster start/stop
cycle takes much longer than it ought to. I suppose this patch is
breaking its assumptions, but I've not studied it. We'd have to do
something about that before this would be committable.

I'll add this to the next commitfest.

regards, tom lane