Date: Sat, 11 May 2019 12:07:15 -0700
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Unexpected "shared memory block is still in use"
Message-ID: <20190511190715.GA1124997@rfd.leadboat.com>
References: <22224.1557340366@sss.pgh.pa.us>
 <20190509055414.GB1066859@rfd.leadboat.com>
 <2663.1557409654@sss.pgh.pa.us>
 <2226.1557439179@sss.pgh.pa.us>
 <17642.1557442078@sss.pgh.pa.us>
 <20190510072213.GA1098093@rfd.leadboat.com>
 <25485.1557500141@sss.pgh.pa.us>
 <16908.1557521200@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <16908.1557521200@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Fri, May 10, 2019 at 04:46:40PM -0400, Tom Lane wrote:
> I wrote:
> > Will go fix/backpatch in a minute.
> 
> 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.

> I think the reason for doing it this way originally was to allow
> one to identify which shmem segment is which in "ipcs -m" output.
> But that was back when having to clean up shmem segments manually
> was still a common task.  It's been a long time since I can remember
> needing to figure out which was which.

I don't see that presenting a problem these days, agreed.