Lists: | pgsql-hackerspgsql-patches |
---|
From: | "Magnus Hagander" <mha(at)sollentuna(dot)net> |
---|---|
To: | "Andreas Pflug" <pgadmin(at)pse-consulting(dot)de> |
Cc: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "PostgreSQL-patches" <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: BUG #1466: syslogger issues |
Date: | 2005-02-21 19:56:57 |
Message-ID: | 6BCB9D8A16AC4241919521715F4D8BCE476936@algol.sollentuna.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
>>>>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 | Content-Type | Size |
---|---|---|
stderr.patch | application/octet-stream | 2.1 KB |
From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Magnus Hagander <mha(at)sollentuna(dot)net> |
Cc: | Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: BUG #1466: syslogger issues |
Date: | 2005-02-25 04:33:39 |
Message-ID: | 200502250433.j1P4Xdf03667@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Magnus Hagander <mha(at)sollentuna(dot)net> |
Cc: | Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
Subject: | Re: [PATCHES] BUG #1466: syslogger issues |
Date: | 2005-03-11 21:28:45 |
Message-ID: | 200503112128.j2BLSji24891@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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?
---------------------------------------------------------------------------
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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
Cc: | Magnus Hagander <mha(at)sollentuna(dot)net>, Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
Subject: | Re: [PATCHES] BUG #1466: syslogger issues |
Date: | 2005-03-11 21:54:38 |
Message-ID: | 5566.1110578078@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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
From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Magnus Hagander <mha(at)sollentuna(dot)net>, Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
Subject: | Re: [PATCHES] BUG #1466: syslogger issues |
Date: | 2005-03-11 21:56:27 |
Message-ID: | 200503112156.j2BLuRC01448@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Magnus Hagander <mha(at)sollentuna(dot)net> |
Cc: | Andreas Pflug <pgadmin(at)pse-consulting(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: BUG #1466: syslogger issues |
Date: | 2005-03-12 02:45:31 |
Message-ID: | 200503120245.j2C2jVk09352@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073