mirror of
https://github.com/postgrespro/pg_probackup.git
synced 2026-06-21 01:34:15 +02:00
[Issue #138] always set "restore_command" in recovery.conf if "--restore-command" option is provided
This commit is contained in:
Grigory Smolkin
+59
-16
@@ -842,7 +842,10 @@ restore_files(void *arg)
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Create recovery.conf with given recovery target parameters */
|
||||
/*
|
||||
* Create recovery.conf (probackup_recovery.conf in case of PG12)
|
||||
* with given recovery target parameters
|
||||
*/
|
||||
static void
|
||||
create_recovery_conf(time_t backup_id,
|
||||
pgRecoveryTarget *rt,
|
||||
@@ -851,9 +854,17 @@ create_recovery_conf(time_t backup_id,
|
||||
{
|
||||
char path[MAXPGPATH];
|
||||
FILE *fp;
|
||||
bool need_restore_conf;
|
||||
bool pitr_requested;
|
||||
bool target_latest;
|
||||
bool target_immediate;
|
||||
bool restore_command_provided = false;
|
||||
char restore_command_guc[16384];
|
||||
|
||||
if (instance_config.restore_command &&
|
||||
(pg_strcasecmp(instance_config.restore_command, "none") != 0))
|
||||
{
|
||||
restore_command_provided = true;
|
||||
}
|
||||
|
||||
/* restore-target='latest' support */
|
||||
target_latest = rt->target_stop != NULL &&
|
||||
@@ -862,16 +873,39 @@ create_recovery_conf(time_t backup_id,
|
||||
target_immediate = rt->target_stop != NULL &&
|
||||
strcmp(rt->target_stop, "immediate") == 0;
|
||||
|
||||
need_restore_conf = !backup->stream || rt->time_string ||
|
||||
/*
|
||||
* Note that setting restore_command alone interpreted
|
||||
* as PITR with target - "until all available WAL is replayed".
|
||||
* We do this because of the following case:
|
||||
* The user is restoring STREAM backup as replica but
|
||||
* also relies on WAL archive to catch-up with master.
|
||||
* If restore_command is provided, then it should be
|
||||
* added to recovery config.
|
||||
* In this scenario, "would be" replica will replay
|
||||
* all WAL segments available in WAL archive, after that
|
||||
* it will try to connect to master via repprotocol.
|
||||
*
|
||||
* The risk is obvious, what if masters current state is
|
||||
* in "the past" relatively to latest state in the archive?
|
||||
* We will get a replica that is "in the future" to the master.
|
||||
* We accept this risk because its probability is low.
|
||||
*/
|
||||
pitr_requested = !backup->stream || rt->time_string ||
|
||||
rt->xid_string || rt->lsn_string || rt->target_name ||
|
||||
target_immediate || target_latest;
|
||||
target_immediate || target_latest || restore_command_provided;
|
||||
|
||||
/* No need to generate recovery.conf at all. */
|
||||
if (!(need_restore_conf || params->restore_as_replica))
|
||||
if (!(pitr_requested || params->restore_as_replica))
|
||||
{
|
||||
/*
|
||||
* Restoring STREAM backup without PITR and not as replica,
|
||||
* recovery.signal and standby.signal for PG12 are not needed
|
||||
*
|
||||
* We do not add "include" option in this case because
|
||||
* here we are creating empty "probackup_recovery.conf"
|
||||
* to handle possible already existing "include"
|
||||
* directive pointing to "probackup_recovery.conf".
|
||||
* If don`t do that, recovery will fail.
|
||||
*/
|
||||
pg12_recovery_config(backup, false);
|
||||
return;
|
||||
@@ -901,13 +935,10 @@ create_recovery_conf(time_t backup_id,
|
||||
#endif
|
||||
|
||||
/* construct restore_command */
|
||||
if (need_restore_conf)
|
||||
if (pitr_requested)
|
||||
{
|
||||
char restore_command_guc[16384];
|
||||
|
||||
/* If restore_command is provided, use it */
|
||||
if (instance_config.restore_command &&
|
||||
(pg_strcasecmp(instance_config.restore_command, "none") != 0))
|
||||
/* If restore_command is provided, use it. Otherwise construct it from scratch. */
|
||||
if (restore_command_provided)
|
||||
sprintf(restore_command_guc, "%s", instance_config.restore_command);
|
||||
else
|
||||
{
|
||||
@@ -937,10 +968,6 @@ create_recovery_conf(time_t backup_id,
|
||||
}
|
||||
}
|
||||
|
||||
elog(LOG, "Setting restore_command to '%s'", restore_command_guc);
|
||||
fio_fprintf(fp, "restore_command = '%s'\n",
|
||||
restore_command_guc);
|
||||
|
||||
/*
|
||||
* We've already checked that only one of the four following mutually
|
||||
* exclusive options is specified, so the order of calls is insignificant.
|
||||
@@ -996,13 +1023,29 @@ create_recovery_conf(time_t backup_id,
|
||||
fio_fprintf(fp, "primary_conninfo = '%s'\n", backup->primary_conninfo);
|
||||
}
|
||||
|
||||
if (pitr_requested)
|
||||
{
|
||||
elog(LOG, "Setting restore_command to '%s'", restore_command_guc);
|
||||
fio_fprintf(fp, "restore_command = '%s'\n", restore_command_guc);
|
||||
}
|
||||
|
||||
if (fio_fflush(fp) != 0 ||
|
||||
fio_fclose(fp))
|
||||
elog(ERROR, "cannot write file \"%s\": %s", path,
|
||||
strerror(errno));
|
||||
|
||||
#if PG_VERSION_NUM >= 120000
|
||||
if (need_restore_conf)
|
||||
/*
|
||||
* Create "recovery.signal" to mark this recovery as PITR for PostgreSQL.
|
||||
* In older versions presense of recovery.conf alone was enough.
|
||||
* To keep behaviour consistent with older versions,
|
||||
* we are forced to create "recovery.signal"
|
||||
* even when only restore_command is provided.
|
||||
* Presense of "recovery.signal" by itself determine only
|
||||
* one thing: do PostgreSQL must switch to a new timeline
|
||||
* after successfull recovery or not?
|
||||
*/
|
||||
if (pitr_requested)
|
||||
{
|
||||
elog(LOG, "creating recovery.signal file");
|
||||
snprintf(path, lengthof(path), "%s/recovery.signal", instance_config.pgdata);
|
||||
|
||||
+26
-20
@@ -3247,8 +3247,8 @@ class RestoreTest(ProbackupTest, unittest.TestCase):
|
||||
# Clean after yourself
|
||||
self.del_test_dir(module_name, fname)
|
||||
|
||||
@unittest.skip("skip")
|
||||
def test_stream_restore_command_options(self):
|
||||
# @unittest.skip("skip")
|
||||
def test_stream_restore_command_option(self):
|
||||
"""
|
||||
correct handling of restore command options
|
||||
when restoring STREAM backup
|
||||
@@ -3291,40 +3291,46 @@ class RestoreTest(ProbackupTest, unittest.TestCase):
|
||||
|
||||
node.pgbench_init(scale=1)
|
||||
|
||||
node.safe_psql(
|
||||
'postgres',
|
||||
'CHECKPOINT')
|
||||
|
||||
node.safe_psql(
|
||||
'postgres',
|
||||
'create table t1()')
|
||||
|
||||
# restore backup
|
||||
node.cleanup()
|
||||
shutil.rmtree(os.path.join(node.logs_dir))
|
||||
|
||||
# self.restore_node(backup_dir, 'node', node)
|
||||
restore_cmd = self.get_restore_command(backup_dir, 'node', node)
|
||||
|
||||
self.restore_node(
|
||||
backup_dir, 'node', node,
|
||||
options=[
|
||||
'--restore-command={0}'.format(restore_cmd),
|
||||
'--recovery-target-action=pause',
|
||||
'--recovery-target=latest'])
|
||||
'--restore-command={0}'.format(restore_cmd)])
|
||||
|
||||
self.assertTrue(
|
||||
os.path.isfile(recovery_conf),
|
||||
"File {0} do not exists".format(recovery_conf))
|
||||
"File '{0}' do not exists".format(recovery_conf))
|
||||
|
||||
if self.get_version(node) >= self.version_to_num('12.0'):
|
||||
recovery_signal = os.path.join(node.data_dir, 'recovery.signal')
|
||||
self.assertTrue(
|
||||
os.path.isfile(recovery_signal),
|
||||
"File '{0}' do not exists".format(recovery_signal))
|
||||
|
||||
node.slow_start()
|
||||
|
||||
exit(1)
|
||||
node.safe_psql(
|
||||
'postgres',
|
||||
'select * from t1')
|
||||
|
||||
# self.set_config(
|
||||
# backup_dir ,'node',
|
||||
# options=['--restore-command="cp {0}/%f %p"'.format(wal_dir)])
|
||||
#
|
||||
# # restore delta backup
|
||||
# node.cleanup()
|
||||
# self.restore_node(
|
||||
# backup_dir, 'node', node, options=['--recovery-target=immediate'])
|
||||
#
|
||||
# self.assertTrue(
|
||||
# os.path.isfile(recovery_conf),
|
||||
# "File {0} do not exists".format(recovery_conf))
|
||||
timeline_id = node.safe_psql(
|
||||
'postgres',
|
||||
'select timeline_id from pg_control_checkpoint()').rstrip()
|
||||
|
||||
self.assertEqual('2', timeline_id)
|
||||
|
||||
# Clean after yourself
|
||||
self.del_test_dir(module_name, fname)
|
||||
Reference in New Issue
Block a user