Skip to content

Commit 9cd7a12

Browse files
authored
[Store] Fix P2PClientService::Put silently swallowing write errors (#1825)
* fix: propagate PutViaRoute error in P2PClientService::Put Put() logged errors from PutViaRoute but always returned success (empty expected), causing callers to believe the write succeeded when the underlying route write actually failed with SEGMENT_NOT_FOUND or NO_AVAILABLE_HANDLE. Return the error for non-idempotent failures while still treating REPLICA_NUM_EXCEEDED / REPLICA_ALREADY_EXISTS as success (object already stored). Fixes #1817 * fix: also capture remote write errors in PutViaRoute result Address review feedback from Gemini Code Assist: 1. PutViaRoute: assign write_result error to result before continue in the remote write path. Previously, if all remote candidates failed, result stayed default-initialized (success) because only local write failures were captured. 2. Put: simplify return to use result directly instead of re-wrapping with tl::unexpected.
1 parent 99fb4d4 commit 9cd7a12

1 file changed

Lines changed: 4 additions & 2 deletions

File tree

mooncake-store/src/p2p_client_service.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ tl::expected<void, ErrorCode> P2PClientService::PutViaRoute(
458458
write_result.error() != ErrorCode::REPLICA_ALREADY_EXISTS) {
459459
LOG(WARNING) << "Remote write to " << endpoint
460460
<< " failed: " << write_result.error();
461+
result = tl::unexpected(write_result.error());
461462
continue; // write failed, attempt next candidate
462463
} else {
463464
// ErrorCode::REPLICA_NUM_EXCEEDED or
@@ -505,9 +506,10 @@ tl::expected<void, ErrorCode> P2PClientService::Put(const ObjectKey& key,
505506
result.error() != ErrorCode::REPLICA_ALREADY_EXISTS) {
506507
LOG(ERROR) << "Failed to put key: " << key
507508
<< " error: " << result.error();
508-
} else {
509-
// the key exists, just ignore the error
509+
return result;
510510
}
511+
// REPLICA_NUM_EXCEEDED / REPLICA_ALREADY_EXISTS: object already
512+
// stored, treat as success so callers don't retry needlessly.
511513
}
512514

513515
return {};

0 commit comments

Comments
 (0)