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, 22 insertions, 24 deletions

diff --git a/src/zenstore/compactcas.cpp b/src/zenstore/compactcas.cpp
index a8914ed20..8cf241e34 100644
--- a/src/zenstore/compactcas.cpp
+++ b/src/zenstore/compactcas.cpp
@@ -461,7 +461,7 @@ CasContainerStrategy::Flush()
 	ZEN_TRACE_CPU("CasContainer::Flush");
 	m_BlockStore.Flush(/*ForceNewBlock*/ false);
 	m_CasLog.Flush();
-	MakeIndexSnapshot(/*FlushLockPosition*/ false);
+	MakeIndexSnapshot(/*ResetLog*/ false);
 }
 
 void
@@ -924,13 +924,12 @@ CasContainerStrategy::StorageSize() const
 }
 
 void
-CasContainerStrategy::MakeIndexSnapshot(bool FlushLockPosition)
+CasContainerStrategy::MakeIndexSnapshot(bool ResetLog)
 {
 	ZEN_MEMSCOPE(GetCasContainerTag());
 	ZEN_TRACE_CPU("CasContainer::MakeIndexSnapshot");
 
-	const uint64_t LogCount = FlushLockPosition ? 0 : m_CasLog.GetLogCount();
-	if (m_LogFlushPosition == LogCount)
+	if (m_LogFlushPosition == m_CasLog.GetLogCount())
 	{
 		return;
 	}
@@ -947,7 +946,7 @@ CasContainerStrategy::MakeIndexSnapshot(bool FlushLockPosition)
 
 	namespace fs = std::filesystem;
 
-	fs::path IndexPath = cas::impl::GetIndexPath(m_RootDirectory, m_ContainerBaseName);
+	const fs::path IndexPath = cas::impl::GetIndexPath(m_RootDirectory, m_ContainerBaseName);
 
 	try
 	{
@@ -957,7 +956,10 @@ CasContainerStrategy::MakeIndexSnapshot(bool FlushLockPosition)
 
 		{
 			RwLock::SharedLockScope ___(m_LocationMapLock);
-			IndexLogPosition = m_CasLog.GetLogCount();
+			if (!ResetLog)
+			{
+				IndexLogPosition = m_CasLog.GetLogCount();
+			}
 			Entries.resize(m_LocationMap.size());
 
 			uint64_t EntryIndex = 0;
@@ -967,6 +969,7 @@ CasContainerStrategy::MakeIndexSnapshot(bool FlushLockPosition)
 				IndexEntry.Key				  = Entry.first;
 				IndexEntry.Location			  = m_Locations[Entry.second];
 			}
+			EntryCount = m_LocationMap.size();
 		}
 
 		TemporaryFile	ObjectIndexFile;
@@ -976,7 +979,6 @@ CasContainerStrategy::MakeIndexSnapshot(bool FlushLockPosition)
 		{
 			throw std::system_error(Ec, fmt::format("Failed to create temp file for index snapshot at '{}'", IndexPath));
 		}
-		EntryCount				  = Entries.size();
 		CasDiskIndexHeader Header = {.EntryCount	   = EntryCount,
 									 .LogPosition	   = IndexLogPosition,
 									 .PayloadAlignment = gsl::narrow<uint32_t>(m_PayloadAlignment)};
@@ -989,32 +991,28 @@ CasContainerStrategy::MakeIndexSnapshot(bool FlushLockPosition)
 		ObjectIndexFile.MoveTemporaryIntoPlace(IndexPath, Ec);
 		if (Ec)
 		{
-			ZEN_WARN("snapshot failed to rename new snapshot '{}' to '{}', reason: '{}'",
-					 ObjectIndexFile.GetPath(),
-					 IndexPath,
-					 Ec.message());
+			throw std::system_error(Ec,
+									fmt::format("Snapshot failed to rename new snapshot '{}' to '{}', reason: '{}'",
+												ObjectIndexFile.GetPath(),
+												IndexPath,
+												Ec.message()));
 		}
-		if (FlushLockPosition)
+
+		if (ResetLog)
 		{
-			std::filesystem::path LogPath = cas::impl::GetLogPath(m_RootDirectory, m_ContainerBaseName);
+			const std::filesystem::path LogPath = cas::impl::GetLogPath(m_RootDirectory, m_ContainerBaseName);
 
 			if (IsFile(LogPath))
 			{
 				if (!RemoveFile(LogPath, Ec) || Ec)
 				{
-					ZEN_WARN("snapshot failed to clean log file '{}', removing index at '{}', reason: '{}'",
-							 LogPath,
-							 IndexPath,
-							 Ec.message());
-					std::error_code RemoveIndexEc;
-					RemoveFile(IndexPath, RemoveIndexEc);
+					// 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());
 				}
 			}
 		}
-		if (!Ec)
-		{
-			m_LogFlushPosition = LogCount;
-		}
+		m_LogFlushPosition = IndexLogPosition;
 	}
 	catch (const std::exception& Err)
 	{
@@ -1214,7 +1212,7 @@ CasContainerStrategy::OpenContainer(bool IsNewStore)
 
 	if (IsNewStore || (LogEntryCount > 0))
 	{
-		MakeIndexSnapshot(/*FlushLockPosition*/ true);
+		MakeIndexSnapshot(/*ResetLog*/ true);
 	}
 
 	// TODO: should validate integrity of container files here