This patch implements the protocol Tom suggested for writing to the
syslogger pipe. It seems to pass my tests (basically "make installcheck"
against a server with stderr redirection turned on and log_statement set
to 'all').

The effect of this should be to prevent two problems:
.. partial messages get written to the log file, which messes with
rotation, and
.. messages from various backends get interleaved, causing garbled logs.

Please review ASAP. I want to get this applied soon so that a) it gets
wider testing and b) I can use it as the basis for the adapted CSV log
patch. If this is acceptable I intend to backpatch this all the way to
wherever we started using the syslogger pipe (was that 8.0?).

cheers

andrew


---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

and here's the patch


Andrew Dunstan wrote:
>
> This patch implements the protocol Tom suggested for writing to the
> syslogger pipe. It seems to pass my tests (basically "make
> installcheck" against a server with stderr redirection turned on and
> log_statement set to 'all').
>
> The effect of this should be to prevent two problems:
> . partial messages get written to the log file, which messes with
> rotation, and
> . messages from various backends get interleaved, causing garbled logs.
>
> Please review ASAP. I want to get this applied soon so that a) it gets
> wider testing and b) I can use it as the basis for the adapted CSV log
> patch. If this is acceptable I intend to backpatch this all the way to
> wherever we started using the syslogger pipe (was that 8.0?).
>
> cheers
>
> andrew
>
>


---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>>> This patch implements the protocol Tom suggested for writing to the
>>> syslogger pipe. It seems to pass my tests (basically "make
>>> installcheck" against a server with stderr redirection turned on and
>>> log_statement set to 'all').
>>>
>
> I didn't like this patch much --- it broke the API of
> write_syslogger_file, which is supposed to just write what it's given
> (and it is used from outside syslogger.c with that expectation). Also
> the way it slung unconsumed data back and forth between two buffers
> seemed both confusing and inefficient. Here's a revised version.
>

Well. it was like the curate's egg :-) Anyway, thanks for the cleanup.

> In my testing, I found that a standard "make installcheck" run produces
> only one message large enough to be split (the "infinite_recurse" thing
> in errors.sql), so this is definitely not a Good Enough test. Maybe
> we could get Ed L. or one of the other complainants to try it.

Yeah, what I did was to wind back the chunk size - try 128 and you'll
see plenty of chunked messages :-) But we really need to do this with
installcheck-parallel to exercise it properly.

> (The
> patch seems to need some adjustment to apply against 8.2, though.)
>
>
>

I know we normally try not to do this, but I'd be happy to wait for the
back branches in this case.

cheers

andrew




---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Andrew Dunstan <andrew@dunslane.net> writes:
> Yeah, what I did was to wind back the chunk size - try 128 and you'll
> see plenty of chunked messages :-) But we really need to do this with
> installcheck-parallel to exercise it properly.

Doh, of course. I ran installcheck-parallel with log_statement = all
and the chunk size reduced to 64, and saw no apparent problems.
So that gives me at least enough confidence to apply it to HEAD.

>> (The
>> patch seems to need some adjustment to apply against 8.2, though.)

> I know we normally try not to do this, but I'd be happy to wait for the
> back branches in this case.

[confused...] How do you envision proceeding exactly?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Yeah, what I did was to wind back the chunk size - try 128 and you'll
>> see plenty of chunked messages :-) But we really need to do this with
>> installcheck-parallel to exercise it properly.
>>
>
> Doh, of course. I ran installcheck-parallel with log_statement = all
> and the chunk size reduced to 64, and saw no apparent problems.
> So that gives me at least enough confidence to apply it to HEAD.
>
>
>>> (The
>>> patch seems to need some adjustment to apply against 8.2, though.)
>>>
>
>
>> I know we normally try not to do this, but I'd be happy to wait for the
>> back branches in this case.
>>
>
> [confused...] How do you envision proceeding exactly?
>
>
>

Never mind, if you're happy adapting and applying this right away to
back branches then I'm happy too. I just didn't want to have to wait
much before I start work on the CSVlog adaptation.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> [confused...] How do you envision proceeding exactly?

> Never mind, if you're happy adapting and applying this right away to
> back branches then I'm happy too. I just didn't want to have to wait
> much before I start work on the CSVlog adaptation.

Actually, I was hoping you'd adapt/apply to the back branches ;-)

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Tom Lane wrote:
>
> Actually, I was hoping you'd adapt/apply to the back branches ;-)
>
>
>

curses! foiled again!

OK, will do.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings