From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Alena Vinter' <dlaaren8(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: Resetting recovery target parameters in pg_createsubscriber |
Date: | 2025-10-01 04:38:42 |
Message-ID: | OSCPR01MB14966D3955B36CBD8BE58FCBDF5E6A@OSCPR01MB14966.jpnprd01.prod.outlook.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2025-10-01 05:00:59 | Re: [PATCH] Add tests for Bitmapset |
Previous Message | Michael Paquier | 2025-10-01 04:02:25 | Re: Resetting recovery target parameters in pg_createsubscriber |