Merge branch 'main' into lm/restrict-content-type

4 files changed, 50 insertions, 34 deletions

diff --git a/src/zenstore/cache/cachedisklayer.cpp b/src/zenstore/cache/cachedisklayer.cpp
index ead7e4f3a..4640309d9 100644
--- a/src/zenstore/cache/cachedisklayer.cpp
+++ b/src/zenstore/cache/cachedisklayer.cpp
@@ -602,7 +602,7 @@ BucketManifestSerializer::ReadSidecarFile(RwLock::ExclusiveLockScope&								  B
 
 	if (FileSize < sizeof(BucketMetaHeader))
 	{
-		ZEN_WARN("Failed to read sidecar file '{}'. Minimum size {} expected, actual size: ",
+		ZEN_WARN("Failed to read sidecar file '{}'. Minimum size {} expected, actual size: {}",
 				 SidecarPath,
 				 sizeof(BucketMetaHeader),
 				 FileSize);
@@ -626,7 +626,7 @@ BucketManifestSerializer::ReadSidecarFile(RwLock::ExclusiveLockScope&								  B
 		return false;
 	}
 
-	const uint64_t ExpectedEntryCount = (FileSize - sizeof(sizeof(BucketMetaHeader))) / sizeof(ManifestData);
+	const uint64_t ExpectedEntryCount = (FileSize - sizeof(BucketMetaHeader)) / sizeof(ManifestData);
 	if (Header.EntryCount > ExpectedEntryCount)
 	{
 		ZEN_WARN(
@@ -654,6 +654,7 @@ BucketManifestSerializer::ReadSidecarFile(RwLock::ExclusiveLockScope&								  B
 					 SidecarPath,
 					 sizeof(ManifestData),
 					 CurrentReadOffset);
+			break;
 		}
 		CurrentReadOffset += sizeof(ManifestData);
 
@@ -1011,7 +1012,7 @@ ZenCacheDiskLayer::CacheBucket::WriteIndexSnapshotLocked(uint64_t						  LogPosi
 				{
 					// 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());
+					ZEN_WARN("snapshot failed to clean log file '{}', reason: '{}'", LogPath, Ec.message());
 				}
 				m_SlogFile.Open(LogPath, CasLogFile::Mode::kWrite);
 			}
@@ -1057,7 +1058,7 @@ ZenCacheDiskLayer::CacheBucket::ReadIndexFile(RwLock::ExclusiveLockScope&, const
 		return 0;
 	}
 
-	const uint64_t ExpectedEntryCount = (FileSize - sizeof(sizeof(cache::impl::CacheBucketIndexHeader))) / sizeof(DiskIndexEntry);
+	const uint64_t ExpectedEntryCount = (FileSize - sizeof(cache::impl::CacheBucketIndexHeader)) / sizeof(DiskIndexEntry);
 	if (Header.EntryCount > ExpectedEntryCount)
 	{
 		return 0;
@@ -1267,10 +1268,10 @@ ZenCacheDiskLayer::CacheBucket::InitializeIndexFromDisk(RwLock::ExclusiveLockSco
 				{
 					RemoveMemCachedData(IndexLock, Payload);
 					RemoveMetaData(IndexLock, Payload);
+					Location.Flags |= DiskLocation::kTombStone;
+					MissingEntries.push_back(DiskIndexEntry{.Key = It.first, .Location = Location});
 				}
 			}
-			Location.Flags |= DiskLocation::kTombStone;
-			MissingEntries.push_back(DiskIndexEntry{.Key = It.first, .Location = Location});
 		}
 
 		ZEN_ASSERT(!MissingEntries.empty());
@@ -2812,7 +2813,7 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 					 m_BucketDir,
 					 Ec.message(),
 					 RetriesLeft);
-			Sleep(100 - (3 - RetriesLeft) * 100);  // Total 600 ms
+			Sleep((3 - RetriesLeft) * 100);	 // Total 600 ms
 			Ec.clear();
 			DataFile.MoveTemporaryIntoPlace(FsPath, Ec);
 			RetriesLeft--;
@@ -2866,11 +2867,12 @@ ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue(const IoHash& HashKey, c
 	{
 		EntryIndex = It.value();
 		ZEN_ASSERT_SLOW(EntryIndex < PayloadIndex(m_AccessTimes.size()));
-		BucketPayload& Payload	  = m_Payloads[EntryIndex];
-		uint64_t	   OldSize	  = Payload.Location.Size();
+		BucketPayload& Payload = m_Payloads[EntryIndex];
+		uint64_t	   OldSize = Payload.Location.Size();
+		RemoveMemCachedData(IndexLock, Payload);
+		RemoveMetaData(IndexLock, Payload);
 		Payload					  = BucketPayload{.Location = Loc};
 		m_AccessTimes[EntryIndex] = GcClock::TickCount();
-		RemoveMemCachedData(IndexLock, Payload);
 		m_StandaloneSize.fetch_sub(OldSize, std::memory_order::relaxed);
 	}
 	if ((Value.RawSize != 0 || Value.RawHash != IoHash::Zero) && Value.RawSize <= std::numeric_limits<std::uint32_t>::max())
@@ -3521,7 +3523,7 @@ ZenCacheDiskLayer::CacheBucket::GetReferences(const LoggerRef&	   Logger,
 		}
 		else
 		{
-			ZEN_WARN("Cache record {} payload is malformed. Reason: ", RawHash, ToString(Error));
+			ZEN_WARN("Cache record {} payload is malformed. Reason: {}", RawHash, ToString(Error));
 		}
 		return false;
 	};
@@ -4282,8 +4284,8 @@ ZenCacheDiskLayer::DiscoverBuckets()
 	RwLock SyncLock;
 
 	WorkerThreadPool& Pool = GetLargeWorkerPool(EWorkloadType::Burst);
-	std::atomic<bool> AbortFlag;
-	std::atomic<bool> PauseFlag;
+	std::atomic<bool> AbortFlag{false};
+	std::atomic<bool> PauseFlag{false};
 	ParallelWork	  Work(AbortFlag, PauseFlag, WorkerThreadPool::EMode::DisableBacklog);
 	try
 	{
@@ -4454,8 +4456,8 @@ ZenCacheDiskLayer::Flush()
 	}
 	{
 		WorkerThreadPool& Pool = GetMediumWorkerPool(EWorkloadType::Burst);
-		std::atomic<bool> AbortFlag;
-		std::atomic<bool> PauseFlag;
+		std::atomic<bool> AbortFlag{false};
+		std::atomic<bool> PauseFlag{false};
 		ParallelWork	  Work(AbortFlag, PauseFlag, WorkerThreadPool::EMode::DisableBacklog);
 		try
 		{
@@ -4496,8 +4498,8 @@ ZenCacheDiskLayer::Scrub(ScrubContext& Ctx)
 
 	RwLock::SharedLockScope _(m_Lock);
 
-	std::atomic<bool> Abort;
-	std::atomic<bool> Pause;
+	std::atomic<bool> Abort{false};
+	std::atomic<bool> Pause{false};
 	ParallelWork	  Work(Abort, Pause, WorkerThreadPool::EMode::DisableBacklog);
 
 	try
@@ -4559,9 +4561,11 @@ ZenCacheDiskLayer::Stats() const
 ZenCacheDiskLayer::Info
 ZenCacheDiskLayer::GetInfo() const
 {
-	ZenCacheDiskLayer::Info Info = {.RootDir = m_RootDir, .Config = m_Configuration};
+	ZenCacheDiskLayer::Info Info;
+	Info.RootDir = m_RootDir;
 	{
 		RwLock::SharedLockScope _(m_Lock);
+		Info.Config = m_Configuration;
 		Info.BucketNames.reserve(m_Buckets.size());
 		for (auto& Kv : m_Buckets)
 		{
diff --git a/src/zenstore/cache/cachepolicy.cpp b/src/zenstore/cache/cachepolicy.cpp
index ca8a95ca1..c1e7dc5b3 100644
--- a/src/zenstore/cache/cachepolicy.cpp
+++ b/src/zenstore/cache/cachepolicy.cpp
@@ -284,6 +284,9 @@ CacheRecordPolicyBuilder::Build()
 }
 
 #if ZEN_WITH_TESTS
+
+TEST_SUITE_BEGIN("store.cachepolicy");
+
 TEST_CASE("cachepolicy")
 {
 	SUBCASE("atomics serialization")
@@ -400,13 +403,13 @@ TEST_CASE("cacherecordpolicy")
 			RecordPolicy.Save(Writer);
 			CbObject		  Saved	 = Writer.Save()->AsObject();
 			CacheRecordPolicy Loaded = CacheRecordPolicy::Load(Saved).Get();
-			CHECK(!RecordPolicy.IsUniform());
-			CHECK(RecordPolicy.GetRecordPolicy() == UnionPolicy);
-			CHECK(RecordPolicy.GetBasePolicy() == DefaultPolicy);
-			CHECK(RecordPolicy.GetValuePolicy(PartialOid) == PartialOverlap);
-			CHECK(RecordPolicy.GetValuePolicy(NoOverlapOid) == NoOverlap);
-			CHECK(RecordPolicy.GetValuePolicy(OtherOid) == DefaultValuePolicy);
-			CHECK(RecordPolicy.GetValuePolicies().size() == 2);
+			CHECK(!Loaded.IsUniform());
+			CHECK(Loaded.GetRecordPolicy() == UnionPolicy);
+			CHECK(Loaded.GetBasePolicy() == DefaultPolicy);
+			CHECK(Loaded.GetValuePolicy(PartialOid) == PartialOverlap);
+			CHECK(Loaded.GetValuePolicy(NoOverlapOid) == NoOverlap);
+			CHECK(Loaded.GetValuePolicy(OtherOid) == DefaultValuePolicy);
+			CHECK(Loaded.GetValuePolicies().size() == 2);
 		}
 	}
 
@@ -416,6 +419,8 @@ TEST_CASE("cacherecordpolicy")
 		CHECK(Loaded.IsNull());
 	}
 }
+
+TEST_SUITE_END();
 #endif
 
 void
diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp
index 94abcf547..90c5a5e60 100644
--- a/src/zenstore/cache/cacherpc.cpp
+++ b/src/zenstore/cache/cacherpc.cpp
@@ -866,8 +866,8 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb
 						Request.Complete = false;
 					}
 				}
-				Request.ElapsedTimeUs += Timer.GetElapsedTimeUs();
 			}
+			Request.ElapsedTimeUs += Timer.GetElapsedTimeUs();
 		};
 
 		m_UpstreamCache.GetCacheRecords(*Namespace, UpstreamRequests, std::move(OnCacheRecordGetComplete));
