Compare commits

..

1 Commits

Author SHA1 Message Date
唐小鸭 a3c812527f fix(ecstore): keep unknown-size sentinel in create_bitrot_writer
SSE and compression wrap the payload so its length is unknown and
advertise HashReader::SIZE_PRESERVE_LAYER (-1). Every layer preserved
that sentinel except create_bitrot_writer, which clamped it to 0 before
calling DiskAPI::create_file. RemoteDisk forwards that size verbatim in
the put_file_stream query, so remote peers were told the body was empty.

Since the authenticated put-file trailer (#5868) the receiver used the
declared size to split body from trailer, turning the clamp into a fatal
"auth trailer has trailing data" failure for every SSE PUT on multi-node
deployments (rc.2). #6320 relaxed the receiver to only trust size > 0;
this change fixes the sender so the sentinel survives end to end and the
wire no longer conflates empty objects with unknown-length streams.

Refs #6331
2026-08-22 16:24:40 +08:00
5 changed files with 66 additions and 140 deletions
+17 -84
View File
@@ -50,10 +50,10 @@ use rustfs_protos::evict_failed_connection;
use rustfs_protos::proto_gen::node_service::RenamePartRequest;
use rustfs_protos::proto_gen::node_service::{
BatchReadVersionRequest, BatchReadVersionResponse, CheckPartsRequest, DeletePathsRequest, DeleteRequest,
DeleteVersionRequest, DeleteVersionsRequest, DeleteVersionsResponse, DeleteVolumeRequest, DiskInfoRequest, ListDirRequest,
ListVolumesRequest, MakeVolumeRequest, MakeVolumesRequest, PreparePartTransactionRequest, ReadAllRequest,
ReadMetadataRequest, ReadMultipleRequest, ReadMultipleResponse, ReadPartsRequest, ReadVersionRequest, ReadXlRequest,
RenameDataRequest, RenameFileRequest, SettlePartTransactionRequest, SnapshotLeaseReleaseRequest, SnapshotLeaseRenewRequest,
DeleteVersionRequest, DeleteVersionsRequest, DeleteVolumeRequest, DiskInfoRequest, ListDirRequest, ListVolumesRequest,
MakeVolumeRequest, MakeVolumesRequest, PreparePartTransactionRequest, ReadAllRequest, ReadMetadataRequest,
ReadMultipleRequest, ReadMultipleResponse, ReadPartsRequest, ReadVersionRequest, ReadXlRequest, RenameDataRequest,
RenameFileRequest, SettlePartTransactionRequest, SnapshotLeaseReleaseRequest, SnapshotLeaseRenewRequest,
SnapshotLeaseRequest, SnapshotLeaseResponse, StatVolumeRequest, UpdateMetadataRequest, VerifyFileRequest, WriteAllRequest,
WriteMetadataRequest, node_service_client::NodeServiceClient,
};
@@ -112,28 +112,6 @@ const EVENT_REMOTE_DISK_RPC: &str = "remote_disk_rpc";
const SNAPSHOT_LEASE_PROTOCOL_VERSION: u32 = 1;
pub const REMOTE_SNAPSHOT_LEASE_TTL: Duration = Duration::from_secs(60);
fn decode_delete_versions_errors(response: DeleteVersionsResponse, expected_len: usize) -> Vec<Option<Error>> {
if !response.item_errors.is_empty() {
if response.item_errors.len() != expected_len {
return vec![Some(Error::other("malformed delete_versions item errors")); expected_len];
}
return response
.item_errors
.into_iter()
.map(|error| (error.code != 0).then(|| error.into()))
.collect();
}
if response.errors.len() != expected_len {
return vec![Some(Error::other("malformed delete_versions errors")); expected_len];
}
response
.errors
.into_iter()
.map(|error| (!error.is_empty()).then(|| Error::other(error)))
.collect()
}
fn snapshot_lease_token_from_response(response: SnapshotLeaseResponse) -> Result<SnapshotLeaseToken> {
if !response.success {
return Err(response.error.unwrap_or_default().into());
@@ -2428,6 +2406,8 @@ impl DiskAPI for RemoteDisk {
return errors;
}
// TODO(backlog): replace string errors with typed `StorageError` variants
let result = self
.execute_with_timeout(
|| async {
@@ -2459,7 +2439,17 @@ impl DiskAPI for RemoteDisk {
}
return errors;
}
decode_delete_versions_errors(response, versions.len())
response
.errors
.iter()
.map(|error| {
if error.is_empty() {
None
} else {
Some(Error::other(error.to_string()))
}
})
.collect()
}
#[tracing::instrument(level = "trace", skip_all)]
@@ -3770,63 +3760,6 @@ mod tests {
static INIT: Once = Once::new();
#[test]
fn delete_versions_response_preserves_typed_item_errors() {
let errors = decode_delete_versions_errors(
DeleteVersionsResponse {
success: true,
errors: vec!["file not found".to_string(), String::new()],
error: None,
item_errors: vec![
rustfs_protos::proto_gen::node_service::Error {
code: DiskError::FileNotFound.to_u32(),
error_info: "file not found".to_string(),
},
rustfs_protos::proto_gen::node_service::Error::default(),
],
},
2,
);
assert!(matches!(errors.as_slice(), [Some(DiskError::FileNotFound), None]));
}
#[test]
fn delete_versions_response_accepts_legacy_string_errors() {
let errors = decode_delete_versions_errors(
DeleteVersionsResponse {
success: true,
errors: vec!["legacy error".to_string(), String::new()],
error: None,
item_errors: Vec::new(),
},
2,
);
assert_eq!(errors.len(), 2);
assert_eq!(errors[0].as_ref().map(ToString::to_string).as_deref(), Some("io error legacy error"));
assert!(errors[1].is_none());
}
#[test]
fn delete_versions_response_rejects_misaligned_item_errors() {
let errors = decode_delete_versions_errors(
DeleteVersionsResponse {
success: true,
errors: vec!["file not found".to_string()],
error: None,
item_errors: vec![rustfs_protos::proto_gen::node_service::Error {
code: DiskError::FileNotFound.to_u32(),
error_info: "file not found".to_string(),
}],
},
2,
);
assert_eq!(errors.len(), 2);
assert!(errors.iter().all(Option::is_some));
}
#[test]
fn disk_mutation_digest_marks_rolling_compatibility() {
let mut request = Request::new(());
+38 -6
View File
@@ -784,6 +784,24 @@ pub(crate) fn create_deferred_bitrot_reader_with_stripe_handle(
///
/// # Returns
/// A Result containing the BitrotWriterWrapper or an error
/// Size hint handed to `DiskAPI::create_file` for a bitrot-wrapped shard.
///
/// A known length is grown by one checksum per shard so the on-disk file size
/// matches what the bitrot writer emits. A negative length is the
/// unknown-size sentinel (`HashReader::SIZE_PRESERVE_LAYER`, used by SSE and
/// compression) and must be preserved: `RemoteDisk::create_file` forwards it
/// in the `put_file_stream` query, and the receiver only treats `size > 0` as
/// a fixed body length when locating the authenticated trailer. Clamping it
/// to `0` would claim an empty body and misframe the stream. `0` stays `0`
/// because a genuinely empty object still means an empty body.
fn bitrot_create_file_size(length: i64, shard_size: usize, checksum_algo: &HashAlgorithm) -> i64 {
if length <= 0 {
return length;
}
let length = length as usize;
(length.div_ceil(shard_size) * checksum_algo.size() + length) as i64
}
pub async fn create_bitrot_writer(
is_inline_buffer: bool,
disk: Option<&DiskStore>,
@@ -796,12 +814,7 @@ pub async fn create_bitrot_writer(
let writer = if is_inline_buffer {
CustomWriter::new_inline_buffer()
} else if let Some(disk) = disk {
let length = if length > 0 {
let length = length as usize;
(length.div_ceil(shard_size) * checksum_algo.size() + length) as i64
} else {
0
};
let length = bitrot_create_file_size(length, shard_size, &checksum_algo);
let file = disk.create_file("", volume, path, length).await?;
#[cfg(feature = "hotpath")]
@@ -820,6 +833,25 @@ mod tests {
use rustfs_rio::ChunkReader;
use std::collections::VecDeque;
#[test]
fn bitrot_create_file_size_grows_known_length_by_checksums() {
// 10 bytes over 4-byte shards = 3 shards, each followed by a 32-byte hash.
assert_eq!(bitrot_create_file_size(10, 4, &HashAlgorithm::HighwayHash256), 10 + 3 * 32);
assert_eq!(bitrot_create_file_size(10, 4, &HashAlgorithm::None), 10);
}
#[test]
fn bitrot_create_file_size_keeps_empty_and_unknown_distinct() {
assert_eq!(bitrot_create_file_size(0, 4, &HashAlgorithm::HighwayHash256), 0);
// SSE/compression streams advertise SIZE_PRESERVE_LAYER (-1); the remote
// put_file_stream receiver relies on a non-positive size to parse the auth
// trailer from the stream tail, so the sentinel must survive untouched.
assert_eq!(
bitrot_create_file_size(rustfs_rio::HashReader::SIZE_PRESERVE_LAYER, 4, &HashAlgorithm::HighwayHash256),
rustfs_rio::HashReader::SIZE_PRESERVE_LAYER
);
}
struct TestChunkReader {
chunks: VecDeque<Bytes>,
}
@@ -722,10 +722,6 @@ pub struct DeleteVersionsResponse {
pub errors: ::prost::alloc::vec::Vec<::prost::alloc::string::String>,
#[prost(message, optional, tag = "3")]
pub error: ::core::option::Option<Error>,
/// Senders dual-write the legacy strings and typed entries. Receivers prefer typed entries
/// when present and fall back to strings for peers that predate this field. Code zero means success.
#[prost(message, repeated, tag = "4")]
pub item_errors: ::prost::alloc::vec::Vec<Error>,
}
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
pub struct ReadMultipleRequest {
-3
View File
@@ -493,9 +493,6 @@ message DeleteVersionsResponse {
bool success = 1;
repeated string errors = 2;
optional Error error = 3;
// Senders dual-write the legacy strings and typed entries. Receivers prefer typed entries
// when present and fall back to strings for peers that predate this field. Code zero means success.
repeated Error item_errors = 4;
}
message ReadMultipleRequest {
+11 -43
View File
@@ -146,29 +146,6 @@ fn encode_file_info_msgpack(value: &FileInfo) -> std::result::Result<Vec<u8>, Di
encode_msgpack_with_capacity(value, "FileInfo", FILE_INFO_MSGPACK_ENCODE_CAPACITY_HINT)
}
fn encode_delete_versions_errors(disk_errors: Vec<Option<DiskError>>) -> (Vec<String>, Vec<Error>) {
let mut errors = Vec::with_capacity(disk_errors.len());
let mut item_errors = Vec::with_capacity(disk_errors.len());
for error in disk_errors {
match error {
Some(error) => {
let code = match &error {
DiskError::Io(source) if source.kind() == std::io::ErrorKind::NotFound => DiskError::FileNotFound.to_u32(),
_ => error.to_u32(),
};
let error_info = error.to_string();
errors.push(error_info.clone());
item_errors.push(Error { code, error_info });
}
None => {
errors.push(String::new());
item_errors.push(Error::default());
}
}
}
(errors, item_errors)
}
fn encode_msgpack_named<T: serde::Serialize>(value: &T, value_name: &str) -> std::result::Result<Vec<u8>, DiskError> {
let mut serializer = rmp_serde::Serializer::new(Vec::with_capacity(MSGPACK_ENCODE_CAPACITY_HINT)).with_struct_map();
value
@@ -575,7 +552,6 @@ impl NodeService {
success: false,
errors: Vec::new(),
error: Some(DiskError::other(format!("decode FileInfoVersions failed: {err}")).into()),
item_errors: Vec::new(),
}));
}
};
@@ -587,26 +563,30 @@ impl NodeService {
success: false,
errors: Vec::new(),
error: Some(DiskError::other(format!("decode DeleteOptions failed: {err}")).into()),
item_errors: Vec::new(),
}));
}
};
let (errors, item_errors) =
encode_delete_versions_errors(disk.delete_versions(&request.volume, versions, opts).await);
let errors = disk
.delete_versions(&request.volume, versions, opts)
.await
.into_iter()
.map(|error| match error {
Some(e) => e.to_string(),
None => "".to_string(),
})
.collect();
Ok(Response::new(DeleteVersionsResponse {
success: true,
errors,
error: None,
item_errors,
}))
} else {
Ok(Response::new(DeleteVersionsResponse {
success: false,
errors: Vec::new(),
error: Some(DiskError::other("cannot find disk".to_string()).into()),
item_errors: Vec::new(),
}))
}
}
@@ -1632,8 +1612,8 @@ impl NodeService {
mod tests {
use super::{
compat_response_json, decode_msgpack_or_json, decode_rename_data_request_file_info,
encode_batch_read_version_response_payloads, encode_delete_versions_errors, encode_file_info_msgpack, encode_msgpack,
encode_msgpack_named, encode_read_multiple_response_payloads, encode_rename_data_response_payloads,
encode_batch_read_version_response_payloads, encode_file_info_msgpack, encode_msgpack, encode_msgpack_named,
encode_read_multiple_response_payloads, encode_rename_data_response_payloads,
};
use crate::storage::rpc::node_service::make_server;
use crate::storage::storage_api::ReadMultipleResp;
@@ -1652,18 +1632,6 @@ mod tests {
count: u32,
}
#[test]
fn delete_versions_response_dual_writes_typed_item_errors() {
let raw_not_found = super::DiskError::Io(std::io::Error::from(std::io::ErrorKind::NotFound));
let (errors, item_errors) = encode_delete_versions_errors(vec![Some(raw_not_found), None]);
assert!(errors[0].starts_with("io error "));
assert!(errors[1].is_empty());
assert_eq!(item_errors[0].code, super::DiskError::FileNotFound.to_u32());
assert_eq!(item_errors[0].error_info, errors[0]);
assert_eq!(item_errors[1].code, 0);
}
#[tokio::test]
#[serial]
async fn handle_read_version_records_attribution_for_missing_disk() {