RE: Resetting recovery target parameters in pg_createsubscriber

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.