gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-08-28 07:57:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ecstore): optimize ObjectInfo clone and fix critical TODOs (#3149)

Browse Source 
* fix(ecstore): optimize ObjectInfo clone and fix critical TODOs

## ObjectInfo hot-path clone optimization (issue #653 item 2)
- Wrap user_defined (HashMap), user_tags (String), parts (Vec) in Arc
- Clone cost reduced from ~20 heap allocations to O(1) ref count bump
- Updated 11 downstream access sites with explicit deref where needed

## Critical TODO/FIXME fixes (issue #653 item 7)
- rpc/peer_s3_client.rs: descriptive error message for empty peer response
- set_disk/write.rs: concurrent rollback deletes via tokio::spawn + join_all
- store_list_objects.rs: resolved FIXME with explanation + 4 regression tests
- store/bucket.rs: namespace write locks for make_bucket and delete_bucket

Skipped TODOs (too risky without broader context):
- multipart.rs:30 nslock — causes lock timeout in existing tests
- bucket.rs:88 cached list_bucket — needs cache invalidation strategy
- bucket.rs:149 replication delete — needs replication subsystem integration

1134 tests pass (1 flaky under parallel execution, passes individually).

* fix: address PR #3149 review comments

- Preserve NamespaceLockQuorumUnavailable variant in bucket create/delete
- Fix cancellation test to actually test cancellation (keep senders open)
- Update ObjectInfo consumers in app/ for Arc-backed fields
- Update bucket_usecase.rs user_tags to Arc<String>

* fix: update ObjectInfo Arc consumers

* fix: address ecstore merge review comments

* fix: cancel blocked merge output sends

* fix(ecstore): satisfy ObjectInfo clone clippy
This commit is contained in:
安正超
2026-06-01 08:44:58 +08:00
committed by GitHub
parent bd571a575f
commit 97dd825468
19 changed files with 479 additions and 162 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ecstore/src/store_api/readers.rs
+27 -27
View File
@@ -961,7 +961,7 @@ mod tests {
fn test_http_range_spec_from_object_info_valid_and_invalid_parts() {
let object_info = ObjectInfo {
size: 300,
parts: vec![
parts: Arc::new(vec![
ObjectPartInfo {
etag: String::new(),
number: 1,
@@ -983,7 +983,7 @@ mod tests {
actual_size: 100,
..Default::default()
},
],
]),
..Default::default()
};
@@ -999,7 +999,7 @@ mod tests {
fn test_http_range_spec_from_object_info_uses_actual_size() {
let object_info = ObjectInfo {
size: 90,
parts: vec![
parts: Arc::new(vec![
ObjectPartInfo {
etag: String::new(),
number: 1,
@@ -1021,7 +1021,7 @@ mod tests {
actual_size: 50,
..Default::default()
},
],
]),
..Default::default()
};
@@ -1034,7 +1034,7 @@ mod tests {
fn test_http_range_spec_from_object_info_falls_back_to_part_size_when_actual_size_missing() {
let object_info = ObjectInfo {
size: 90,
parts: vec![
parts: Arc::new(vec![
ObjectPartInfo {
etag: String::new(),
number: 1,
@@ -1056,7 +1056,7 @@ mod tests {
actual_size: 0,
..Default::default()
},
],
]),
..Default::default()
};
@@ -1155,7 +1155,7 @@ mod tests {
("X-Rustfs-Encryption-IV".to_string(), BASE64_STANDARD.encode(base_nonce)),
]);
let object_info = ObjectInfo {
user_defined: metadata,
user_defined: Arc::new(metadata),
..Default::default()
};
let material = resolve_encryption_material(&object_info, &HeaderMap::new())
@@ -1172,10 +1172,10 @@ mod tests {
async fn test_get_object_reader_rejects_ssec_read_without_headers() {
let object_info = ObjectInfo {
size: 10,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption-customer-algorithm".to_string(), "AES256".to_string()),
("x-amz-server-side-encryption-customer-original-size".to_string(), "20".to_string()),
]),
])),
..Default::default()
};
@@ -1201,10 +1201,10 @@ mod tests {
async fn test_get_object_reader_restore_request_bypasses_encryption_range_rewrite() {
let object_info = ObjectInfo {
size: 10,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-rustfs-encryption-key".to_string(), "encrypted-key".to_string()),
("x-rustfs-encryption-original-size".to_string(), "20".to_string()),
]),
])),
..Default::default()
};
@@ -1247,12 +1247,12 @@ mod tests {
let object_info = ObjectInfo {
size: encrypted.len() as i64,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption".to_string(), "AES256".to_string()),
("x-rustfs-encryption-key".to_string(), BASE64_STANDARD.encode(encrypted_dek.as_bytes())),
("x-rustfs-encryption-iv".to_string(), BASE64_STANDARD.encode(base_nonce)),
("x-rustfs-encryption-original-size".to_string(), plaintext.len().to_string()),
]),
])),
..Default::default()
};
@@ -1293,12 +1293,12 @@ mod tests {
let object_info = ObjectInfo {
size: encrypted.len() as i64,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption".to_string(), "AES256".to_string()),
("x-rustfs-encryption-key".to_string(), BASE64_STANDARD.encode(encrypted_dek.as_bytes())),
("x-rustfs-encryption-iv".to_string(), BASE64_STANDARD.encode(base_nonce)),
("x-rustfs-encryption-original-size".to_string(), plaintext.len().to_string()),
]),
])),
..Default::default()
};
let range = HTTPRangeSpec {
@@ -1349,12 +1349,12 @@ mod tests {
let object_info = ObjectInfo {
size: encrypted.len() as i64,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption".to_string(), "AES256".to_string()),
("x-rustfs-encryption-key".to_string(), BASE64_STANDARD.encode(encrypted_dek.as_bytes())),
("x-rustfs-encryption-iv".to_string(), BASE64_STANDARD.encode(base_nonce)),
("x-rustfs-encryption-original-size".to_string(), plaintext.len().to_string()),
]),
])),
..Default::default()
};
@@ -1386,18 +1386,18 @@ mod tests {
let object_info = ObjectInfo {
size: 3_000_000,
parts: vec![ObjectPartInfo {
parts: Arc::new(vec![ObjectPartInfo {
etag: String::new(),
number: 1,
size: 3_000_000,
actual_size: 4_194_304,
index: Some(index.into_vec()),
..Default::default()
}],
user_defined: HashMap::from([
}]),
user_defined: Arc::new(HashMap::from([
("x-minio-internal-compression".to_string(), "gzip".to_string()),
("x-minio-internal-actual-size".to_string(), "4194304".to_string()),
]),
])),
..Default::default()
};
@@ -1443,7 +1443,7 @@ mod tests {
bucket: bucket.to_string(),
name: object.to_string(),
size: encrypted.len() as i64,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption-customer-algorithm".to_string(), "AES256".to_string()),
(
"x-amz-server-side-encryption-customer-key-md5".to_string(),
@@ -1453,7 +1453,7 @@ mod tests {
"x-amz-server-side-encryption-customer-original-size".to_string(),
plaintext.len().to_string(),
),
]),
])),
..Default::default()
};
@@ -1496,7 +1496,7 @@ mod tests {
bucket: bucket.to_string(),
name: object.to_string(),
size: encrypted.len() as i64,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption-customer-algorithm".to_string(), "AES256".to_string()),
(
"x-amz-server-side-encryption-customer-key-md5".to_string(),
@@ -1506,7 +1506,7 @@ mod tests {
"x-amz-server-side-encryption-customer-original-size".to_string(),
plaintext.len().to_string(),
),
]),
])),
..Default::default()
};
let range = HTTPRangeSpec {
@@ -1560,7 +1560,7 @@ mod tests {
bucket: bucket.to_string(),
name: object.to_string(),
size: encrypted.len() as i64,
user_defined: HashMap::from([
user_defined: Arc::new(HashMap::from([
("x-amz-server-side-encryption-customer-algorithm".to_string(), "AES256".to_string()),
(
"x-amz-server-side-encryption-customer-key-md5".to_string(),
@@ -1572,7 +1572,7 @@ mod tests {
),
("x-minio-internal-compression".to_string(), CompressionAlgorithm::default().to_string()),
("x-minio-internal-actual-size".to_string(), plaintext.len().to_string()),
]),
])),
..Default::default()
};
let range = HTTPRangeSpec {
crates/ecstore/src/store_api/types.rs
+82 -16
View File
@@ -293,7 +293,7 @@ pub struct ObjectInfo {
// Actual size is the real size of the object uploaded by client.
pub actual_size: i64,
pub is_dir: bool,
pub user_defined: HashMap<String, String>,
pub user_defined: Arc<HashMap<String, String>>,
pub parity_blocks: usize,
pub data_blocks: usize,
pub version_id: Option<Uuid>,
@@ -301,8 +301,8 @@ pub struct ObjectInfo {
pub transitioned_object: TransitionedObject,
pub restore_ongoing: bool,
pub restore_expires: Option<OffsetDateTime>,
pub user_tags: String,
pub parts: Vec<ObjectPartInfo>,
pub user_tags: Arc<String>,
pub parts: Arc<Vec<ObjectPartInfo>>,
pub is_latest: bool,
pub content_type: Option<String>,
pub content_encoding: Option<String>,
@@ -477,7 +477,11 @@ impl ObjectInfo {
};
// tags
let user_tags = fi.metadata.get(AMZ_OBJECT_TAGGING).cloned().unwrap_or_default();
let user_tags: Arc<String> = fi
.metadata
.get(AMZ_OBJECT_TAGGING)
.map(|s| Arc::new(s.clone()))
.unwrap_or_default();
let inlined = fi.inline_data();
@@ -571,7 +575,7 @@ impl ObjectInfo {
number: part.number,
error: part.error.clone(),
})
.collect();
.collect::<Vec<_>>();
// TODO: part checksums
@@ -585,7 +589,7 @@ impl ObjectInfo {
delete_marker: fi.deleted,
mod_time: fi.mod_time,
size: fi.size,
parts,
parts: Arc::new(parts),
is_latest: fi.is_latest,
user_tags,
content_type,
@@ -595,7 +599,7 @@ impl ObjectInfo {
successor_mod_time: fi.successor_mod_time,
etag,
inlined,
user_defined: metadata,
user_defined: Arc::new(metadata),
transitioned_object,
checksum: fi.checksum.clone(),
storage_class,
@@ -1207,7 +1211,7 @@ mod tests {
let info = ObjectInfo {
size: 100,
actual_size: 0,
user_defined,
user_defined: Arc::new(user_defined),
..Default::default()
};
@@ -1225,7 +1229,7 @@ mod tests {
let info = ObjectInfo {
size: 100,
actual_size: 0,
user_defined,
user_defined: Arc::new(user_defined),
..Default::default()
};
@@ -1277,8 +1281,8 @@ mod tests {
let info = ObjectInfo {
size: 12,
actual_size: 0,
user_defined,
parts: vec![
user_defined: Arc::new(user_defined),
parts: Arc::new(vec![
rustfs_filemeta::ObjectPartInfo {
actual_size: 4,
..Default::default()
@@ -1287,7 +1291,7 @@ mod tests {
actual_size: 5,
..Default::default()
},
],
]),
..Default::default()
};
@@ -1305,7 +1309,7 @@ mod tests {
let info = ObjectInfo {
size: 12,
actual_size: 0,
user_defined,
user_defined: Arc::new(user_defined),
..Default::default()
};
@@ -1329,7 +1333,7 @@ mod tests {
});
let info = ObjectInfo {
user_defined,
user_defined: Arc::new(user_defined),
..Default::default()
};
@@ -1359,7 +1363,7 @@ mod tests {
});
let info = ObjectInfo {
user_defined,
user_defined: Arc::new(user_defined),
..Default::default()
};
@@ -1372,10 +1376,72 @@ mod tests {
user_defined.insert("X-Rustfs-Encryption-Key".to_string(), "encrypted-key".to_string());
let info = ObjectInfo {
user_defined,
user_defined: Arc::new(user_defined),
..Default::default()
};
assert!(info.is_encrypted());
}
#[test]
fn objectinfo_clone_shares_arc_data_and_is_correct() {
let mut ud = HashMap::new();
ud.insert("content-type".to_string(), "application/octet-stream".to_string());
ud.insert("x-custom-header".to_string(), "custom-value".to_string());
let original = ObjectInfo {
bucket: "test-bucket".to_string(),
name: "test-object".to_string(),
user_defined: Arc::new(ud),
user_tags: Arc::new("env=prod&team=storage".to_string()),
parts: Arc::new(vec![
rustfs_filemeta::ObjectPartInfo {
number: 1,
size: 1024,
actual_size: 1024,
..Default::default()
},
rustfs_filemeta::ObjectPartInfo {
number: 2,
size: 512,
actual_size: 512,
..Default::default()
},
]),
size: 1536,
etag: Some("abc123".to_string()),
..Default::default()
};
let cloned = original.clone();
// Verify cloned values are correct
assert_eq!(cloned.bucket, "test-bucket");
assert_eq!(cloned.name, "test-object");
assert_eq!(cloned.size, 1536);
assert_eq!(cloned.etag, Some("abc123".to_string()));
// Verify Arc fields share the same allocation
assert!(Arc::ptr_eq(&original.user_defined, &cloned.user_defined));
assert!(Arc::ptr_eq(&original.user_tags, &cloned.user_tags));
assert!(Arc::ptr_eq(&original.parts, &cloned.parts));
// Verify Arc-wrapped data is accessible through the clone
assert_eq!(
cloned.user_defined.get("content-type").map(String::as_str),
Some("application/octet-stream")
);
assert_eq!(cloned.user_tags.as_str(), "env=prod&team=storage");
assert_eq!(cloned.parts.len(), 2);
assert_eq!(cloned.parts[0].number, 1);
assert_eq!(cloned.parts[1].size, 512);
// Verify default ObjectInfo clone also works
let default_obj = ObjectInfo::default();
let default_cloned = default_obj.clone();
assert!(default_obj.user_defined.is_empty());
assert!(default_cloned.user_defined.is_empty());
assert!(default_cloned.user_tags.is_empty());
assert!(default_cloned.parts.is_empty());
}
}