Re: replication commands and log_statements

On Thu, Jun 19, 2014 at 5:29 PM, Ian Barwick <ian(at)2ndquadrant(dot)com> wrote:
> 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:

Sorry, I've noticed this email right now. Thanks for reviewing the patch!

> - 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>

Yep, fixed. Attached is the updated version of the patch.

>
> - 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.

But log_statement is in the singular form. So I just used
log_replication_command. For the consistency, maybe we need to
change both parameters in the plural form? I don't have strong
opinion about this.

> - 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

Yes. I added that.

Regards,

--
Fujii Masao