Re: replication commands and log_statements

On 12/06/14 20:37, Fujii Masao wrote:
> On Wed, Jun 11, 2014 at 11:55 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)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 line with
"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