RE: Resetting recovery target parameters in pg_createsubscriber

Ilyasov Ian <ianilyasov@outlook.com>

From: Ilyasov Ian <ianilyasov@outlook.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "dlaaren8@gmail.com" <dlaaren8@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-10-05T22:30:53Z
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

Hello Alena!

I am new in reviewing here and tried to review your patch:

I agree with the review of Michael considering
+char		data[1024];

This looks unsafe.

+static bool recovery_params_set = false;
+static bool recovery_params_reset = false;

Using two booleans here looks wrong to me.
Maybe one is enough with refactored logic in
cleanup_objects_atexit()?

+pg_log_warning_hint("Manual removal of recovery parameters is required from 'postgresql.auto.conf' (PostgreSQL %d+) or 'recovery.conf' (older versions)",

Do we need info about recovery.conf here since patch applies only to master?

Also I am not sure what scenario we are protecting against.
I set up logical replication via pg_createsubscriber first and did this:
./bin/pg_ctl -D standby -l standby stop -m fast
touch standby/recovery.signal
./bin/pg_ctl -D standby -l standby start
with restore_command = 'cp /home/postgresql-install/wal_archive/%f "%p"'

With no patch I got:
LOG:  invalid record length at 0/A0000A0: expected at least 24, got 0
LOG:  redo is not required
FATAL:  recovery ended before configured recovery target was reached

But with patches applied I successfully started the standby.

Did I get the idea right?

Kind regards,
Ian Ilyasov.