On 12/06/14 20:37, Fujii Masao wrote:
> On Wed, Jun 11, 2014 at 11:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Andres Freund <andres@2ndquadrant.com> writes:
>>> Your wish just seems like a separate feature to me. Including
>>> replication commands in 'all' seems correct independent of the desire
>>> for a more granular control.
>>
>> No, I think I've got to vote with the other side on that.
>>
>> The reason we can have log_statement as a scalar progression
>> "none < ddl < mod < all" is that there's little visible use-case
>> for logging DML but not DDL, nor for logging SELECTS but not
>> INSERT/UPDATE/DELETE. However, logging replication commands seems
>> like something people would reasonably want an orthogonal control for.
>> There's no nice way to squeeze such a behavior into log_statement.
>>
>> I guess you could say that log_statement treats replication commands
>> as if they were DDL, but is that really going to satisfy users?
>>
>> I think we should consider log_statement to control logging of
>> SQL only, and invent a separate GUC (or, in the future, likely
>> more than one GUC?) for logging of replication activity.
>
> Seems reasonable. OK. The attached patch adds log_replication_command
> parameter which causes replication commands to be logged. I added this to
> next CF.
A quick review:
- Compiles against HEAD
- Works as advertised
- Code style looks fine
A couple of suggestions:
- minor rewording for the description, mentioning that statements will still be logged as DEBUG1 even if parameter set
to'off' (might prevent reports of the kind "I set it to 'off', why am I still seeing log entries?").
<para> Causes each replication command to be logged in the server log. See <xref
linkend="protocol-replication">for more information about replication commands. The default value is
<literal>off</>.When set to <literal>off</>, commands will be logged at log level <literal>DEBUG1</literal>.
Only superusers can change this setting. </para>
- I feel it would be more consistent to use the plural form for this parameter, i.e. "log_replication_commands", in
linewith "log_lock_waits", "log_temp_files", "log_disconnections" etc.
- It might be an idea to add a cross-reference to this parameter from the "Streaming Replication Protocol" page:
http://www.postgresql.org/docs/devel/static/protocol-replication.html
Regards
Ian Barwick
-- Ian Barwick http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services