@@ -934,7 +934,7 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb
 							  *Namespace,
 							  Key.Bucket,
 							  Key.Hash,
-							  Request.RecordObject ? ""sv : " (PARTIAL)"sv,
+							  Request.RecordObject ? " (PARTIAL)"sv : ""sv,
 							  Request.Source ? Request.Source->Url : "LOCAL"sv,
 							  NiceLatencyNs(Request.ElapsedTimeUs * 1000));
 					m_CacheStats.MissCount++;
@@ -966,7 +966,7 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb
 				}
 				else
 				{
-					ResponseObject.AddBool(true);
+					ResponseObject.AddBool(false);
 				}
 			}
 			ResponseObject.EndArray();
diff --git a/src/zenstore/cache/structuredcachestore.cpp b/src/zenstore/cache/structuredcachestore.cpp
index 52b494e45..cff0e9a35 100644
--- a/src/zenstore/cache/structuredcachestore.cpp
+++ b/src/zenstore/cache/structuredcachestore.cpp
@@ -608,7 +608,10 @@ ZenCacheStore::GetBatch::Commit()
 				m_CacheStore.m_HitCount++;
 				OpScope.SetBytes(Result.Value.GetSize());
 			}
-			m_CacheStore.m_MissCount++;
+			else
+			{
+				m_CacheStore.m_MissCount++;
+			}
 		}
 	}
 }
