Обсуждение: pgsql: Fix for early log messages during postmaster startup getting lost

Log Message:
-----------
Fix for early log messages during postmaster startup getting lost when
running as a service on Win32.

Per report from Harald Armin Massa.

Modified Files:
--------------
    pgsql/src/backend/postmaster:
        postmaster.c (r1.519 -> r1.520)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/postmaster.c.diff?r1=1.519&r2=1.520)
    pgsql/src/backend/utils/error:
        elog.c (r1.181 -> r1.182)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/error/elog.c.diff?r1=1.181&r2=1.182)

On Sun, 2007-02-11 at 07:59 -0400, Magnus Hagander wrote:
> Fix for early log messages during postmaster startup getting lost when
> running as a service on Win32.

FYI, it is considered good practise to commit a patch at approximately
(or exactly) the same time on all branches, so tools like cvs2cl will be
more likely to collect the changes together.

>     pgsql/src/backend/postmaster:
>         postmaster.c (r1.519 -> r1.520)
>         (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/postmaster.c.diff?r1=1.519&r2=1.520)

Comments like that are fragile (elog.c could change, for example), and
basically content-free anyway, IMHO. If you need to make SysLoggerPID
part of postmaster.c's external API, why not just do that, remove the
comment, and add the extern declaration to postmaster.h?

-Neil

Neil Conway <neilc@samurai.com> writes:
> FYI, it is considered good practise to commit a patch at approximately
> (or exactly) the same time on all branches, so tools like cvs2cl will be
> more likely to collect the changes together.

It's also helpful to use exactly the same text for the log messages in
all the branches.  Again, this is so that cvs2cl understands they're
the same patch.  I tend not to bother with "Backpatch to xxx" comments,
as the CVS log makes that perfectly clear anyway; but if you use them,
they should be the same for all branches committed to.

> Comments like that are fragile (elog.c could change, for example), and
> basically content-free anyway, IMHO. If you need to make SysLoggerPID
> part of postmaster.c's external API, why not just do that, remove the
> comment, and add the extern declaration to postmaster.h?

The counterargument is that he avoided exporting SysLoggerPID to
anything except elog.c.  If it's in postmaster.h then who knows what
will start depending on it?  But I see Neil's point too; this coding
is definitely a bit fragile.

            regards, tom lane

On Sun, Feb 11, 2007 at 11:47:01PM -0500, Tom Lane wrote:
> Neil Conway <neilc@samurai.com> writes:
> > FYI, it is considered good practise to commit a patch at approximately
> > (or exactly) the same time on all branches, so tools like cvs2cl will be
> > more likely to collect the changes together.

Ok. I specifically delayed the second parts to see it run on a couple of
different platforms on th ebuildfarm. Wasn't aware of that part of how
cvs2cl worked - will do different next time.

> It's also helpful to use exactly the same text for the log messages in
> all the branches.  Again, this is so that cvs2cl understands they're
> the same patch.  I tend not to bother with "Backpatch to xxx" comments,
> as the CVS log makes that perfectly clear anyway; but if you use them,
> they should be the same for all branches committed to.

Ok. Wasn't aware of that part either, but will try to remember next time.

> > Comments like that are fragile (elog.c could change, for example), and
> > basically content-free anyway, IMHO. If you need to make SysLoggerPID
> > part of postmaster.c's external API, why not just do that, remove the
> > comment, and add the extern declaration to postmaster.h?
>
> The counterargument is that he avoided exporting SysLoggerPID to
> anything except elog.c.  If it's in postmaster.h then who knows what
> will start depending on it?  But I see Neil's point too; this coding
> is definitely a bit fragile.

That was my argument for doing it the way I did. I'll change it if you
want me to, of course :-) I can see both arguments, but can't quite
decide which wins...

//Magnus

Magnus Hagander wrote:
> > > Comments like that are fragile (elog.c could change, for example), and
> > > basically content-free anyway, IMHO. If you need to make SysLoggerPID
> > > part of postmaster.c's external API, why not just do that, remove the
> > > comment, and add the extern declaration to postmaster.h?
> >
> > The counterargument is that he avoided exporting SysLoggerPID to
> > anything except elog.c.  If it's in postmaster.h then who knows what
> > will start depending on it?  But I see Neil's point too; this coding
> > is definitely a bit fragile.
>
> That was my argument for doing it the way I did. I'll change it if you
> want me to, of course :-) I can see both arguments, but can't quite
> decide which wins...

I add it so the commit message on HEAD also shows the back branchs.  It
makes it easier when creating the major release notes whether the patch
has already appeared in a back branch, and hence shouldn't be mentioned.

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +