From 7a94b22eafdbd3f394fb9200e713cbb3b2b0cd56 Mon Sep 17 00:00:00 2001 From: zousar 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 Results; + eastl::fixed_vector 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 From c7eebb0c8fc5d046d630b462bbfdab1f46bb2d42 Mon Sep 17 00:00:00 2001 From: zousar Date: Tue, 23 Sep 2025 09:26:11 -0600 Subject: Adjust the responses from PUT commands - Ensure that text responses are in a field named "Message" - Change the record response to be named "Record" instead of "Object" --- src/zenstore/cache/cacherpc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/zenstore/cache/cacherpc.cpp') diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp index 5cecc8b6b..77ca26409 100644 --- a/src/zenstore/cache/cacherpc.cpp +++ b/src/zenstore/cache/cacherpc.cpp @@ -438,7 +438,7 @@ CacheRpcHandler::PutCacheRecord(PutRequestData& Request, const CbPackage* Packag if (Count.Invalid > 0) { CbObjectWriter DetailWriter; - DetailWriter.AddString(fmt::format("Found {}/{} invalid attachments", Count.Invalid, Count.Total)); + DetailWriter.AddString("Message", fmt::format("Found {}/{} invalid attachments", Count.Invalid, Count.Total)); return ZenCacheStore::PutResult{PutStatus::Invalid, DetailWriter.Save()}; } @@ -1068,7 +1068,7 @@ CacheRpcHandler::HandleRpcPutCacheValues(const CacheRequestContext& Context, con else { CbObjectWriter DetailWriter; - DetailWriter.AddString(fmt::format("Missing attachment with raw hash {}", RawHash)); + DetailWriter.AddString("Message", fmt::format("Missing attachment with raw hash {}", RawHash)); Results.push_back({zen::PutStatus::Fail, DetailWriter.Save()}); } } -- cgit v1.2.3 From ebfade799e7199f7c6b981f17a55ed67d4323c41 Mon Sep 17 00:00:00 2001 From: zousar Date: Wed, 24 Sep 2025 22:12:11 -0600 Subject: Report Incomplete Records To Client When requesting partial records, report back when a record is incomplete via an "Incomplete" array of bools that is a sibling to the "Result" array for batch/rpc operations, or via the HttpResponseCode::PartialContent status code for individual record requests. --- src/zenstore/cache/cacherpc.cpp | 44 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) (limited to 'src/zenstore/cache/cacherpc.cpp') diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp index 77ca26409..fc34a28cf 100644 --- a/src/zenstore/cache/cacherpc.cpp +++ b/src/zenstore/cache/cacherpc.cpp @@ -860,14 +860,10 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb Key.Hash); } } - if (!Value.Exists && !EnumHasAllFlags(ValuePolicy, CachePolicy::SkipData)) + if (!Value.Exists) { Request.Complete = false; } - // Request.Complete does not need to be set to false for upstream SkipData attachments. - // In the PartialRecord==false case, the upstream will have failed the entire record if any SkipData attachment - // didn't exist and we will not get here. In the PartialRecord==true case, we do not need to inform the client of - // any missing SkipData attachments. } Request.ElapsedTimeUs += Timer.GetElapsedTimeUs(); } @@ -883,7 +879,9 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb ResponsePackage.ReserveAttachments(Requests.size()); + eastl::fixed_vector IncompleteResultIndexes; ResponseObject.BeginArray("Result"sv); + size_t ResultIndex = 0; for (RecordRequestData& Request : Requests) { const CacheKey& Key = Request.Key; @@ -899,6 +897,12 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb } } + if (!Request.Complete) + { + // If requesting a partial record, report back that the record overall is incomplete to the client + IncompleteResultIndexes.push_back(ResultIndex); + } + ZEN_DEBUG("GETCACHERECORD HIT - '{}/{}/{}' {}{} ({}) in {}", *Namespace, Key.Bucket, @@ -935,8 +939,38 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb m_CacheStats.MissCount++; } } + ++ResultIndex; } ResponseObject.EndArray(); + + if (!IncompleteResultIndexes.empty()) + { + size_t IndexIntoIncompleteResultArray = 0; + size_t IncompleteResultIndex = IncompleteResultIndexes[IndexIntoIncompleteResultArray]; + ResultIndex = 0; + ResponseObject.BeginArray("Incomplete"sv); + for (ResultIndex = 0; ResultIndex < Requests.size(); ++ResultIndex) + { + if (IncompleteResultIndex == ResultIndex) + { + ResponseObject.AddBool(true); + if (++IndexIntoIncompleteResultArray >= IncompleteResultIndexes.size()) + { + // Set IncompleteResultIndex into a value we can't encounter while marching ResultIndex forward + IncompleteResultIndex = 0; + } + else + { + IncompleteResultIndex = IncompleteResultIndexes[IndexIntoIncompleteResultArray]; + } + } + else + { + ResponseObject.AddBool(true); + } + } + ResponseObject.EndArray(); + } ResponsePackage.SetObject(ResponseObject.Save()); return ResponsePackage; } -- cgit v1.2.3 From 49db9aa18dc564c3de9ca23eae64d81de54db2d8 Mon Sep 17 00:00:00 2001 From: zousar Date: Thu, 25 Sep 2025 08:11:42 -0600 Subject: Improvement to Incomplete Result Iteration From review feedback --- src/zenstore/cache/cacherpc.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/zenstore/cache/cacherpc.cpp') diff --git a/src/zenstore/cache/cacherpc.cpp b/src/zenstore/cache/cacherpc.cpp index fc34a28cf..e4f23816d 100644 --- a/src/zenstore/cache/cacherpc.cpp +++ b/src/zenstore/cache/cacherpc.cpp @@ -956,8 +956,7 @@ CacheRpcHandler::HandleRpcGetCacheRecords(const CacheRequestContext& Context, Cb ResponseObject.AddBool(true); if (++IndexIntoIncompleteResultArray >= IncompleteResultIndexes.size()) { - // Set IncompleteResultIndex into a value we can't encounter while marching ResultIndex forward - IncompleteResultIndex = 0; + IncompleteResultIndex = (size_t)-1; } else { -- cgit v1.2.3