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.
