Re: [COMMITTERS] pgsql: Reserve the shared memory region during backend startup on

On Sat, Jul 25, 2009 at 19:50, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> mha(at)postgresql(dot)org (Magnus Hagander) writes:
>> Log Message:
>> -----------
>> Reserve the shared memory region during backend startup on Windows, so
>> that memory allocated by starting third party DLLs doesn't end up
>> conflicting with it.
>
> I am wondering why failure of the various TerminateProcess calls in
> postmaster.c is elog(ERROR) and not elog(LOG).  While that probably
> shouldn't happen, aborting the postmaster isn't a good response if it
> does.  This patch introduces a new occurrence, but I see it is just
> copying what was there already.

The case where it's doing it now is really a "can't happen" place, so
I don't think it's a big issue there. It could be argued that if we
can't terminate a process we just created (but never even started!),
something is very very badly broken on the system and we might as well
give up. Same for the part where we fail to ResumeThread() on the main
thread of a new process.

However, it seems that for example the one at line 3629 - where we're
just failing to save our backend state - shouldn't be such a FATAL
error. But if you actually look up into the function
save_backend_variables(), it's actually hardcoded to return true... In
case something goes wrong in there, there's an actual ereport(ERROR)
happening deep down already
(write_inheritable_socket/write_duplicated_handle).

To fix that we'd just have to turn those functions all into returning
boolean and log with LOG instead. AFAIK, we've had zero reports of
this actually happening, so I'm not sure it's worth redesigning.
Thoughts?

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/