oplog prep gc fix (#216)

- Added option gc-validation to zenserver that does a check for missing references in all oplog post full GC. Enabled by default.
- Feature: Added option gc-validation to zen gc command to control reference validation. Enabled by default.
- Added more details in post GC log.
- Fixed race condition in oplog writes which could cause used attachments to be incorrectly removed by GC

3 files changed, 62 insertions, 26 deletions

diff --git a/src/zenstore/include/zenstore/cache/cachedisklayer.h b/src/zenstore/include/zenstore/cache/cachedisklayer.h
index 4b7cf6101..711b96c8f 100644
--- a/src/zenstore/include/zenstore/cache/cachedisklayer.h
+++ b/src/zenstore/include/zenstore/cache/cachedisklayer.h
@@ -199,7 +199,7 @@ public:
 
 	void					 EnableUpdateCapture();
 	void					 DisableUpdateCapture();
-	std::vector<std::string> GetCapturedBuckets();
+	std::vector<std::string> GetCapturedBucketsLocked();
 
 #if ZEN_WITH_TESTS
 	void SetAccessTime(std::string_view Bucket, const IoHash& HashKey, GcClock::TimePoint Time);
@@ -367,9 +367,10 @@ public:
 		std::atomic_uint64_t				 m_StandaloneSize{};
 		std::atomic_uint64_t				 m_MemCachedSize{};
 
-		virtual std::string						 GetGcName(GcCtx& Ctx) override;
-		virtual GcStoreCompactor*				 RemoveExpiredData(GcCtx& Ctx, GcStats& Stats) override;
-		virtual std::vector<GcReferenceChecker*> CreateReferenceCheckers(GcCtx& Ctx) override;
+		virtual std::string						   GetGcName(GcCtx& Ctx) override;
+		virtual GcStoreCompactor*				   RemoveExpiredData(GcCtx& Ctx, GcStats& Stats) override;
+		virtual std::vector<GcReferenceChecker*>   CreateReferenceCheckers(GcCtx& Ctx) override;
+		virtual std::vector<GcReferenceValidator*> CreateReferenceValidators(GcCtx& Ctx) override;
 
 		void	 BuildPath(PathBuilderBase& Path, const IoHash& HashKey) const;
 		void	 PutStandaloneCacheValue(const IoHash& HashKey, const ZenCacheValue& Value, std::span<IoHash> References);
@@ -491,7 +492,6 @@ private:
 	mutable RwLock												  m_Lock;
 	std::unordered_map<std::string, std::unique_ptr<CacheBucket>> m_Buckets;
 	std::vector<std::unique_ptr<CacheBucket>>					  m_DroppedBuckets;
-	mutable RwLock												  m_UpdateCaptureLock;
 	uint32_t													  m_UpdateCaptureRefCounter = 0;
 	std::unique_ptr<std::vector<std::string>>					  m_CapturedBuckets;
 
diff --git a/src/zenstore/include/zenstore/cache/structuredcachestore.h b/src/zenstore/include/zenstore/cache/structuredcachestore.h
index dcdca71c6..82fec9b0e 100644
--- a/src/zenstore/include/zenstore/cache/structuredcachestore.h
+++ b/src/zenstore/include/zenstore/cache/structuredcachestore.h
@@ -279,13 +279,14 @@ public:
 
 	virtual std::vector<RwLock::SharedLockScope> LockState(GcCtx& Ctx) override;
 
-	virtual std::string						 GetGcName(GcCtx& Ctx) override;
-	virtual GcStoreCompactor*				 RemoveExpiredData(GcCtx& Ctx, GcStats& Stats) override;
-	virtual std::vector<GcReferenceChecker*> CreateReferenceCheckers(GcCtx& Ctx) override;
+	virtual std::string						   GetGcName(GcCtx& Ctx) override;
+	virtual GcStoreCompactor*				   RemoveExpiredData(GcCtx& Ctx, GcStats& Stats) override;
+	virtual std::vector<GcReferenceChecker*>   CreateReferenceCheckers(GcCtx& Ctx) override;
+	virtual std::vector<GcReferenceValidator*> CreateReferenceValidators(GcCtx& Ctx) override;
 
 	void					 EnableUpdateCapture();
 	void					 DisableUpdateCapture();
-	std::vector<std::string> GetCapturedNamespaces();
+	std::vector<std::string> GetCapturedNamespacesLocked();
 
 	bool GetContentStats(std::string_view Namespace, std::string_view BucketName, CacheContentStats& OutContentStats) const;
 
@@ -301,7 +302,6 @@ private:
 	mutable RwLock									m_NamespacesLock;
 	NamespaceMap									m_Namespaces;
 	std::vector<std::unique_ptr<ZenCacheNamespace>> m_DroppedNamespaces;
-	mutable RwLock									m_UpdateCaptureLock;
 	uint32_t										m_UpdateCaptureRefCounter = 0;
 	std::unique_ptr<std::vector<std::string>>		m_CapturedNamespaces;
 
diff --git a/src/zenstore/include/zenstore/gc.h b/src/zenstore/include/zenstore/gc.h
index e191a0930..3daae0a93 100644
--- a/src/zenstore/include/zenstore/gc.h
+++ b/src/zenstore/include/zenstore/gc.h
@@ -68,6 +68,7 @@ struct GcSettings
 	IoHash				  AttachmentRangeMax			 = IoHash::Max;
 	bool				  StoreCacheAttachmentMetaData	 = false;
 	bool				  StoreProjectAttachmentMetaData = false;
+	bool				  EnableValidation				 = true;
 };
 
 struct GcCompactStoreStats
