On 2015-10-12 21:38:12 +0900, Michael Paquier wrote:
> >> It feels wrong to do this in syslogger.c - I mean it's not the only
> >> process that's not attached to shared memory. Sure, the others get
> >> killed, but nonetheless...
> >
> >
> > +1. It feels like we're setting our selves up for repeating this mistake at
> > some later time :)
>
> Actually, doesn't this apply as well to the archiver and the pgstat
> collector?
As mentioned above? The difference is that the archiver et al get killed
by postmaster during a PANIC restart thus don't present the problem
discussed here.
> So perhaps we may want to do that in SubPostmasterMain with
> PGSharedMemoryDetach. See for example the attached as an idea (patch
> completely untested).
> + /*
> + * Close any existing shared memory segment as those processes do not
> + * need to have an access to it. This state is inherited from the
> + * postmaster whether they need it or not.
> + */
> + if (strcmp(argv[1], "--forkarch") == 0 ||
> + strcmp(argv[1], "--forkcol") == 0 ||
> + strcmp(argv[1], "--forklog") == 0)
> + PGSharedMemoryDetach();
> +
Well, in those cases we won't have attached to shared memory, so I'm not
convinced that this is the right solution. In fact, won't this lead to
hitting the elog in
void
PGSharedMemoryDetach(void)
{if (UsedShmemSegAddr != NULL){ if (!UnmapViewOfFile(UsedShmemSegAddr)) elog(LOG, "could not unmap view of
sharedmemory: error code %lu", GetLastError());
UsedShmemSegAddr = NULL;}
}
UsedShmemSegAddr will have been setup by read_backend_variables(), but
the process won't have anything mapped at this point?
Greetings,
Andres Freund