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 по дате отправления: