restore write using rename in PutStandaloneCacheValue

1 files changed, 61 insertions, 89 deletions

diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp
index 77307bc2d..dee4c55f0 100644
--- a/zenserver/cache/structuredcachestore.cpp
+++ b/zenserver/cache/structuredcachestore.cpp
@@ -1729,124 +1729,96 @@ ZenCacheDiskLayer::UpdateAccessTimes(const zen::access_tracking::AccessTimes& Ac
 void
 ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, const ZenCacheValue& Value)
 {
+	TemporaryFile DataFile;
+
+	std::error_code Ec;
+	DataFile.CreateTemporary(m_BucketDir.c_str(), Ec);
+	if (Ec)
+	{
+		throw std::system_error(Ec, fmt::format("Failed to open temporary file for put at '{}'", m_BucketDir));
+	}
+
+	DataFile.WriteAll(Value.Value, Ec);
+	if (Ec)
+	{
+		throw std::system_error(Ec,
+								fmt::format("Failed to write payload ({} bytes) to temporary file for put in '{}'",
+											NiceBytes(Value.Value.Size()),
+											m_BucketDir));
+	}
+
 	ExtendablePathBuilder<256> DataFilePath;
 	BuildPath(DataFilePath, HashKey);
 	std::filesystem::path FsPath{DataFilePath.ToPath()};
 
-	auto UpdateIndex = [&]() {
-		uint8_t EntryFlags = DiskLocation::kStandaloneFile;
-
-		if (Value.Value.GetContentType() == ZenContentType::kCbObject)
-		{
-			EntryFlags |= DiskLocation::kStructured;
-		}
-		else if (Value.Value.GetContentType() == ZenContentType::kCompressedBinary)
+	// We retry to open the file since it can be held open for read.
+	// This happens if the server processes a Get request for the file or
+	// if we are busy sending the file upstream
+	int RetryCount = 3;
+	do
+	{
+		Ec.clear();
 		{
-			EntryFlags |= DiskLocation::kCompressed;
+			RwLock::ExclusiveLockScope ValueLock(LockForHash(HashKey));
+			DataFile.MoveTemporaryIntoPlace(FsPath, Ec);
 		}
 
-		RwLock::ExclusiveLockScope _(m_IndexLock);
-
-		DiskLocation Loc(Value.Value.Size(), EntryFlags);
-		IndexEntry	 Entry = IndexEntry(Loc, GcClock::TickCount());
-
-		if (auto It = m_Index.find(HashKey); It == m_Index.end())
-		{
-			// Previously unknown object
-			m_Index.insert({HashKey, Entry});
-		}
-		else
+		if (!Ec)
 		{
-			// TODO: should check if write is idempotent and bail out if it is?
-			It.value() = Entry;
-		}
+			uint8_t EntryFlags = DiskLocation::kStandaloneFile;
 
-		m_SlogFile.Append({.Key = HashKey, .Location = Loc});
-		m_TotalSize.fetch_add(Loc.Size(), std::memory_order::relaxed);
-	};
-
-	auto WritePayload = [&](BasicFile& File, const IoBuffer& Payload) {
-		std::error_code Ec;
-		File.WriteAll(Payload, Ec);
-		if (Ec)
-		{
-			File.Close();
-			std::error_code RemoveEc;
-			std::filesystem::remove(FsPath, RemoveEc);
-			if (RemoveEc)
+			if (Value.Value.GetContentType() == ZenContentType::kCbObject)
 			{
-				ZEN_WARN("Failed cleaning up file '{}' after failed write for put in '{}', reason '{}'",
-						 FsPath.string(),
-						 m_BucketDir,
-						 RemoveEc.message());
+				EntryFlags |= DiskLocation::kStructured;
 			}
+			else if (Value.Value.GetContentType() == ZenContentType::kCompressedBinary)
+			{
+				EntryFlags |= DiskLocation::kCompressed;
+			}
+			DiskLocation Loc(Value.Value.Size(), EntryFlags);
+			IndexEntry	 Entry = IndexEntry(Loc, GcClock::TickCount());
 
-			throw std::system_error(Ec,
-									fmt::format("Failed to write payload ({} bytes) to file '{}' for put in '{}'",
-												NiceBytes(Payload.Size()),
-												FsPath,
-												m_BucketDir));
-		}
-		File.Close();
-	};
-
-	// Happy path - directory structure exists and nobody is busy reading the file
-	{
-		std::error_code Ec;
-		BasicFile		DataFile;
+			RwLock::ExclusiveLockScope _(m_IndexLock);
+			if (auto It = m_Index.find(HashKey); It == m_Index.end())
+			{
+				// Previously unknown object
+				m_Index.insert({HashKey, Entry});
+			}
+			else
+			{
+				// TODO: should check if write is idempotent and bail out if it is?
+				It.value() = Entry;
+			}
 
-		RwLock::ExclusiveLockScope ValueLock(LockForHash(HashKey));
-		DataFile.Open(FsPath, BasicFile::Mode::kTruncate, Ec);
-		if (!Ec)
-		{
-			WritePayload(DataFile, Value.Value);
-			ValueLock.ReleaseNow();
-			UpdateIndex();
+			m_SlogFile.Append({.Key = HashKey, .Location = Loc});
+			m_TotalSize.fetch_add(Loc.Size(), std::memory_order::relaxed);
 			return;
 		}
-	}
 
-	std::filesystem::path ParentPath = FsPath.parent_path();
-	if (!std::filesystem::is_directory(ParentPath))
-	{
-		std::error_code Ec;
-		std::filesystem::create_directories(ParentPath, Ec);
-		if (Ec)
+		std::filesystem::path ParentPath = FsPath.parent_path();
+		if (!std::filesystem::is_directory(ParentPath))
 		{
+			Ec.clear();
+			std::filesystem::create_directories(ParentPath, Ec);
+			if (!Ec)
+			{
+				// Retry without sleep
+				continue;
+			}
 			throw std::system_error(
 				Ec,
 				fmt::format("Failed to create parent directory '{}' for file '{}' for put in '{}'", ParentPath, FsPath, m_BucketDir));
 		}
-	}
 
-	// We retry to open the file since it can be held open for read.
-	// This happens if the server processes a Get request for the file or
-	// if we are busy sending the file upstream
-	int				RetryCount = 3;
-	std::error_code Ec;
-	do
-	{
-		BasicFile DataFile;
-		Ec.clear();
-
-		RwLock::ExclusiveLockScope ValueLock(LockForHash(HashKey));
-		DataFile.Open(FsPath, BasicFile::Mode::kTruncate, Ec);
-		if (!Ec)
-		{
-			WritePayload(DataFile, Value.Value);
-			ValueLock.ReleaseNow();
-			UpdateIndex();
-			return;
-		}
 		ZEN_INFO("Failed writing opening file '{}' for writing for put in '{}', pausing and retrying, reason '{}'",
 				 FsPath.string(),
 				 m_BucketDir,
 				 Ec.message());
-		ValueLock.ReleaseNow();
 
 		// Semi arbitrary back-off
 		zen::Sleep(200 * (4 - RetryCount));	 // Sleep at most for a total of 2 seconds
-	} while (RetryCount--);
+		RetryCount--;
+	} while (RetryCount > 0);
 
 	throw std::system_error(Ec, fmt::format("Failed to finalize file '{}' for put in '{}'", DataFilePath.ToUtf8(), m_BucketDir));
 }