Preserve partial files during block incremental delta restore.

Previously files that were smaller than the expected size were not preserved for block incremental, even though it is possible that block incremental could make use of a partial file.

One example is when a restore encounters an error. On retry the partial file can be used as a starting point rather than copying again from the beginning. Another example is restoring a backup where a file is larger than what already exists in the data directory.

Preserve any size file when block incremental will be used for the delta in order to reuse partial files when possible. If the file is smaller than expected then disable the whole-file checksum to reduce overhead.

doc/xml/release/2024/2.50.xml

@@ -15,6 +15,17 @@
         </release-bug-list>
 
         <release-improvement-list>
+            <release-item>
+                <github-pull-request id="2254"/>
+
+                <release-item-contributor-list>
+                    <release-item-contributor id="david.steele"/>
+                    <release-item-reviewer id="stephen.frost"/>
+                </release-item-contributor-list>
+
+                <p>Preserve partial files during block incremental delta restore.</p>
+            </release-item>
+
             <release-item>
                 <github-pull-request id="2197"/>
 

src/command/restore/file.c

@@ -89,7 +89,7 @@ restoreFile(
                         else
                         {
                             // Only continue delta if the file size is as expected or larger
-                            if (info.size >= file->size)
+                            if (info.size >= file->size || file->blockIncrMapSize != 0)
                             {
                                 const char *const fileName = strZ(storagePathP(storagePg(), file->name));
 
@@ -115,6 +115,12 @@ restoreFile(
 
                                     // Update info
                                     info = storageInfoP(storagePg(), file->name, .followLink = true);
+
+                                    // For block incremental it is very important that the file be exactly the expected size, so
+                                    // make sure the truncate worked as expected
+                                    CHECK_FMT(
+                                        FileWriteError, info.size == file->size, "unable to truncate '%s' to %" PRIu64 " bytes",
+                                        strZ(file->name), file->size);
                                 }
 
                                 // Generate checksum for the file if size is not zero
@@ -123,7 +129,10 @@ restoreFile(
                                 if (file->size != 0)
                                 {
                                     read = storageReadIo(storageNewReadP(storagePg(), file->name));
-                                    ioFilterGroupAdd(ioReadFilterGroup(read), cryptoHashNew(hashTypeSha1));
+
+                                    // Calculate checksum only when size matches
+                                    if (info.size == file->size)
+                                        ioFilterGroupAdd(ioReadFilterGroup(read), cryptoHashNew(hashTypeSha1));
 
                                     // Generate block checksum list if block incremental
                                     if (file->blockIncrMapSize != 0)
@@ -136,11 +145,12 @@ restoreFile(
                                     ioReadDrain(read);
                                 }
 
-                                // If the checksum is the same (or file is zero size) then no need to copy the file
+                                // If size/checksum is the same (or file is zero size) then no need to copy the file
                                 if (file->size == 0 ||
-                                    bufEq(
+                                    (info.size == file->size &&
-                                        file->checksum,
+                                     bufEq(
-                                        pckReadBinP(ioFilterGroupResultP(ioReadFilterGroup(read), CRYPTO_HASH_FILTER_TYPE))))
+                                         file->checksum,
+                                         pckReadBinP(ioFilterGroupResultP(ioReadFilterGroup(read), CRYPTO_HASH_FILTER_TYPE)))))
                                 {
                                     // If the checksum/size are now the same but the time is not, then set the time back to the
                                     // backup time. This helps with unit testing, but also presents a pristine version of the

test/src/module/command/restoreTest.c

@@ -3081,8 +3081,9 @@ testRun(void)
         const String *pgPath = STRDEF(TEST_PATH "/pg");
         const String *repoPath = STRDEF(TEST_PATH "/repo");
 
-        // Created pg_control
+        // Created pg_control and PG_VERSION
         HRN_PG_CONTROL_PUT(storagePgWrite(), PG_VERSION_15, .pageChecksum = false);
+        HRN_STORAGE_PUT_Z(storagePgWrite(), PG_FILE_PGVERSION, PG_VERSION_15_Z);
 
         // Create encrypted stanza
         StringList *argList = strLstNew();
@@ -3172,6 +3173,7 @@ testRun(void)
 
         TEST_STORAGE_LIST(
             storagePg(), NULL,
+            "PG_VERSION\n"
             "base/\n"
             "base/1/\n"
             "base/1/2\n"
@@ -3180,6 +3182,46 @@ testRun(void)
             "global/pg_control\n"
             "postgresql.auto.conf\n",
             .level = storageInfoLevelType);
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("delta restore with block incr");
+
+        // Use detail log level to catch block incremental restore message
+        harnessLogLevelSet(logLevelDetail);
+
+        // Shrink file to make sure block incremental delta will reuse it
+        relation = bufNew(128 * 1024);
+        memset(bufPtr(relation), 0, bufSize(relation));
+        bufUsedSet(relation, bufSize(relation));
+
+        HRN_STORAGE_PUT(storagePgWrite(), PG_PATH_BASE "/1/2", relation);
+
+        argList = strLstNew();
+        hrnCfgArgRawZ(argList, cfgOptStanza, "test1");
+        hrnCfgArgRaw(argList, cfgOptRepoPath, repoPath);
+        hrnCfgArgRaw(argList, cfgOptPgPath, pgPath);
+        hrnCfgArgRawZ(argList, cfgOptSpoolPath, TEST_PATH "/spool");
+        hrnCfgArgRawBool(argList, cfgOptDelta, true);
+        hrnCfgArgRawZ(argList, cfgOptRepoCipherType, "aes-256-cbc");
+        hrnCfgEnvRawZ(cfgOptRepoCipherPass, TEST_CIPHER_PASS);
+        HRN_CFG_LOAD(cfgCmdRestore, argList);
+
+        TEST_RESULT_VOID(cmdRestore(), "restore");
+
+        TEST_STORAGE_LIST(
+            storagePg(), NULL,
+            "PG_VERSION\n"
+            "base/\n"
+            "base/1/\n"
+            "base/1/2\n"
+            "base/1/3\n"
+            "global/\n"
+            "global/pg_control\n"
+            "postgresql.auto.conf\n",
+            .level = storageInfoLevelType);
+
+        // Check that file was restored to full size with a partial write
+        TEST_RESULT_LOG_EMPTY_OR_CONTAINS(", bi 128KB/256KB, ");
     }
 
     FUNCTION_HARNESS_RETURN_VOID();