Merge pull request #123 from EpicGames/de/fix-gc-over-multiple-namespaces

We need to make each Cache Key contribution per bucket unique across namespaces

4 files changed, 13 insertions, 16 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 468f4a1ae..3bbe1d001 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,7 @@
 ##
+- Fixed expired cache keys overwriting between namespaces when bucket names were the same in multiple namespaces
+
+## v0.1.2
 - Tweak bundle compression settings to streamline build
 - ZenCacheDiskLayer::CacheBucket::GatherReferences: Don't hold index lock while reading standalone values
 - hardening of ZenCacheDiskLayer::CacheBucket::PutStandaloneCacheValue
diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp
index bc60a1d26..91e374ae2 100644
--- a/zenserver/cache/structuredcachestore.cpp
+++ b/zenserver/cache/structuredcachestore.cpp
@@ -1461,7 +1461,7 @@ ZenCacheDiskLayer::CacheBucket::GatherReferences(GcContext& GcCtx)
 	}
 
 	GcCtx.ContributeCids(Cids);
-	GcCtx.ContributeCacheKeys(m_BucketName, std::move(ExpiredKeys));
+	GcCtx.ContributeCacheKeys(m_BucketDir.string(), std::move(ExpiredKeys));
 }
 
 void
@@ -1506,7 +1506,7 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx)
 
 	m_SlogFile.Flush();
 
-	std::span<const IoHash> ExpiredCacheKeys = GcCtx.ExpiredCacheKeys(m_BucketName);
+	std::span<const IoHash> ExpiredCacheKeys = GcCtx.ExpiredCacheKeys(m_BucketDir.string());
 	std::vector<IoHash>		DeleteCacheKeys;
 	DeleteCacheKeys.reserve(ExpiredCacheKeys.size());
 	GcCtx.FilterCas(ExpiredCacheKeys, [&](const IoHash& ChunkHash, bool Keep) {
diff --git a/zenstore/gc.cpp b/zenstore/gc.cpp
index 8e2d441f8..dfa3d54ab 100644
--- a/zenstore/gc.cpp
+++ b/zenstore/gc.cpp
@@ -170,15 +170,9 @@ SaveCompactBinaryObject(const fs::path& Path, const CbObject& Object)
 
 struct GcContext::GcState
 {
-	struct CacheBucket
-	{
-		std::vector<IoHash> ValidKeys;
-		std::vector<IoHash> ExpiredKeys;
-	};
-
-	using CacheBuckets = std::unordered_map<std::string, CacheBucket>;
+	using CacheKeyContexts = std::unordered_map<std::string, std::vector<IoHash>>;
 
-	CacheBuckets	   m_CacheBuckets;
+	CacheKeyContexts   m_ExpiredCacheKeys;
 	CasChunkSet		   m_CasChunks;
 	CasChunkSet		   m_DeletedCasChunks;
 	CasChunkSet		   m_CidChunks;
@@ -212,9 +206,9 @@ GcContext::ContributeCas(std::span<const IoHash> Cas)
 }
 
 void
-GcContext::ContributeCacheKeys(const std::string& Bucket, std::vector<IoHash>&& ExpiredKeys)
+GcContext::ContributeCacheKeys(const std::string& CacheKeyContext, std::vector<IoHash>&& ExpiredKeys)
 {
-	m_State->m_CacheBuckets[Bucket].ExpiredKeys = std::move(ExpiredKeys);
+	m_State->m_ExpiredCacheKeys[CacheKeyContext] = std::move(ExpiredKeys);
 }
 
 void
@@ -254,9 +248,9 @@ GcContext::DeletedCas()
 }
 
 std::span<const IoHash>
-GcContext::ExpiredCacheKeys(const std::string& Bucket) const
+GcContext::ExpiredCacheKeys(const std::string& CacheKeyContext) const
 {
-	return m_State->m_CacheBuckets[Bucket].ExpiredKeys;
+	return m_State->m_ExpiredCacheKeys[CacheKeyContext];
 }
 
 bool
diff --git a/zenstore/include/zenstore/gc.h b/zenstore/include/zenstore/gc.h
index 6268588ec..a3ad19e10 100644
--- a/zenstore/include/zenstore/gc.h
+++ b/zenstore/include/zenstore/gc.h
@@ -53,7 +53,7 @@ public:
 
 	void ContributeCids(std::span<const IoHash> Cid);
 	void ContributeCas(std::span<const IoHash> Hash);
-	void ContributeCacheKeys(const std::string& Bucket, std::vector<IoHash>&& ExpiredKeys);
+	void ContributeCacheKeys(const std::string& CacheKeyContext, std::vector<IoHash>&& ExpiredKeys);
 
 	void IterateCids(std::function<void(const IoHash&)> Callback);
 
@@ -64,7 +64,7 @@ public:
 	void		 DeletedCas(std::span<const IoHash> Cas);
 	CasChunkSet& DeletedCas();
 
-	std::span<const IoHash> ExpiredCacheKeys(const std::string& Bucket) const;
+	std::span<const IoHash> ExpiredCacheKeys(const std::string& CacheKeyContext) const;
 
 	bool IsDeletionMode() const;
 	void SetDeletionMode(bool NewState);