Horton

Re: [HACKERS] Weaker shmem interlock w/o postmaster.pid

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: 9erthalion6@gmail.com, sfrost@snowman.net, pgsql-hackers@postgresql.org
Date: 2019-03-09T07:16:10Z
Lists: pgsql-hackers

Attachments

On Wed, Mar 06, 2019 at 07:24:22PM -0800, Noah Misch wrote:
> On Mon, Mar 04, 2019 at 06:04:20PM +0900, Kyotaro HORIGUCHI wrote:
> > PGSharedMemoryCreate changed to choose a usable shmem id using
> > the IpcMemoryAnalyze().  But some of the statuses from
> > IpcMemoryAnalyze() is concealed by failure of
> > PGSharedMemoryAttach() and ignored silently opposed to what the
> > code is intending to do.
> 
> SHMSTATE_FOREIGN is ignored silently.  The patch modifies the
> PGSharedMemoryAttach() header comment to direct callers to treat
> PGSharedMemoryAttach() failure like SHMSTATE_FOREIGN.  I don't think the code
> had an unintentional outcome due to the situation you describe.  Perhaps I
> could have made the situation clearer.

I think v3, attached, avoids this appearance of unintended behavior.

> > (By the way SHMSTATE_EEXISTS seems
> > suggesting oppsite thing from EEXIST, which would be confusing.)
> 
> Good catch.  Is SHMSTATE_ENOENT better?

I did s/SHMSTATE_EEXISTS/SHMSTATE_ENOENT/.

> > PGSharedMemoryCreate() repeats shmat/shmdt twice in every
> > iteration. It won't harm so much but it would be better if we
> > could get rid of that.
> 
> I'll try to achieve that and see if the code turns out cleaner.

I renamed IpcMemoryAnalyze() to PGSharedMemoryAttach() and deleted the old
function of that name.  Now, this function never calls shmdt(); the caller is
responsible for that.  I do like things better this way.  What do you think?

Commits

  1. Probe only 127.0.0.1 when looking for ports on Unix.

  2. Don't write to stdin of a test process that could have already exited.

  3. Test both 0.0.0.0 and 127.0.0.x addresses to find a usable port.

  4. MSYS: Translate REGRESS_SHLIB to a Windows file name.

  5. MSYS: Skip src/test/recovery/t/017_shm.pl.

  6. When Perl "kill(9, ...)" fails, try "pg_ctl kill".

  7. Consistently test for in-use shared memory.

  8. Revert "Consistently test for in-use shared memory."

  9. Silence -Wimplicit-fallthrough in sysv_shmem.c.

  10. Make src/test/recovery/t/017_shm.pl safe for concurrent execution.

  11. Update HINT for pre-existing shared memory block.

  12. Add WL_EXIT_ON_PM_DEATH pseudo-event.

  13. The default values for shared_buffers and max_connections are now 1000

  14. XLOG (and related) changes:

  15. Significant cleanups in SysV IPC handling (shared mem and semaphores).