From 684832aa44baf807c1ee74ec6072e5259c19c16d Mon Sep 17 00:00:00 2001 From: neethuhaneesha Date: Mon, 16 Feb 2026 11:17:32 -0800 Subject: [PATCH] Fix for minRestorableVersion and maxRestorableVersion not updating correctly in case of missing mutation logs. (#12710) --- fdbclient/BackupContainerFileSystem.actor.cpp | 30 +++++++++---------- ...kupAndParallelRestoreCorrectness.actor.cpp | 15 ++++------ 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/fdbclient/BackupContainerFileSystem.actor.cpp b/fdbclient/BackupContainerFileSystem.actor.cpp index 0ec137223f..44b135d570 100644 --- a/fdbclient/BackupContainerFileSystem.actor.cpp +++ b/fdbclient/BackupContainerFileSystem.actor.cpp @@ -716,24 +716,26 @@ public: desc.snapshotBytes += s.totalSize; - // If the snapshot is at a single version then it requires no logs. Update min and max restorable. - // TODO: Somehow check / report if the restorable range is not or may not be contiguous. - if (s.beginVersion == s.endVersion && - (!desc.contiguousLogEnd.present() || // no logs - (desc.contiguousLogEnd.present() && - desc.contiguousLogEnd.get() >= s.beginVersion)) // have logs, then should cover snapshot - ) { - if (!desc.minRestorableVersion.present() || s.endVersion < desc.minRestorableVersion.get()) - desc.minRestorableVersion = s.endVersion; - - if (!desc.maxRestorableVersion.present() || s.endVersion > desc.maxRestorableVersion.get()) - desc.maxRestorableVersion = s.endVersion; + // If the snapshot is at a single version and then it requires no logs. Update min and max restorable. + // Update only if minRestorableVersion and maxRestorableVersion are not set. If they are set, we should + // check for log continuity between current minRestorableVersion to s.endVersion which happens in the + // next if block. + if (s.beginVersion == s.endVersion && !desc.minRestorableVersion.present() && + !desc.maxRestorableVersion.present()) { + desc.minRestorableVersion = s.endVersion; + desc.maxRestorableVersion = s.endVersion; } // If the snapshot is covered by the contiguous log chain then update min/max restorable. if (desc.minLogBegin.present() && s.beginVersion >= desc.minLogBegin.get() && s.endVersion < desc.contiguousLogEnd.get()) { - if (!desc.minRestorableVersion.present() || s.endVersion < desc.minRestorableVersion.get()) + // If minRestorableVersion not present, update minRestorableVersion to snapshot endVersion. + // If minRestorableVersion present and if it has continuous logs from minRestorableVersion + // to snapshot endVersion, don't update the minRestorableVersion. + // Else, means it has no continous logs, so update minRestorableVersion to s.endVersion. + if (!desc.minRestorableVersion.present() || + !(desc.minRestorableVersion.get() >= desc.minLogBegin.get() && + desc.minRestorableVersion.get() < desc.contiguousLogEnd.get())) desc.minRestorableVersion = s.endVersion; if (!desc.maxRestorableVersion.present() || @@ -749,8 +751,6 @@ public: (desc.contiguousLogEnd.get() == s.beginVersion && s.beginVersion != s.endVersion)) && s.restorable.get()) { if (desc.minRestorableVersion.present() && desc.maxRestorableVersion.present()) { - ASSERT(desc.minRestorableVersion.get() < s.beginVersion); - // check if we have contiguous logs from minRestorableVersion to current snapshot endVersion bool contiguousLogs = false; if (desc.partitioned) diff --git a/fdbserver/workloads/BackupAndParallelRestoreCorrectness.actor.cpp b/fdbserver/workloads/BackupAndParallelRestoreCorrectness.actor.cpp index 690a062aac..ff0cf33f07 100644 --- a/fdbserver/workloads/BackupAndParallelRestoreCorrectness.actor.cpp +++ b/fdbserver/workloads/BackupAndParallelRestoreCorrectness.actor.cpp @@ -561,16 +561,11 @@ struct BackupAndParallelRestoreCorrectnessWorkload : TestWorkload { targetVersion = desc.minRestorableVersion.get(); } else if (deterministicRandom()->random01() < 0.1) { targetVersion = desc.maxRestorableVersion.get(); - } else if (deterministicRandom()->random01() < 0.5 && - desc.minRestorableVersion.get() < desc.contiguousLogEnd.get()) { - // The assertion may fail because minRestorableVersion may be decided by snapshot version. - // ASSERT_WE_THINK(desc.minRestorableVersion.get() <= desc.contiguousLogEnd.get()); - // This assertion can fail when contiguousLogEnd < maxRestorableVersion and - // the snapshot version > contiguousLogEnd. I.e., there is a gap between - // contiguousLogEnd and snapshot version. - // ASSERT_WE_THINK(desc.contiguousLogEnd.get() > desc.maxRestorableVersion.get()); - targetVersion = deterministicRandom()->randomInt64(desc.minRestorableVersion.get(), - desc.contiguousLogEnd.get()); + } else if (deterministicRandom()->random01() < 0.5) { + targetVersion = (desc.minRestorableVersion.get() != desc.maxRestorableVersion.get()) + ? deterministicRandom()->randomInt64(desc.minRestorableVersion.get(), + desc.maxRestorableVersion.get()) + : desc.maxRestorableVersion.get(); } }