Re: Add support for logging the current role

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add support for logging the current role
Date: 2011-02-15 13:43:23
Message-ID: 20110215134323.GU4116@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

* Itagaki Takahiro (itagaki(dot)takahiro(at)gmail(dot)com) wrote:
> On Mon, Feb 14, 2011 at 23:30, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > > * In assign_csvlog_fields(), we need to cleanup memory and memory context
> > > before return on error.
> > Fixed this and a couple of similar issues.
>
> Not yet fixed. Switched memory context is not restored on error.

Ugh, sorry about that, I should have realized that needed to be done.
Updated patch attached.

> > Updated patch attached, git log below.
>
> Now I mark the patch to "Ready for Committer",
> because I don't have suggestions any more.

Thanks.

> For reference, I note my previous questions. Some of them might be TODO
> items, or might not. We can add the basic feature in 9.1, and improve it
> 9.2 or later versions.

That's what I would have thought, ah well. :)

> * csvlog_fields and csvlog_header won't work with non-default log_filename
> when it doesn't contain seconds in the format. They expect they can always
> open empty log files.

Or that the user will deal with changes and header lines mid-file if
they decide to use a log_filename which causes it to happen.

> * The long default value for csvlog_fields leads long text line in
> postgresql.conf, SHOW ALL, pg_settings view, but there were no better
> alternative solutions in the past discussion.

Not without making GUCs able to be multi-line. If people are interested
in this, I'll try to make it happen for 9.2.

> * csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats
> in a csv file on default log_filename, but other similar GUC variables
> are usually marked AS PGC_SIGHUP.

The problem here is primairly that each backend does write_csvlog() and
there's no easy way to make sure that none of them write the new format
to the old file (or the old format to the new file) before a switch is
done. I can try looking into this but I'm concerned the only solution
would be to introduce some amount of locking which could slow down the
overall logging process which might be unacceptable performance-wise.

Preventing CSV logs from appending to existing files could be pretty
easily done, provided we can agree on what to call the new files, but
that wouldn't change PGC_POSTMASTER.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2011-02-15 13:51:31 Re: Add support for logging the current role
Previous Message Stephen Frost 2011-02-15 13:33:02 Re: pg_ctl failover Re: Latches, signals, and waiting