On 12/06/14 20:37, Fujii Masao wrote:
On Wed, Jun 11, 2014 at 11:55 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
Andres Freund <and...@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 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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to