From 7a94b22eafdbd3f394fb9200e713cbb3b2b0cd56 Mon Sep 17 00:00:00 2001
From: zousar <zousar.shaker@epicgames.com>
Date: Fri, 19 Sep 2025 23:46:52 -0600
Subject: Change batch put responses for client reporting

Conflicts are now treated as successes, and we optionally return a Details array instead of an ErrorMessages array.  Details are returned  for all requests in a batch, or no requests in a batch depending on whether there are any details to be shared about any of the put requests.  The details for a conflict include the raw hash and raw size of the item.  If the item is a record, we also include the record as an object.
---
 src/zenstore/cache/cacherpc.cpp | 64 ++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 20 deletions(-)

(limited to 'src/zenstore/cache/cacherpc.cpp')

diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp
index 83301f863..5cecc8b6b 100644
--- a/src/zenstore/cache/cacherpc.cpp
+++ b/src/zenstore/cache/cacherpc.cpp
@@ -310,7 +310,7 @@ CacheRpcHandler::HandleRpcPutCacheRecords(const CacheRequestContext& Context, co
 	}
 	DefaultPolicy = !PolicyText.empty() ? ParseCachePolicy(PolicyText) : CachePolicy::Default;
 
-	eastl::fixed_vector<bool, 32> Results;
+	eastl::fixed_vector<ZenCacheStore::PutResult, 32> Results;
 
 	CbArrayView RequestsArray = Params["Requests"sv].AsArrayView();
 	for (CbFieldView RequestField : RequestsArray)
@@ -332,33 +332,50 @@ CacheRpcHandler::HandleRpcPutCacheRecords(const CacheRequestContext& Context, co
 									 .Policy	   = std::move(Policy),
 									 .Context	   = Context};
 
-		PutStatus Result = PutCacheRecord(PutRequest, &BatchRequest);
+		ZenCacheStore::PutResult Result = PutCacheRecord(PutRequest, &BatchRequest);
 
-		if (Result == PutStatus::Invalid)
+		if (Result.Status == PutStatus::Invalid)
 		{
 			return CbPackage{};
 		}
-		Results.push_back(Result == PutStatus::Success);
+		Results.push_back(Result);
 	}
 	if (Results.empty())
 	{
 		return CbPackage{};
 	}
 
+	bool		   bWriteAllDetails = false;
 	CbObjectWriter ResponseObject{256};
 	ResponseObject.BeginArray("Result"sv);
-	for (bool Value : Results)
+	for (const ZenCacheStore::PutResult& Result : Results)
 	{
-		ResponseObject.AddBool(Value);
+		// Conflicts will be treated for response purposes
+		// as successful puts because the key is present in the cache.
+		ResponseObject.AddBool((Result.Status == PutStatus::Success) || (Result.Status == PutStatus::Conflict));
+		if (Result.Details)
+		{
+			bWriteAllDetails = true;
+		}
 	}
 	ResponseObject.EndArray();
 
+	if (bWriteAllDetails)
+	{
+		ResponseObject.BeginArray("Details"sv);
+		for (const ZenCacheStore::PutResult& Result : Results)
+		{
+			ResponseObject.AddObject(Result.Details);
+		}
+		ResponseObject.EndArray();
+	}
+
 	CbPackage RpcResponse;
 	RpcResponse.SetObject(ResponseObject.Save());
 	return RpcResponse;
 }
 
-PutStatus
+ZenCacheStore::PutResult
 CacheRpcHandler::PutCacheRecord(PutRequestData& Request, const CbPackage* Package)
 {
 	CbObjectView Record			  = Request.RecordObject;
@@ -420,7 +437,9 @@ CacheRpcHandler::PutCacheRecord(PutRequestData& Request, const CbPackage* Packag
 
 	if (Count.Invalid > 0)
 	{
-		return PutStatus::Invalid;
+		CbObjectWriter DetailWriter;
+		DetailWriter.AddString(fmt::format("Found {}/{} invalid attachments", Count.Invalid, Count.Total));
+		return ZenCacheStore::PutResult{PutStatus::Invalid, DetailWriter.Save()};
 	}
 
 	ZenCacheValue CacheValue;
@@ -440,7 +459,7 @@ CacheRpcHandler::PutCacheRecord(PutRequestData& Request, const CbPackage* Packag
 														  nullptr);
 	if (PutResult.Status != zen::PutStatus::Success)
 	{
-		return PutResult.Status;
+		return PutResult;
 	}
 	m_CacheStats.WriteCount++;
 
@@ -478,7 +497,7 @@ CacheRpcHandler::PutCacheRecord(PutRequestData& Request, const CbPackage* Packag
 										 .Key			  = Request.Key,
 										 .ValueContentIds = std::move(ValidAttachments)});
 	}
-	return PutStatus::Success;
+	return PutResult;
 }
 
 CbPackage
@@ -1048,7 +1067,9 @@ CacheRpcHandler::HandleRpcPutCacheValues(const CacheRequestContext& Context, con
 				}
 				else
 				{
-					Results.push_back({zen::PutStatus::Fail, fmt::format("Missing attachment with raw hash {}", RawHash)});
+					CbObjectWriter DetailWriter;
+					DetailWriter.AddString(fmt::format("Missing attachment with raw hash {}", RawHash));
+					Results.push_back({zen::PutStatus::Fail, DetailWriter.Save()});
 				}
 			}
 			// We do not search the Upstream. No data in a put means the caller is probing for whether they need to do a heavy put.
@@ -1099,29 +1120,32 @@ CacheRpcHandler::HandleRpcPutCacheValues(const CacheRequestContext& Context, con
 		ZEN_TRACE_CPU("Z$::RpcPutCacheValues::Response");
 		CbObjectWriter ResponseObject{1024};
 		ResponseObject.BeginArray("Result"sv);
-		bool bAnyErrors = false;
+		bool bWriteAllDetails = false;
 		for (const ZenCacheStore::PutResult& Value : Results)
 		{
-			if (Value.Status == zen::PutStatus::Success)
+			// Conflicts will be treated for response purposes
+			// as successful puts because the key is present in the cache.
+			if ((Value.Status == zen::PutStatus::Success) || (Value.Status == zen::PutStatus::Conflict))
 			{
 				ResponseObject.AddBool(true);
 			}
 			else
 			{
-				bAnyErrors = true;
 				ResponseObject.AddBool(false);
 			}
+			// If one result has details, we must write all details
+			if (Value.Details)
+			{
+				bWriteAllDetails = true;
+			}
 		}
 		ResponseObject.EndArray();
-		if (bAnyErrors)
+		if (bWriteAllDetails)
 		{
-			ResponseObject.BeginArray("ErrorMessages"sv);
+			ResponseObject.BeginArray("Details"sv);
 			for (const ZenCacheStore::PutResult& Value : Results)
 			{
-				if (Value.Status != zen::PutStatus::Success)
-				{
-					ResponseObject.AddString(Value.Message);
-				}
+				ResponseObject.AddObject(Value.Details);
 			}
 			ResponseObject.EndArray();
 		}
-- 
cgit v1.2.3