Re: BUG #1466: syslogger issues
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 thiscode 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
Attachments:
stderr.patchapplication/octet-stream; name=stderr.patchDownload+26-26
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 thiscode 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?
--
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 19073I would like to apply this patch, and I think it is important enough
that it should be backpatched in to 8.0.X. Any objections?
---------------------------------------------------------------------------
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 thiscode 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?
--
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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
I would like to apply this patch, and I think it is important enough
that it should be backpatched in to 8.0.X. Any objections?
I wanted to review the patch before it went in. Will try to get to it
soon.
regards, tom lane
Tom Lane wrote:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
I would like to apply this patch, and I think it is important enough
that it should be backpatched in to 8.0.X. Any objections?I wanted to review the patch before it went in. Will try to get to it
soon.
OK, I will just keep it in the patch queue.
--
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 19073Patch 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 thiscode 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?
--
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