FAQ

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.

regards, tom lane

Search Discussions

  • Magnus Hagander at Jul 27, 2009 at 9:01 am

    On Sat, Jul 25, 2009 at 19:50, Tom Lanewrote:
    mha@postgresql.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?
  • Tom Lane at Jul 27, 2009 at 2:15 pm

    Magnus Hagander writes:
    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?
    I'm not really insisting on a redesign. I'm talking about the places
    where the code author appears not to have understood that ERROR means
    FATAL, because the code keeps plugging on after it anyway. As far as
    I can see, using ERROR at lines 3630, 3657, 3674, and 3693 is just
    plain bogus, and changing to LOG there requires no other fixing.

    regards, tom lane
  • Magnus Hagander at Jul 28, 2009 at 12:39 pm

    On Mon, Jul 27, 2009 at 16:14, Tom Lanewrote:
    Magnus Hagander <magnus@hagander.net> writes:
    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?
    I'm not really insisting on a redesign.  I'm talking about the places
    where the code author appears not to have understood that ERROR means
    FATAL, because the code keeps plugging on after it anyway.  As far as
    I can see, using ERROR at lines 3630, 3657, 3674, and 3693 is just
    plain bogus, and changing to LOG there requires no other fixing.
    3630: can't happen, because we already elog(ERROR) deep in the
    function, which is what I tried to outline above. That's the one
    requiring a redesign - because the errors *inside* the function it
    calls can certainly happen.
    3657: is one of those "should never happen" issues - which we haven't
    had any reports of.
    3674: see above
    3693 is a comment, I assume you mean 3683, which is also one of those
    can't happen.

    But. I'll look into cleaning those up for HEAD anyway, but due to lack
    of reports I think we should skip backpatch. Reasonable?
  • Tom Lane at Jul 28, 2009 at 1:46 pm

    Magnus Hagander writes:
    On Mon, Jul 27, 2009 at 16:14, Tom Lanewrote:
    I'm not really insisting on a redesign.  I'm talking about the places
    where the code author appears not to have understood that ERROR means
    FATAL, because the code keeps plugging on after it anyway.  As far as
    I can see, using ERROR at lines 3630, 3657, 3674, and 3693 is just
    plain bogus, and changing to LOG there requires no other fixing.
    But. I'll look into cleaning those up for HEAD anyway, but due to lack
    of reports I think we should skip backpatch. Reasonable?
    Fair enough.

    regards, tom lane
  • Magnus Hagander at Aug 5, 2009 at 12:38 pm

    On Tue, Jul 28, 2009 at 15:45, Tom Lanewrote:
    Magnus Hagander <magnus@hagander.net> writes:
    On Mon, Jul 27, 2009 at 16:14, Tom Lanewrote:
    I'm not really insisting on a redesign.  I'm talking about the places
    where the code author appears not to have understood that ERROR means
    FATAL, because the code keeps plugging on after it anyway.  As far as
    I can see, using ERROR at lines 3630, 3657, 3674, and 3693 is just
    plain bogus, and changing to LOG there requires no other fixing.
    But. I'll look into cleaning those up for HEAD anyway, but due to lack
    of reports I think we should skip backpatch. Reasonable?
    Fair enough.
    Here's what I came up with. Seems ok?
  • Tom Lane at Aug 5, 2009 at 2:54 pm

    Magnus Hagander writes:
    But. I'll look into cleaning those up for HEAD anyway, but due to lack
    of reports I think we should skip backpatch. Reasonable?
    Fair enough.
    Here's what I came up with. Seems ok?
    Works for me.

    regards, tom lane
  • Magnus Hagander at Aug 6, 2009 at 12:05 pm

    On Wed, Aug 5, 2009 at 16:53, Tom Lanewrote:
    Magnus Hagander <magnus@hagander.net> writes:
    But. I'll look into cleaning those up for HEAD anyway, but due to lack
    of reports I think we should skip backpatch. Reasonable?
    Fair enough.
    Here's what I came up with. Seems ok?
    Works for me.
    Applied.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJul 25, '09 at 5:50p
activeAug 6, '09 at 12:05p
posts8
users2
websitepostgresql.org...
irc#postgresql

2 users in discussion

Magnus Hagander: 4 posts Tom Lane: 4 posts

People

Translate

site design / logo © 2022 Grokbase