From: Tom Lane <tgl@sss.pgh.pa.us>
To: Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>
cc: pgsql-bugs@lists.postgresql.org,
        Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Subject: Re: BUG #15804: Assertion failure when using logging_collector with
 EXEC_BACKEND
In-reply-to: <27CD45B5-88F7-4A09-8B31-EFE41FC37FE1@crunchydata.com>
References: <15804-3721117bf40fb654@postgresql.org>
 <27CD45B5-88F7-4A09-8B31-EFE41FC37FE1@crunchydata.com>
Comments: In-reply-to Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>
	message dated "Tue, 14 May 2019 14:45:09 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <26331.1557863524.1@sss.pgh.pa.us>
Date: Tue, 14 May 2019 15:52:04 -0400
Message-ID: <26332.1557863524@sss.pgh.pa.us>
Precedence: bulk

Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com> writes:
> Attached is a patch that fixes the issue in the bug report.

I do not think this is a very good way to fix it (even assuming
that it works, which I'm unsure of --- in particular, doing this
would alter the order of changes to postmaster.pid, possibly
breaking pg_ctl or other tools that look at that file).  The whole
point of commit 57431a911 was to switch to log-collector logging
before we've done anything very interesting, and that would surely
include shmem setup.  I'm a bit surprised actually that Peter
didn't move the SysLogger_Start call even further up; in principle
it ought to be safe to do it as soon as we have the data directory
lock file.

It might be better to give up the assertion in PGSharedMemoryNoReAttach,
and just make it work more like PGSharedMemoryDetach, ie "detach if
UsedShmemSegAddr is set, else do nothing".  I don't remember for sure,
but if we do that, there might be no functional difference anymore
between those two functions, in which case we might as well merge 'em.

			regards, tom lane