Heikki Linnakangas <heikki@enterprisedb.com> writes:
> Simon Riggs wrote:
>> +
>> + if (startupAfterRecovery)
>> + ereport(ERROR,
>> + (errmsg("recovery ends normally with startup_after_recovery=false")));
>> +
> I find this part of the patch a bit ugly. Isn't there a better way to
> exit than throwing an error that's not really an error?
This patch has obviously been thrown together with no thought and even
less testing. It breaks the normal case (I think the above if-test is
backwards), and I don't believe that it works for the advertised purpose
either (because nothing gets done to force a checkpoint before aborting,
thus the files on disk are not up to date with the end of WAL).
Also, I'm not sold that the concept is even useful. Apparently the idea
is to offload the expense of taking periodic base backups from a master
server, by instead backing up a PITR slave's fileset --- which is fine.
But why in the world would you want to stop the slave to do it? ISTM
we would want to arrange things so that you can copy the slave's files
while it continues replicating, just as with a standard base backup.
regards, tom lane