happy path should be minimal work

1 files changed, 23 insertions, 23 deletions

diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp
index a4cab881f..c3904d40a 100644
--- a/zenserver/cache/structuredcachestore.cpp
+++ b/zenserver/cache/structuredcachestore.cpp
@@ -1725,6 +1725,8 @@ ZenCacheDiskLayer::UpdateAccessTimes(const zen::access_tracking::AccessTimes& Ac
 void
 ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, const ZenCacheValue& Value)
 {
+	uint64_t NewFileSize = Value.Value.Size();
+
 	TemporaryFile DataFile;
 
 	std::error_code Ec;
@@ -1739,7 +1741,7 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 	{
 		throw std::system_error(Ec,
 								fmt::format("Failed to write payload ({} bytes) to temporary file '{}' for put in '{}'",
-											NiceBytes(Value.Value.Size()),
+											NiceBytes(NewFileSize),
 											DataFile.GetPath().string(),
 											m_BucketDir));
 	}
@@ -1748,9 +1750,7 @@ 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.
+	// We retry to move 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;
@@ -1760,25 +1760,24 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 		{
 			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()))
+			if (Ec)
 			{
-				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;
+				std::error_code ExistingEc;
+				uint64_t		OldFileSize = std::filesystem::file_size(FsPath, ExistingEc);
+				if (!ExistingEc && (OldFileSize == NewFileSize))
+				{
+					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;
+				}
 			}
 		}
 
@@ -1795,9 +1794,10 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 				EntryFlags |= DiskLocation::kCompressed;
 			}
 
-			DiskLocation Loc(Value.Value.Size(), EntryFlags);
+			DiskLocation Loc(NewFileSize, EntryFlags);
 			IndexEntry	 Entry = IndexEntry(Loc, GcClock::TickCount());
 
+			uint64_t				   OldFileSize = 0;
 			RwLock::ExclusiveLockScope _(m_IndexLock);
 			if (auto It = m_Index.find(HashKey); It == m_Index.end())
 			{
@@ -1807,11 +1807,11 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 			else
 			{
 				// TODO: should check if write is idempotent and bail out if it is?
-				It.value() = Entry;
+				OldFileSize = It.value().Location.Size();
+				It.value()	= Entry;
 			}
 
 			m_SlogFile.Append({.Key = HashKey, .Location = Loc});
-			uint64_t NewFileSize = Loc.Size();
 			if (OldFileSize <= NewFileSize)
 			{
 				m_TotalSize.fetch_add(NewFileSize - OldFileSize, std::memory_order::relaxed);