Re: BUG #15804: Assertion failure when using logging_collector withEXEC_BACKEND

On Thu, May 16, 2019 at 03:06:28PM +0900, Michael Paquier wrote:
> As far as my first investigations have gone, assertions in
> pgwin32_ReserveSharedMemoryRegion() and
> PGSharedMemoryReAttach():sysv_shmem.c would need relaxing, but we rely
> on that since a7e5878, which is quite some time ago.  And, actually,
> the failure for pgwin32_ReserveSharedMemoryRegion() should never
> happen at all, because we need to call reset_shared() before starting
> the syslogger as well, no?

Trying to avoid calling pgwin32_ReserveSharedMemoryRegion() for
processes which are never going to connect to shared memory is
actually an interesting option, because this way there is no need to
relax the assertions it includes, and there is no point in reserving a
shared memory area if it is never going to use it.  This would not
cause a process trying to reattach to cause conflicts as well, which
is why we pre-allocate the region in the first place.

So, I have been hacking my way through this idea, and finished with
the attached.  I have added the changes to src/tools/msvc/ and
postgresql.conf.sample (enabling logging_collector by default) to test
the viability of the change, and after fixing the first assertion
issues and the second one with the temporary files, I have bumped into
a third issue causing recovery/t/017_shm.pl to remain stuck on Linux
with -DEXEC_BACKEND as the syslogger keeps starting and stopping, as
it enters.  This gets masked because of the temporary file issues
and/or the assertion problems.

This may sound rough, but based on the timing and the time I can spend
on that, I really think that we should revert 57431a9, as this has not
been completely thought through for EXEC_BACKEND, and the patch I am
attaching is a set of rough counter-measures which keep piling up,
giving a result I am not comfortable with at all.  I am not sure that
I could actually go to the bottom of it without reworking the way we
attach, detach and reattach to shared memory a process, particularly
on Windows where we don't actually need to allocate a region for a
process we know will never use shared memory (right?).  And this is no
time to do so for v12.  Last year I did the same kind of mistake by
underestimating the Windows-related stuff for v11 just before a
release which has resulted in an urgent revert as of df8b5f3.  I don't
want to do the same mistake again.
--
Michael