RE: Resetting recovery target parameters in pg_createsubscriber

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Alena Vinter' <dlaaren8@gmail.com>, Michael Paquier <michael@paquier.xyz>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2025-10-01T04:38:42Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. pg_createsubscriber: Improve handling of automated recovery configuration

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