On Thu, Jun 19, 2014 at 5:29 PM, Ian Barwick <i...@2ndquadrant.com> wrote:
> 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:
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
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 4558,4563 **** FROM pg_stat_activity;
--- 4558,4581 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-replication-command" xreflabel="log_replication_command">
+ <term><varname>log_replication_command</varname> (<type>boolean</type>)
+ <indexterm>
+ <primary><varname>log_replication_command</> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Causes each replication command to be logged in the server log.
+ See <xref linkend="protocol-replication"> for more information about
+ replication command. The default value is <literal>off</>. When
+ set to <literal>off</>, commands will be logged at log level
+ <literal>DEBUG1</> (or lower).
+ Only superusers can change this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-temp-files" xreflabel="log_temp_files">
<term><varname>log_temp_files</varname> (<type>integer</type>)
<indexterm>
*** a/doc/src/sgml/protocol.sgml
--- b/doc/src/sgml/protocol.sgml
***************
*** 1306,1311 **** To initiate streaming replication, the frontend sends the
--- 1306,1314 ----
of <literal>true</> tells the backend to go into walsender mode, wherein a
small set of replication commands can be issued instead of SQL statements. Only
the simple query protocol can be used in walsender mode.
+ Replication commands are logged in the server log at log level
+ <literal>DEBUG1</> (or lower) or when
+ <xref linkend="guc-log-replication-command"> is enabled.
Passing <literal>database</> as the value instructs walsender to connect to
the database specified in the <literal>dbname</> parameter, which will allow
the connection to be used for logical replication from that database.
*** a/src/backend/replication/walsender.c
--- b/src/backend/replication/walsender.c
***************
*** 108,113 **** bool am_db_walsender = false; /* Connected to a database? */
--- 108,114 ----
int max_wal_senders = 0; /* the maximum number of concurrent walsenders */
int wal_sender_timeout = 60 * 1000; /* maximum time to send one
* WAL data message */
+ bool log_replication_command = false;
/*
* State for WalSndWakeupRequest
***************
*** 1268,1280 **** exec_replication_command(const char *cmd_string)
MemoryContext old_context;
/*
* CREATE_REPLICATION_SLOT ... LOGICAL exports a snapshot until the next
* command arrives. Clean up the old stuff if there's anything.
*/
SnapBuildClearExportedSnapshot();
- elog(DEBUG1, "received replication command: %s", cmd_string);
-
CHECK_FOR_INTERRUPTS();
cmd_context = AllocSetContextCreate(CurrentMemoryContext,
--- 1269,1287 ----
MemoryContext old_context;
/*
+ * Log replication command if log_replication_command is enabled.
+ * Even when it's disabled, log the command with DEBUG1 level for
+ * backward compatibility.
+ */
+ ereport(log_replication_command ? LOG : DEBUG1,
+ (errmsg("received replication command: %s", cmd_string)));
+
+ /*
* CREATE_REPLICATION_SLOT ... LOGICAL exports a snapshot until the next
* command arrives. Clean up the old stuff if there's anything.
*/
SnapBuildClearExportedSnapshot();
CHECK_FOR_INTERRUPTS();
cmd_context = AllocSetContextCreate(CurrentMemoryContext,
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 925,930 **** static struct config_bool ConfigureNamesBool[] =
--- 925,939 ----
NULL, NULL, NULL
},
{
+ {"log_replication_command", PGC_SUSET, LOGGING_WHAT,
+ gettext_noop("Logs each replication command."),
+ NULL
+ },
+ &log_replication_command,
+ false,
+ NULL, NULL, NULL
+ },
+ {
{"debug_assertions", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows whether the running server has assertion checks enabled."),
NULL,
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 431,436 ****
--- 431,437 ----
# e.g. '<%u%%%d> '
#log_lock_waits = off # log lock waits >= deadlock_timeout
#log_statement = 'none' # none, ddl, mod, all
+ #log_replication_command = off
#log_temp_files = -1 # log temporary files equal or larger
# than the specified size in kilobytes;
# -1 disables, 0 logs all temp files
*** a/src/include/replication/walsender.h
--- b/src/include/replication/walsender.h
***************
*** 25,30 **** extern bool wake_wal_senders;
--- 25,31 ----
/* user-settable parameters */
extern int max_wal_senders;
extern int wal_sender_timeout;
+ extern bool log_replication_command;
extern void InitWalSender(void);
extern void exec_replication_command(const char *query_string);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
