Re: Log a warning in pg_createsubscriber for max_slot_wal_keep_size

Hi Shubham,

Here are some review comments for patch v5-0001.

======
doc/src/sgml/ref/pg_createsubscriber.sgml

1.
+ <para>
+ The source server must have <xref linkend="guc-max-slot-wal-keep-size"/> to
+ be set to <literal>-1</literal> to prevent the automatic removal of WAL
+ replication slots. Setting this parameter to files needed by a
specific size
+ may lead to replication failures if required WAL files are
+ prematurely deleted.
+ </para>

IIUC, this problem is all about the removal of WAL *files*, not "WAL
replication slots".

Also, the "Setting this parameter to files needed by a specific size"
part sounds strange.

I think this can be simplified anyhow like below.

SUGGESTION:
Replication failures can occur if required WAL files are prematurely
deleted. To prevent this, the source server must set <xref
linkend="guc-max-slot-wal-keep-size"/> to <literal>-1</literal>,
ensuring WAL files are not automatically removed.

======
src/bin/pg_basebackup/pg_createsubscriber.c

check_publisher:

2.
+ if (dry_run && max_slot_wal_keep_size != -1)
+ {
+ pg_log_warning("publisher requires WAL size must not be restricted");
+ pg_log_warning_detail("The max_slot_wal_keep_size parameter is
currently set to a non-default value, which may lead to replication
failures. "
+ "This parameter must be set to -1 to ensure that required WAL
files are not prematurely removed.");
+ }

2a.
Whenever you mention a GUC name in a PostgreSQL message then (by
convention) it must be double-quoted.

~

2b.
The detailed message seems verbose and repetitive to me.

~

2c.
The other nearby messages use the terminology "configuration
parameter", so this should be the same.

~

2d.
The other nearby messages use \"%s\" substitution for the GUC name, so
this should be the same.

~

2e.
IMO advising the user to change a configuration parameter should be
done using the pg_log_warning_hint function (e.g. _hint, not
_details).

~~

Result:

CURRENT (pg_log_warning_details)
The max_slot_wal_keep_size parameter is currently set to a non-default
value, which may lead to replication failures. This parameter must be
set to -1 to ensure that required WAL files are not prematurely
removed.

SUGGESTION (pg_log_warning_hint)
Set the configuration parameter \"%s\" to -1 to ensure that required
WAL files are not prematurely removed.

======
Kind Regards,
Peter Smith.
Fujitsu Australia