Dear Alena,
Thanks for updating the patch. Few comments.
```
+ /* Before setting up the recovery parameters save the original content. */
+ savedrecoveryconfcontents = GetRecoveryConfig(conn, datadir);
```
To confirm, you put the connection to the primary/publisher instead of standby/subscriber.
But it is harmless because streaming replication requires that both instances
have the same major version. Is it correct?
```
+ pg_log_warning_hint("Manual removal of recovery parameters is required from 'postgresql.auto.conf'
(PostgreSQL%d+) or 'recovery.conf' (older versions)",
+ MINIMUM_VERSION_FOR_RECOVERY_GUC / 10000);
```
Can we cache the version info when we firstly connect to the primary node to
print appropriate filename? Or is it hacky?
```
+ if (dry_run)
+ {
+ appendPQExpBufferStr(savedrecoveryconfcontents, "# dry run mode");
+ }
```
Per my understanding, setup_recovery() puts the indicator becasue the content
can be printed. I think it is not needed since reset_recovery_params() does not
have that, or we can even print the parameters.
```
+sub test_param_absent
+{
+ my ($node, $param) = @_;
+ my $auto_conf = $node->data_dir . '/postgresql.auto.conf';
+
+ return 1 unless -e $auto_conf;
+
+ my $content = slurp_file($auto_conf);
+ return $content !~ /^\s*$param\s*=/m;
+}
```
Can you add a short comment atop the function? Something like:
"Check whether the given parameter is specified in postgresql.auto.conf"
Best regards,
Hayato Kuroda
FUJITSU LIMITED
