fix(ecstore): describe peer bucket RPC failures with no details (#6190)

heal_bucket, list_bucket, get_bucket_info and delete_bucket returned Error::other("") when a peer answered success=false without an error payload, so operators saw a bare "io error " after quorum reduction. Route all five bucket RPCs through peer_failure_without_details, which names the operation and bucket while staying identical across the peers of one operation so reduce_errs keeps grouping them into a single dominant error.
This commit is contained in:
Zhengchao An
2026-08-18 09:55:29 +08:00
committed by GitHub
parent 35a30cd614
commit 00de43528c
@@ -214,6 +214,19 @@ fn pool_write_quorum(participant_count: usize) -> usize {
(participant_count / 2) + 1
}
/// Error for a peer that reported `success = false` without an error payload.
///
/// The message must stay identical across the peers of one operation: `reduce_errs`
/// buckets `Error::Io` by kind plus rendered message, so any per-peer detail (address,
/// timing) would split one shared failure into single-count buckets and downgrade a real
/// dominant error into `ErasureWriteQuorum`.
fn peer_failure_without_details(op: &str, bucket: Option<&str>) -> Error {
match bucket {
Some(bucket) => Error::other(format!("{op}({bucket}): peer returned failure without error details")),
None => Error::other(format!("{op}: peer returned failure without error details")),
}
}
fn reduce_pool_write_quorum_errs(per_pool_errs: &[Option<Error>]) -> Option<Error> {
if per_pool_errs.is_empty() {
return Some(Error::ErasureWriteQuorum);
@@ -1078,7 +1091,7 @@ impl PeerS3Client for RemotePeerS3Client {
return if let Some(err) = response.error {
Err(err.into())
} else {
Err(Error::other(""))
Err(peer_failure_without_details("heal_bucket", Some(bucket)))
};
}
@@ -1105,7 +1118,7 @@ impl PeerS3Client for RemotePeerS3Client {
return if let Some(err) = response.error {
Err(err.into())
} else {
Err(Error::other(""))
Err(peer_failure_without_details("list_bucket", None))
};
}
let bucket_infos = response
@@ -1136,9 +1149,7 @@ impl PeerS3Client for RemotePeerS3Client {
return if let Some(err) = response.error {
Err(err.into())
} else {
Err(Error::other(format!(
"make_bucket({bucket}): peer returned failure without error details"
)))
Err(peer_failure_without_details("make_bucket", Some(bucket)))
};
}
@@ -1162,7 +1173,7 @@ impl PeerS3Client for RemotePeerS3Client {
return if let Some(err) = response.error {
Err(err.into())
} else {
Err(Error::other(""))
Err(peer_failure_without_details("get_bucket_info", Some(bucket)))
};
}
let bucket_info = serde_json::from_str::<BucketInfo>(&response.bucket_info)?;
@@ -1190,7 +1201,7 @@ impl PeerS3Client for RemotePeerS3Client {
return if let Some(err) = response.error {
Err(err.into())
} else {
Err(Error::other(""))
Err(peer_failure_without_details("delete_bucket", Some(bucket)))
};
}
@@ -2314,4 +2325,37 @@ mod tests {
.collect::<Vec<_>>();
assert_eq!(calls, vec![1, 1, 0, 0, 0, 0, 0, 0]);
}
#[test]
fn peer_failure_without_details_names_operation_and_bucket() {
for op in ["heal_bucket", "make_bucket", "get_bucket_info", "delete_bucket"] {
let message = peer_failure_without_details(op, Some("ops-bucket")).to_string();
assert!(message.contains(op), "{op} message must name the operation: {message}");
assert!(message.contains("ops-bucket"), "{op} message must name the bucket: {message}");
}
let message = peer_failure_without_details("list_bucket", None).to_string();
assert!(message.contains("list_bucket"), "cluster-wide message must name the operation");
assert!(!message.trim().is_empty());
}
#[test]
fn peer_failure_without_details_keeps_one_reduce_errs_bucket_per_operation() {
// reduce_errs groups Io errors by kind plus rendered message: peers failing the
// same operation on the same bucket must still reach quorum as one dominant error.
let per_pool_errs = vec![
Some(peer_failure_without_details("delete_bucket", Some("shared"))),
Some(peer_failure_without_details("delete_bucket", Some("shared"))),
Some(peer_failure_without_details("delete_bucket", Some("shared"))),
];
assert_eq!(
reduce_pool_write_quorum_errs(&per_pool_errs),
Some(peer_failure_without_details("delete_bucket", Some("shared")))
);
assert_ne!(
peer_failure_without_details("delete_bucket", Some("shared")),
peer_failure_without_details("get_bucket_info", Some("shared"))
);
}
}