@@ -683,8 +686,8 @@ ZenCacheStore::Get(const CacheRequestContext& Context,
 		return false;
 	}
 	ZEN_WARN("request for unknown namespace '{}' in ZenCacheStore::Get [{}], bucket '{}', key '{}'",
-			 Context,
 			 Namespace,
+			 Context,
 			 Bucket,
 			 HashKey.ToHexString());
 
@@ -719,8 +722,8 @@ ZenCacheStore::Get(const CacheRequestContext& Context,
 	}
 
 	ZEN_WARN("request for unknown namespace '{}' in ZenCacheStore::Get [{}], bucket '{}', key '{}'",
-			 Context,
 			 Namespace,
+			 Context,
 			 Bucket,
 			 HashKey.ToHexString());
 
@@ -787,8 +790,8 @@ ZenCacheStore::Put(const CacheRequestContext& Context,
 	}
 
 	ZEN_WARN("request for unknown namespace '{}' in ZenCacheStore::Put [{}] bucket '{}', key '{}'",
-			 Context,
 			 Namespace,
+			 Context,
 			 Bucket,
 			 HashKey.ToHexString());
 
@@ -813,7 +816,7 @@ ZenCacheStore::DropNamespace(std::string_view InNamespace)
 {
 	std::function<void()> PostDropOp;
 	{
-		RwLock::SharedLockScope _(m_NamespacesLock);
+		RwLock::ExclusiveLockScope _(m_NamespacesLock);
 		if (auto It = m_Namespaces.find(std::string(InNamespace)); It != m_Namespaces.end())
 		{
 			ZenCacheNamespace& Namespace = *It->second;
@@ -1392,6 +1395,8 @@ namespace testutils {
 
 }  // namespace testutils
 
+TEST_SUITE_BEGIN("store.structuredcachestore");
+
 TEST_CASE("cachestore.store")
 {
 	ScopedTemporaryDirectory TempDir;
@@ -1548,7 +1553,7 @@ TEST_CASE("cachestore.size")
 	}
 }
 
-TEST_CASE("cachestore.threadedinsert")	// * doctest::skip(true))
+TEST_CASE("cachestore.threadedinsert" * doctest::skip())
 {
 	//	for (uint32_t i = 0; i < 100; ++i)
 	{
@@ -2741,6 +2746,8 @@ TEST_CASE("cachestore.newgc.basics")
 	}
 }
 
+TEST_SUITE_END();
+
 #endif
 
 void