@@ -76,6 +77,18 @@ struct GcCompactStoreStats
 	std::chrono::milliseconds ElapsedMS	  = {};
 };
 
+struct GcReferenceValidatorStats
+{
+	std::uint64_t CheckedCount		 = 0;
+	std::uint64_t MissingChunks		 = 0;
+	std::uint64_t MissingFiles		 = 0;
+	std::uint64_t MissingMetas		 = 0;
+	std::uint64_t MissingAttachments = 0;
+
+	std::uint64_t			  MissingCount = 0;
+	std::chrono::milliseconds ElapsedMS	   = {};
+};
+
 struct GcStats
 {
 	std::uint64_t			  CheckedCount = 0;
@@ -90,10 +103,11 @@ struct GcReferencerStats
 	GcStats				RemoveExpiredDataStats;
 	GcCompactStoreStats CompactStoreStats;
 
-	std::chrono::milliseconds CreateReferenceCheckersMS = {};
-	std::chrono::milliseconds PreCacheStateMS			= {};
-	std::chrono::milliseconds UpdateLockedStateMS		= {};
-	std::chrono::milliseconds ElapsedMS					= {};
+	std::chrono::milliseconds CreateReferenceCheckersMS	  = {};
+	std::chrono::milliseconds CreateReferenceValidatorsMS = {};
+	std::chrono::milliseconds PreCacheStateMS			  = {};
+	std::chrono::milliseconds UpdateLockedStateMS		  = {};
+	std::chrono::milliseconds ElapsedMS					  = {};
 };
 
 struct GcReferenceStoreStats
