pgbackrest.git / summary / log / commit / refs
commit aa72c19a831fb0dd15a50a003fbaad427f335353
Author: David Steele <dwsteele@users.noreply.github.com>
Commit: GitHub <noreply@github.com>
Date: Fri Apr 23 16:33:25 2021 +0000
Do not write files atomically or sync paths during backup copy.
There is no need to write the file atomically (e.g. via a temp file on Posix) because checksums are tested on resume after a failed backup. The path does not need be synced for each file because all paths are synced at the end of the backup.
This functionality was not lost during the migration -- it never existed in the Perl code, though these settings are used in restore. See 59f1353 where backupFile() was migrated to C.
doc/xml/release.xml | 17 ++++++++++++++++-
src/command/backup/file.c | 7 +++++--
test/src/module/command/backupTest.c | 8 --------
3 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/doc/xml/release.xml b/doc/xml/release.xml
index de4cb9a1d..34ba227ed 100644
--- a/doc/xml/release.xml
+++ b/doc/xml/release.xml
@@ -40,7 +40,7 @@
</release-item>
</release-bug-list>
- <release-improvement-list>
+ <release-feature-list>
<release-item>
<github-issue id="986"/>
<github-pull-request id="1337"/>
@@ -52,6 +52,21 @@
<p>Add <br-option>db-exclude</br-option> option.</p>
</release-item>
+ </release-feature-list>
+
+ <release-improvement-list>
+ <release-item>
+ <github-issue id="1373"/>
+
+ <release-item-contributor-list>
+ <release-item-contributor id="david.steele"/>
+ <release-item-reviewer id="stephen.frost"/>
+ <release-item-reviewer id="stefan.fercot"/>
+ <release-item-reviewer id="cynthia.shang"/>
+ </release-item-contributor-list>
+
+ <p>Do not write files atomically or sync paths during <cmd>backup</cmd> copy.</p>
+ </release-item>
</release-improvement-list>
<release-development-list>
diff --git a/src/command/backup/file.c b/src/command/backup/file.c
index 1233a2672..9d42d5a0c 100644
--- a/src/command/backup/file.c
+++ b/src/command/backup/file.c
@@ -227,8 +227,11 @@ backupFile(
storageReadIo(read)), cipherBlockNew(cipherModeEncrypt, cipherType, BUFSTR(cipherPass), NULL));
}
- // Setup the repo file for write
- StorageWrite *write = storageNewWriteP(storageRepoWrite(), repoPathFile, .compressible = compressible);
+ // Setup the repo file for write. There is no need to write the file atomically (e.g. via a temp file on Posix) because
+ // checksums are tested on resume after a failed backup. The path does not need to be synced for each file because all
+ // paths are synced at the end of the backup.
+ StorageWrite *write = storageNewWriteP(
+ storageRepoWrite(), repoPathFile, .compressible = compressible, .noAtomic = true, .noSyncPath = true);
ioFilterGroupAdd(ioWriteFilterGroup(storageWriteIo(write)), ioSizeNew());
// Open the source and destination and copy the file
diff --git a/test/src/module/command/backupTest.c b/test/src/module/command/backupTest.c
index f197b18d4..1dca6deaf 100644
--- a/test/src/module/command/backupTest.c
+++ b/test/src/module/command/backupTest.c
@@ -535,11 +535,6 @@ testRun(void)
uint64_t feature = storageRepo()->pub.interface.feature;
((Storage *)storageRepo())->pub.interface.feature = feature & ((1 << storageFeatureCompress) ^ 0xFFFFFFFFFFFFFFFF);
- // Create tmp file to make it look like a prior backup file failed partway through to ensure that retries work
- TEST_RESULT_VOID(
- storagePutP(storageNewWriteP(storageRepoWrite(), strNewFmt("%s.pgbackrest.tmp", strZ(backupPathFile))), NULL),
- " create tmp file");
-
TEST_ASSIGN(
result,
backupFile(
@@ -556,9 +551,6 @@ testRun(void)
storageExistsP(storageRepo(), backupPathFile) && result.pageChecksumResult == NULL),
true, " copy file to repo success");
- TEST_RESULT_BOOL(
- storageExistsP(storageRepoWrite(), strNewFmt("%s.pgbackrest.tmp", strZ(backupPathFile))), false,
- " check temp file removed");
TEST_RESULT_VOID(storageRemoveP(storageRepoWrite(), backupPathFile), " remove repo file");
// -------------------------------------------------------------------------------------------------------------------------
[parent: aaa15b9709cd]