Thread: Re: BUG #1466: syslogger issues

Re: BUG #1466: syslogger issues

From
"Magnus Hagander"
Date:
>>>>There is special code in the send_message_to_server_log
>>>
>>>function to make
>>>
>>>>sure it's written directly to the file.
>>>
>>>If the logger is complaining, it's quite possibly because it's
>>>unable to
>>>write to its file.  Now that you mention it, doesn't this
>code go into
>>>infinite recursion if write_syslogger_file_binary() tries to ereport?
>
>Yes, apparently.
>
>Actually, elog.c code should look like this:
>
>if ((Log_destination & LOG_DESTINATION_STDERR) ...)
>{
>    if (am_syslogger)
>       write_syslogger_file(buf.data, buf.len);
>    else
>       fwrite(buf.data, 1, buf.len, stderr);
>}
>
>This avoids unnecessary pipe traffic (which might fail too)
>and gettext translation.

That's sort of what I thought, but without being certain at all.


>Next, the elog call in write_syslogger_file_binary will almost
>certainly
>loop, so it should call write_stderr then (since eventlog is usually
>fixed-size with cyclic writing, even in out-of-disk-space conditions
>something might get logged).

Ok. I've included these changes in the attached patch. Haven't tested
those specific codepaths, but the other changes still work...

>3rd, I've been proposing to have redirect_stderr=true on by default at
>least on win32 earlier, I still think this is reasonable.

It's already the default if you install from the MSI installer.

//Magnus

Attachment

Re: BUG #1466: syslogger issues

From
Bruce Momjian
Date:
Previous version of patch removed from queue.

Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Magnus Hagander wrote:
> >>>>There is special code in the send_message_to_server_log
> >>>
> >>>function to make
> >>>
> >>>>sure it's written directly to the file.
> >>>
> >>>If the logger is complaining, it's quite possibly because it's
> >>>unable to
> >>>write to its file.  Now that you mention it, doesn't this
> >code go into
> >>>infinite recursion if write_syslogger_file_binary() tries to ereport?
> >
> >Yes, apparently.
> >
> >Actually, elog.c code should look like this:
> >
> >if ((Log_destination & LOG_DESTINATION_STDERR) ...)
> >{
> >    if (am_syslogger)
> >       write_syslogger_file(buf.data, buf.len);
> >    else
> >       fwrite(buf.data, 1, buf.len, stderr);
> >}
> >
> >This avoids unnecessary pipe traffic (which might fail too)
> >and gettext translation.
>
> That's sort of what I thought, but without being certain at all.
>
>
> >Next, the elog call in write_syslogger_file_binary will almost
> >certainly
> >loop, so it should call write_stderr then (since eventlog is usually
> >fixed-size with cyclic writing, even in out-of-disk-space conditions
> >something might get logged).
>
> Ok. I've included these changes in the attached patch. Haven't tested
> those specific codepaths, but the other changes still work...
>
> >3rd, I've been proposing to have redirect_stderr=true on by default at
> >least on win32 earlier, I still think this is reasonable.
>
> It's already the default if you install from the MSI installer.
>
> //Magnus

Content-Description: stderr.patch

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: BUG #1466: syslogger issues

From
Bruce Momjian
Date:
Patch applied by Tom.  Thanks.

Backpatched to 8.0.X.

---------------------------------------------------------------------------


Magnus Hagander wrote:
> >>>>There is special code in the send_message_to_server_log
> >>>
> >>>function to make
> >>>
> >>>>sure it's written directly to the file.
> >>>
> >>>If the logger is complaining, it's quite possibly because it's
> >>>unable to
> >>>write to its file.  Now that you mention it, doesn't this
> >code go into
> >>>infinite recursion if write_syslogger_file_binary() tries to ereport?
> >
> >Yes, apparently.
> >
> >Actually, elog.c code should look like this:
> >
> >if ((Log_destination & LOG_DESTINATION_STDERR) ...)
> >{
> >    if (am_syslogger)
> >       write_syslogger_file(buf.data, buf.len);
> >    else
> >       fwrite(buf.data, 1, buf.len, stderr);
> >}
> >
> >This avoids unnecessary pipe traffic (which might fail too)
> >and gettext translation.
>
> That's sort of what I thought, but without being certain at all.
>
>
> >Next, the elog call in write_syslogger_file_binary will almost
> >certainly
> >loop, so it should call write_stderr then (since eventlog is usually
> >fixed-size with cyclic writing, even in out-of-disk-space conditions
> >something might get logged).
>
> Ok. I've included these changes in the attached patch. Haven't tested
> those specific codepaths, but the other changes still work...
>
> >3rd, I've been proposing to have redirect_stderr=true on by default at
> >least on win32 earlier, I still think this is reasonable.
>
> It's already the default if you install from the MSI installer.
>
> //Magnus

Content-Description: stderr.patch

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073