@@ -107,23 +121,27 @@ struct GcReferenceStoreStats
 
 struct GcResult
 {
-	std::vector<std::pair<std::string, GcReferencerStats>>	   ReferencerStats;
-	std::vector<std::pair<std::string, GcReferenceStoreStats>> ReferenceStoreStats;
+	std::vector<std::pair<std::string, GcReferencerStats>>		   ReferencerStats;
+	std::vector<std::pair<std::string, GcReferenceStoreStats>>	   ReferenceStoreStats;
+	std::vector<std::pair<std::string, GcReferenceValidatorStats>> ReferenceValidatorStats;
 
-	GcReferencerStats	  ReferencerStatSum;
-	GcReferenceStoreStats ReferenceStoreStatSum;
-	GcCompactStoreStats	  CompactStoresStatSum;
+	GcReferencerStats		  ReferencerStatSum;
+	GcReferenceStoreStats	  ReferenceStoreStatSum;
+	GcCompactStoreStats		  CompactStoresStatSum;
+	GcReferenceValidatorStats ReferenceValidatorStatSum;
 
 	// Wall times, not sum of each
-	std::chrono::milliseconds RemoveExpiredDataMS		= {};
-	std::chrono::milliseconds CreateReferenceCheckersMS = {};
-	std::chrono::milliseconds PreCacheStateMS			= {};
-	std::chrono::milliseconds LockStateMS				= {};
-	std::chrono::milliseconds UpdateLockedStateMS		= {};
+	std::chrono::milliseconds RemoveExpiredDataMS		  = {};
+	std::chrono::milliseconds CreateReferenceCheckersMS	  = {};
+	std::chrono::milliseconds CreateReferenceValidatorsMS = {};
+	std::chrono::milliseconds PreCacheStateMS			  = {};
+	std::chrono::milliseconds LockStateMS				  = {};
+	std::chrono::milliseconds UpdateLockedStateMS		  = {};
 
 	std::chrono::milliseconds CreateReferencePrunersMS = {};
 	std::chrono::milliseconds RemoveUnreferencedDataMS = {};
 	std::chrono::milliseconds CompactStoresMS		   = {};
+	std::chrono::milliseconds ValidateReferencersMS	   = {};
 
 	std::chrono::milliseconds WriteBlockMS = {};
 
@@ -167,6 +185,17 @@ public:
 	virtual std::string GetGcName(GcCtx& Ctx) = 0;
 };
 
+class GcReferenceValidator
+{
+public:
+	virtual ~GcReferenceValidator() = default;
+
+	virtual std::string GetGcName(GcCtx&) = 0;
+
+	// Validate that that GC did not remove anything needed by this reference checker
+	virtual void Validate(GcCtx& Ctx, GcReferenceValidatorStats& Stats) = 0;
+};
+
 /**
  * @brief An interface to check if a set of Cids are referenced
  *
@@ -240,6 +269,10 @@ public:
 	// Create 0-n GcReferenceChecker for this GcReferencer. Caller will manage lifetime of
 	// returned instances
 	virtual std::vector<GcReferenceChecker*> CreateReferenceCheckers(GcCtx& Ctx) = 0;
+
+	// Create 0-n GcReferenceValidator for this GcReferencer. Caller will manage lifetime of
+	// returned instances
+	virtual std::vector<GcReferenceValidator*> CreateReferenceValidators(GcCtx& Ctx) = 0;
 };
 
 /**
@@ -392,6 +425,7 @@ struct GcSchedulerConfig
 	uint16_t			  AttachmentPassCount				= 1;
 	bool				  StoreCacheAttachmentMetaData		= false;
 	bool				  StoreProjectAttachmentMetaData	= false;
+	bool				  EnableValidation					= true;
 };
 
 struct GcSchedulerState
@@ -471,8 +505,9 @@ public:
 		std::optional<bool>		 SingleThreaded;
 		std::optional<IoHash>	 AttachmentRangeMin;
 		std::optional<IoHash>	 AttachmentRangeMax;
-		std::optional<bool>		 StoreCacheAttachmentMetaData	= false;
-		std::optional<bool>		 StoreProjectAttachmentMetaData = false;
+		std::optional<bool>		 StoreCacheAttachmentMetaData;
+		std::optional<bool>		 StoreProjectAttachmentMetaData;
+		std::optional<bool>		 EnableValidation;
 	};
 
 	bool TriggerGc(const TriggerGcParams& Params);
@@ -504,6 +539,7 @@ private:
 								const IoHash&			  AttachmentRangeMax,
 								bool					  StoreCacheAttachmentMetaData,
 								bool					  StoreProjectAttachmentMetaData,
+								bool					  EnableValidation,
 								bool					  SilenceErrors);
 	void		 ScrubStorage(bool DoDelete, bool SkipCid, std::chrono::seconds TimeSlice);
 	LoggerRef	 Log() { return m_Log; }