keep snapshot on log delete fail (#391)

- Improvement: Cleaned up snapshot writing for CompactCAS/FileCas/Cache/Project stores
- Improvement: Safer recovery when failing to delete log for CompactCAS/FileCas/Cache/Project stores
- Improvement: Added log file reset when writing snapshot at startup for FileCas

1 files changed, 31 insertions, 47 deletions

diff --git a/src/zenstore/filecas.cpp b/src/zenstore/filecas.cpp
index 14bdc41f0..4911bffb9 100644
--- a/src/zenstore/filecas.cpp
+++ b/src/zenstore/filecas.cpp
@@ -251,7 +251,7 @@ FileCasStrategy::Initialize(const std::filesystem::path& RootDirectory, bool IsN
 
 	if (IsNewStore || LogEntryCount > 0)
 	{
-		MakeIndexSnapshot();
+		MakeIndexSnapshot(/*ResetLog*/ true);
 	}
 }
 
@@ -727,7 +727,7 @@ FileCasStrategy::Flush()
 	ZEN_TRACE_CPU("FileCas::Flush");
 
 	m_CasLog.Flush();
-	MakeIndexSnapshot();
+	MakeIndexSnapshot(/*ResetLog*/ false);
 }
 
 void
@@ -912,15 +912,14 @@ FileCasStrategy::ValidateEntry(const FileCasIndexEntry& Entry, std::string& OutR
 }
 
 void
-FileCasStrategy::MakeIndexSnapshot()
+FileCasStrategy::MakeIndexSnapshot(bool ResetLog)
 {
 	ZEN_MEMSCOPE(GetFileCasTag());
 	ZEN_TRACE_CPU("FileCas::MakeIndexSnapshot");
 
 	using namespace filecas::impl;
 
-	uint64_t LogCount = m_CasLog.GetLogCount();
-	if (m_LogFlushPosition == LogCount)
+	if (m_LogFlushPosition == m_CasLog.GetLogCount())
 	{
 		return;
 	}
@@ -937,34 +936,20 @@ FileCasStrategy::MakeIndexSnapshot()
 
 	namespace fs = std::filesystem;
 
-	fs::path IndexPath	   = GetIndexPath(m_RootDirectory);
-	fs::path STmpIndexPath = GetTempIndexPath(m_RootDirectory);
-
-	// Move index away, we keep it if something goes wrong
-	if (IsFile(STmpIndexPath))
-	{
-		std::error_code Ec;
-		if (!RemoveFile(STmpIndexPath, Ec) || Ec)
-		{
-			ZEN_WARN("snapshot failed to clean up temp snapshot at {}, reason: '{}'", STmpIndexPath, Ec.message());
-			return;
-		}
-	}
+	const fs::path IndexPath = GetIndexPath(m_RootDirectory);
 
 	try
 	{
-		if (IsFile(IndexPath))
-		{
-			RenameFile(IndexPath, STmpIndexPath);
-		}
-
 		// Write the current state of the location map to a new index state
 		std::vector<FileCasIndexEntry> Entries;
 		uint64_t					   IndexLogPosition = 0;
 
 		{
 			RwLock::SharedLockScope __(m_Lock);
-			IndexLogPosition = m_CasLog.GetLogCount();
+			if (!ResetLog)
+			{
+				IndexLogPosition = m_CasLog.GetLogCount();
+			}
 			Entries.resize(m_Index.size());
 
 			uint64_t EntryIndex = 0;
@@ -974,6 +959,7 @@ FileCasStrategy::MakeIndexSnapshot()
 				IndexEntry.Key				  = Entry.first;
 				IndexEntry.Size				  = Entry.second.Size;
 			}
+			EntryCount = m_Index.size();
 		}
 
 		TemporaryFile	ObjectIndexFile;
@@ -983,47 +969,45 @@ FileCasStrategy::MakeIndexSnapshot()
 		{
 			throw std::system_error(Ec, fmt::format("Failed to create temp file for index snapshot at '{}'", IndexPath));
 		}
-		filecas::impl::FileCasIndexHeader Header = {.EntryCount = Entries.size(), .LogPosition = IndexLogPosition};
+		filecas::impl::FileCasIndexHeader Header = {.EntryCount = EntryCount, .LogPosition = IndexLogPosition};
 
 		Header.Checksum = filecas::impl::FileCasIndexHeader::ComputeChecksum(Header);
 
 		ObjectIndexFile.Write(&Header, sizeof(filecas::impl::FileCasIndexHeader), 0);
-		ObjectIndexFile.Write(Entries.data(), Entries.size() * sizeof(FileCasIndexEntry), sizeof(filecas::impl::FileCasIndexHeader));
+		ObjectIndexFile.Write(Entries.data(), EntryCount * sizeof(FileCasIndexEntry), sizeof(filecas::impl::FileCasIndexHeader));
 		ObjectIndexFile.Flush();
 		ObjectIndexFile.MoveTemporaryIntoPlace(IndexPath, Ec);
 		if (Ec)
 		{
-			throw std::system_error(Ec, fmt::format("Failed to move temp file '{}' to '{}'", ObjectIndexFile.GetPath(), IndexPath));
+			throw std::system_error(Ec,
+									fmt::format("Snapshot failed to rename new snapshot '{}' to '{}', reason: '{}'",
+												ObjectIndexFile.GetPath(),
+												IndexPath,
+												Ec.message()));
 		}
-		EntryCount		   = Entries.size();
-		m_LogFlushPosition = IndexLogPosition;
-	}
-	catch (const std::exception& Err)
-	{
-		ZEN_WARN("snapshot FAILED, reason: '{}'", Err.what());
-
-		// Restore any previous snapshot
 
-		if (IsFile(STmpIndexPath))
+		if (ResetLog)
 		{
-			std::error_code Ec;
-			RemoveFile(IndexPath, Ec);	// We don't care if this fails, we try to move the old temp file regardless
-			RenameFile(STmpIndexPath, IndexPath, Ec);
-			if (Ec)
+			const std::filesystem::path LogPath = GetLogPath(m_RootDirectory);
+
+			if (IsFile(LogPath))
 			{
-				ZEN_WARN("snapshot failed to restore old snapshot from {}, reason: '{}'", STmpIndexPath, Ec.message());
+				if (!RemoveFile(LogPath, Ec) || Ec)
+				{
+					// This is non-critical, it only means that we will replay the events of the log over the snapshot - inefficent but in
+					// the end it will be the same result
+					ZEN_WARN("Snapshot failed to clean log file '{}', reason: '{}'", LogPath, IndexPath, Ec.message());
+				}
 			}
 		}
+		m_LogFlushPosition = IndexLogPosition;
 	}
-	if (IsFile(STmpIndexPath))
+	catch (const std::exception& Err)
 	{
-		std::error_code Ec;
-		if (!RemoveFile(STmpIndexPath, Ec) || Ec)
-		{
-			ZEN_WARN("snapshot failed to remove temporary file {}, reason: '{}'", STmpIndexPath, Ec.message());
-		}
+		ZEN_WARN("snapshot FAILED, reason: '{}'", Err.what());
 	}
 }
+
 uint64_t
 FileCasStrategy::ReadIndexFile(const std::filesystem::path& IndexPath, uint32_t& OutVersion)
 {