Restore logic where we accept failed overwrite if resulting size is the same

Correctly calculate the m_TotalSize difference when overwriting file

1 files changed, 44 insertions, 3 deletions

diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp
index 411717e61..b6fd44742 100644
--- a/zenserver/cache/structuredcachestore.cpp
+++ b/zenserver/cache/structuredcachestore.cpp
@@ -1748,6 +1748,8 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 	BuildPath(DataFilePath, HashKey);
 	std::filesystem::path FsPath{DataFilePath.ToPath()};
 
+	uint64_t OldFileSize = 0;
+
 	// 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
@@ -1757,7 +1759,27 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 		Ec.clear();
 		{
 			RwLock::ExclusiveLockScope ValueLock(LockForHash(HashKey));
+
+			std::error_code ExistingEc;
+			OldFileSize = std::filesystem::file_size(FsPath, ExistingEc);
+			if (ExistingEc)
+			{
+				OldFileSize = 0;
+			}
+
 			DataFile.MoveTemporaryIntoPlace(FsPath, Ec);
+
+			if (Ec && (!ExistingEc) && (OldFileSize == Value.Value.Size()))
+			{
+				ZEN_INFO(
+					"Failed to move temporary file '{}' to '{}'. Target file has same size, assuming concurrent write of same value, move "
+					"failed with reason '{}'",
+					DataFile.GetPath(),
+					FsPath.string(),
+					m_BucketDir,
+					Ec.message());
+				return;
+			}
 		}
 
 		if (!Ec)
@@ -1789,7 +1811,15 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 			}
 
 			m_SlogFile.Append({.Key = HashKey, .Location = Loc});
-			m_TotalSize.fetch_add(Loc.Size(), std::memory_order::relaxed);
+			uint64_t NewFileSize = Loc.Size();
+			if (OldFileSize <= NewFileSize)
+			{
+				m_TotalSize.fetch_add(NewFileSize - OldFileSize, std::memory_order::relaxed);
+			}
+			else
+			{
+				m_TotalSize.fetch_sub(OldFileSize - NewFileSize, std::memory_order::relaxed);
+			}
 			return;
 		}
 
@@ -3014,8 +3044,19 @@ TEST_CASE("z$.blocked.disklayer.put")
 	ZenCacheValue BufferGet;
 	CHECK(Zcs.Get("test_bucket", HashKey, BufferGet));
 
-	MemoryView ValueView = BufferGet.Value.GetView();
-	CHECK_THROWS(Zcs.Put("test_bucket", HashKey, {.Value = Buffer}));
+	// Overwriting with a value of same size should go fine
+	Zcs.Put("test_bucket", HashKey, {.Value = Buffer});
+
+	CbObject CacheValue2 = CreateCacheValue(64 * 1024 + 64 + 1);
+	IoBuffer Buffer2	 = CacheValue2.GetBuffer().AsIoBuffer();
+	Buffer2.SetContentType(ZenContentType::kCbObject);
+	// Overwriting with different size should throw exception if file is held open
+	CHECK_THROWS(Zcs.Put("test_bucket", HashKey, {.Value = Buffer2}));
+
+	BufferGet = ZenCacheValue{};
+
+	// Read access has been removed, we should now be able to overwrite it
+	Zcs.Put("test_bucket", HashKey, {.Value = Buffer2});
 }
 #	endif