RE: Resetting recovery target parameters in pg_createsubscriber

Поиск
Список
Период
Сортировка
От Hayato Kuroda (Fujitsu)
Тема RE: Resetting recovery target parameters in pg_createsubscriber
Дата
Msg-id OSCPR01MB14966D3955B36CBD8BE58FCBDF5E6A@OSCPR01MB14966.jpnprd01.prod.outlook.com
обсуждение исходный текст
Ответ на Re: Resetting recovery target parameters in pg_createsubscriber  (Alena Vinter <dlaaren8@gmail.com>)
Список 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


В списке pgsql-hackers по дате отправления: