db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-12 10:04:14 +02:00
Code Issues Releases Activity

Skip writing recovery.signal by default for restores of offline backups.

Browse Source 
When restoring an offline backup on PostgreSQL >= 12, skip writing recovery.signal by default since this will error if the backup was made with wal_level=minimal. If the user explicitly sets the type option to something other than none, then write recovery.signal as usual since it is possible to do Point-In-Time-Recovery from an offline backup as long as wal_level was not minimal.
This commit is contained in:
David Steele 2023-03-08 19:05:23 +07:00 committed by GitHub
parent 7e5adc0359
commit 2fa7e53c5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 65 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
doc/xml/release.xml
View File

@ -16,6 +16,24 @@
<release-list>
<release date="XXXX-XX-XX" version="2.45dev" title="UNDER DEVELOPMENT">
<release-core-list>
<release-bug-list>
<release-item>
<commit subject="Set online flag in manifest in command/restore unit tests."/>
<commit subject="Skip writing recovery.signal by default for restores of offline backups.">
<github-issue id="2018"/>
<github-pull-request id="2019"/>
</commit>
<release-item-contributor-list>
<release-item-ideator id="marcel.borger"/>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="stefan.fercot"/>
</release-item-contributor-list>
<p>Skip writing <file>recovery.signal</file> by default for restores of offline backups.</p>
</release-item>
</release-bug-list>
<release-feature-list>
<release-item>
<commit subject="Add block incremental to real/all test output."/>

2
src/build/help/help.xml
View File

@ -2159,6 +2159,8 @@
<list-item><id>standby</id> - add <setting>standby_mode=on</setting> to <file>recovery.conf</file> file so cluster will start in standby mode.</list-item>
<list-item><id>none</id> - no <file>recovery.conf</file> file is written so <postgres/> will attempt to achieve consistency using WAL segments present in <path>pg_xlog</path>/<path>pg_wal</path>. Provide the required WAL segments or use the <setting>archive-copy</setting> setting to include them with the backup.</list-item>
</list>
<p>Note that the default restore <br-option>type</br-option> for offline backups is <id>none</id> since Point-in-Time-Recovery is not possible if <setting>wal_level=minimal</setting>. If <br-option>type</br-option> is set explicitly then it will be honored since Point-in-Time-Recovery is possible from offline backups as long as <setting>wal_level > minimal</setting>.</p>
</text>
<example>xid</example>

17
src/command/restore/restore.c
View File

@ -1765,9 +1765,10 @@ restoreRecoveryWriteConf(const Manifest *const manifest, const unsigned int pgVe
// Helper to write recovery options into postgresql.auto.conf
static void
restoreRecoveryWriteAutoConf(unsigned int pgVersion, const String *restoreLabel)
restoreRecoveryWriteAutoConf(const Manifest *const manifest, const unsigned int pgVersion, const String *const restoreLabel)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(MANIFEST, manifest);
FUNCTION_LOG_PARAM(UINT, pgVersion);
FUNCTION_LOG_PARAM(STRING, restoreLabel);
FUNCTION_LOG_END();
@ -1879,8 +1880,9 @@ restoreRecoveryWriteAutoConf(unsigned int pgVersion, const String *restoreLabel)
.noAtomic = true, .noSyncPath = true, .user = dataPath.user, .group = dataPath.group),
NULL);
}
// Else the recovery.signal file is required for targeted recovery
else
// Else the recovery.signal file is required for targeted recovery. Skip writing this file if the backup was offline and
// recovery type is none since PostgreSQL will error in this case when wal_level=minimal.
else if (cfgOptionStrId(cfgOptType) != CFGOPTVAL_TYPE_NONE || manifestData(manifest)->backupOptionOnline)
{
storagePutP(
storageNewWriteP(
@ -1933,7 +1935,7 @@ restoreRecoveryWrite(const Manifest *manifest)
// Write recovery file based on PostgreSQL version
if (pgVersion >= PG_VERSION_RECOVERY_GUC)
restoreRecoveryWriteAutoConf(pgVersion, restoreLabel);
restoreRecoveryWriteAutoConf(manifest, pgVersion, restoreLabel);
else
restoreRecoveryWriteConf(manifest, pgVersion, restoreLabel);
}
@ -2453,6 +2455,13 @@ cmdRestore(void)
// Remotes (if any) are no longer needed since the rest of the repository reads will be done by the local processes
protocolFree();
// If the backup was made offline and no restore type was specified then set to none. This prevents recovery.signal from
// being generated by default for PostgreSQL >= 12. Offline backups created with wal_level=minimal will error in this case
// so this should be a good default. However, if the user explicitly sets type and it does not equal none then they will get
// an error if wal_level=minimal.
if (!manifestData(jobData.manifest)->backupOptionOnline && cfgOptionSource(cfgOptType) == cfgSourceDefault)
cfgOptionSet(cfgOptType, cfgSourceParam, VARUINT64(CFGOPTVAL_TYPE_NONE));
// Validate manifest. Don't use strict mode because we'd rather ignore problems that won't affect a restore.
manifestValidate(jobData.manifest, false);

38
test/src/module/command/restoreTest.c
View File

@ -1872,7 +1872,7 @@ testRun(void)
HRN_CFG_LOAD(cfgCmdRestore, argList);
TEST_ERROR(
restoreRecoveryWriteAutoConf(PG_VERSION_12, restoreLabel), OptionInvalidError,
restoreRecoveryWriteAutoConf(manifest, PG_VERSION_12, restoreLabel), OptionInvalidError,
"'standby_mode' setting is not valid for PostgreSQL >= 12\n"
"HINT: use --type=standby instead of --recovery-option=standby_mode=on.");
@ -1888,7 +1888,7 @@ testRun(void)
hrnCfgArgRawZ(argList, cfgOptType, "none");
HRN_CFG_LOAD(cfgCmdRestore, argList);
restoreRecoveryWriteAutoConf(PG_VERSION_12, restoreLabel);
restoreRecoveryWriteAutoConf(manifest, PG_VERSION_12, restoreLabel);
TEST_STORAGE_GET_EMPTY(storagePg(), PG_FILE_POSTGRESQLAUTOCONF, .comment = "check postgresql.auto.conf");
TEST_STORAGE_LIST(
@ -1911,7 +1911,7 @@ testRun(void)
"# DO NOT MODIFY\n"
"\t recovery_target_action='promote'\n\n");
restoreRecoveryWriteAutoConf(PG_VERSION_12, restoreLabel);
restoreRecoveryWriteAutoConf(manifest, PG_VERSION_12, restoreLabel);
TEST_STORAGE_GET(
storagePg(), PG_FILE_POSTGRESQLAUTOCONF,
@ -1926,6 +1926,33 @@ testRun(void)
TEST_RESULT_LOG("P00 INFO: write updated " TEST_PATH "/pg/postgresql.auto.conf");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("PG12 restore type none and offline");
manifest->pub.data.backupOptionOnline = false;
HRN_SYSTEM_FMT("rm -rf %s/*", strZ(pgPath));
HRN_STORAGE_PUT_Z(
storagePgWrite(), PG_FILE_POSTGRESQLAUTOCONF,
"# DO NOT MODIFY\n"
"\t recovery_target_action='promote'\n\n");
restoreRecoveryWriteAutoConf(manifest, PG_VERSION_12, restoreLabel);
TEST_STORAGE_GET(
storagePg(), PG_FILE_POSTGRESQLAUTOCONF,
"# DO NOT MODIFY\n"
RECOVERY_SETTING_PREFIX "\t recovery_target_action='promote'\n\n",
.comment = "check postgresql.auto.conf");
TEST_STORAGE_LIST(
storagePg(), NULL,
PG_FILE_POSTGRESQLAUTOCONF "\n",
.comment = "recovery.signal missing, standby.signal missing");
TEST_RESULT_LOG("P00 INFO: write updated " TEST_PATH "/pg/postgresql.auto.conf");
manifest->pub.data.backupOptionOnline = true;
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("PG12 restore type standby and remove existing recovery settings");
@ -1945,7 +1972,7 @@ testRun(void)
hrnCfgArgRawZ(argList, cfgOptRecoveryOption, "restore-command=my_restore_command");
HRN_CFG_LOAD(cfgCmdRestore, argList);
restoreRecoveryWriteAutoConf(PG_VERSION_12, restoreLabel);
restoreRecoveryWriteAutoConf(manifest, PG_VERSION_12, restoreLabel);
TEST_STORAGE_GET(
storagePg(), PG_FILE_POSTGRESQLAUTOCONF,
@ -3124,8 +3151,7 @@ testRun(void)
"base/1/2\n"
"global/\n"
"global/pg_control\n"
"postgresql.auto.conf\n"
"recovery.signal\n",
"postgresql.auto.conf\n",
.level = storageInfoLevelType);
}