On 9/28/23 19:59, Michael Paquier wrote:
On Thu, Sep 28, 2023 at 04:23:42PM -0400, David Steele wrote:
So overall, +1 for Michael's patch, though I have only read through it and
not tested it yet.
Reviews, thoughts and opinions are welcome.
OK, I have now reviewed and tested the patch and it looks good to me. I
stopped short of marking this RfC since there are other reviewers in the
mix.
I dislike that we need to repeat:
OwnLatch(&XLogRecoveryCtl->recoveryWakeupLatch);
But I see the logic behind why you did it and there's no better way to
do it as far as I can see.
One comment, though, if we are going to require recovery.signal when
backup_label is present, should it just be implied? Why error and force the
user to create it?
That's one thing I was considering, but I also cannot convince myself
that this is the best option because the presence of recovery.signal
or standby.standby (if both, standby.signal takes priority) makes it
clear what type of recovery is wanted at disk level. I'd be OK if
folks think that this is a sensible consensus, as well, even if I
don't really agree with it.
I'm OK with keeping it as required for now.
Another idea I had was to force the creation of recovery.signal by
pg_basebackup even if -R is not used. All the reports we've seen with
people getting confused came from pg_basebackup that enforces no
configuration.
This change makes it more obvious if configuration is missing (since
you'll get an error), however +1 for adding this to pg_basebackup.
A last thing, that had better be covered in a separate thread and
patch, is about validateRecoveryParameters(). These days, I'd like to
think that it may be OK to lift at least the restriction on
restore_command being required if we are doing recovery to ease the
case of self-contained backups (aka the case where all the WAL needed
to reach a consistent point is in pg_wal/ or its tarball)
Hmmm, I'm not sure about this. I'd prefer users set
restore_command=/bin/false explicitly to fetch WAL from pg_wal by
default if that's what they really intend.
Regards,
-David
