perf(ecstore): borrow rename metadata during commit fanout (#6104)

* perf(ecstore): borrow rename metadata during commit fanout

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): preserve rename_data API compatibility

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
This commit is contained in:
houseme
2026-08-14 19:56:36 +08:00
committed by GitHub
parent 6f29431a65
commit 0ff3d4cbf4
10 changed files with 253 additions and 138 deletions
+67 -52
View File
@@ -1359,6 +1359,71 @@ fn validate_decoded_file_info(file_info: &FileInfo) -> Result<()> {
file_info.validate_for_metadata_read().map_err(Into::into)
}
impl RemoteDisk {
#[tracing::instrument(level = "trace", skip_all)]
pub(crate) async fn rename_data_borrowed(
&self,
src_volume: &str,
src_path: &str,
fi: &FileInfo,
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
trace!(
event = EVENT_REMOTE_DISK_RPC,
component = LOG_COMPONENT_ECSTORE,
subsystem = LOG_SUBSYSTEM_REMOTE_DISK,
endpoint = %self.endpoint,
src_volume,
src_path,
dst_volume,
dst_path,
op = "rename_data",
state = "started",
"Remote disk RPC started"
);
self.execute_with_timeout_for_op(
"rename_data",
|| async {
let file_info = compat_json(fi)?;
let file_info_bin = encode_file_info_msgpack(fi)?;
let mut client = self
.get_client()
.await
.map_err(|err| Error::other(format!("can not get client, err: {err}")))?;
let mut request = Request::new(RenameDataRequest {
disk: self.endpoint.to_string(),
src_volume: src_volume.to_string(),
src_path: src_path.to_string(),
file_info,
dst_volume: dst_volume.to_string(),
dst_path: dst_path.to_string(),
file_info_bin: file_info_bin.into(),
});
let canonical_body = rustfs_protos::canonical_rename_data_request_body(request.get_ref());
attach_mutation_body_digest(&mut request, canonical_body, "rename_data")?;
let response = client.rename_data(request).await?.into_inner();
if !response.success {
return Err(response.error.unwrap_or_default().into());
}
let rename_data_resp = decode_msgpack_or_json::<RenameDataResp>(
&response.rename_data_resp_bin,
&response.rename_data_resp,
"RenameDataResp",
)?;
Ok(rename_data_resp)
},
get_max_timeout_duration(),
)
.await
}
}
#[async_trait::async_trait]
impl DiskAPI for RemoteDisk {
#[tracing::instrument(level = "trace", skip_all)]
@@ -2284,58 +2349,8 @@ impl DiskAPI for RemoteDisk {
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
trace!(
event = EVENT_REMOTE_DISK_RPC,
component = LOG_COMPONENT_ECSTORE,
subsystem = LOG_SUBSYSTEM_REMOTE_DISK,
endpoint = %self.endpoint,
src_volume,
src_path,
dst_volume,
dst_path,
op = "rename_data",
state = "started",
"Remote disk RPC started"
);
self.execute_with_timeout_for_op(
"rename_data",
|| async {
let file_info = compat_json(&fi)?;
let file_info_bin = encode_file_info_msgpack(&fi)?;
let mut client = self
.get_client()
.await
.map_err(|err| Error::other(format!("can not get client, err: {err}")))?;
let mut request = Request::new(RenameDataRequest {
disk: self.endpoint.to_string(),
src_volume: src_volume.to_string(),
src_path: src_path.to_string(),
file_info,
dst_volume: dst_volume.to_string(),
dst_path: dst_path.to_string(),
file_info_bin: file_info_bin.into(),
});
let canonical_body = rustfs_protos::canonical_rename_data_request_body(request.get_ref());
attach_mutation_body_digest(&mut request, canonical_body, "rename_data")?;
let response = client.rename_data(request).await?.into_inner();
if !response.success {
return Err(response.error.unwrap_or_default().into());
}
let rename_data_resp = decode_msgpack_or_json::<RenameDataResp>(
&response.rename_data_resp_bin,
&response.rename_data_resp,
"RenameDataResp",
)?;
Ok(rename_data_resp)
},
get_max_timeout_duration(),
)
.await
self.rename_data_borrowed(src_volume, src_path, &fi, dst_volume, dst_path)
.await
}
#[tracing::instrument(level = "trace", skip_all)]
+36 -7
View File
@@ -241,6 +241,40 @@ pub fn get_drive_list_dir_timeout() -> Duration {
)
}
pub(crate) trait DiskStoreRenameDataExt {
async fn rename_data_borrowed(
&self,
src_volume: &str,
src_path: &str,
fi: &FileInfo,
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp>;
}
impl DiskStoreRenameDataExt for LocalDiskWrapper {
async fn rename_data_borrowed(
&self,
src_volume: &str,
src_path: &str,
fi: &FileInfo,
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
self.track_disk_health_mutation(
"rename_data",
DiskMetricMutation::Write,
|| async {
self.disk
.rename_data_borrowed(src_volume, src_path, fi, dst_volume, dst_path)
.await
},
get_max_timeout_duration(),
)
.await
}
}
pub fn get_drive_walkdir_timeout() -> Duration {
get_drive_timeout_duration(
rustfs_config::ENV_DRIVE_WALKDIR_TIMEOUT_SECS,
@@ -1985,13 +2019,8 @@ impl DiskAPI for LocalDiskWrapper {
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
self.track_disk_health_mutation(
"rename_data",
DiskMetricMutation::Write,
|| async { self.disk.rename_data(src_volume, src_path, fi, dst_volume, dst_path).await },
get_max_timeout_duration(),
)
.await
self.rename_data_borrowed(src_volume, src_path, &fi, dst_volume, dst_path)
.await
}
async fn list_dir(&self, origvolume: &str, volume: &str, dir_path: &str, count: i32) -> Result<Vec<String>> {
+15 -1
View File
@@ -8689,11 +8689,12 @@ impl DiskAPI for LocalDisk {
&self,
src_volume: &str,
src_path: &str,
mut fi: FileInfo,
fi: FileInfo,
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
crate::hp_guard!("LocalDisk::rename_data");
let mut fi = fi;
// A non-force DeleteBucket must not remove a directory while a local
// object commit is publishing into it. The peer's empty scan remains
// optimistic; this lease establishes the local commit/delete order and
@@ -10581,6 +10582,19 @@ impl DiskAPI for LocalDisk {
}
}
impl LocalDisk {
pub(crate) async fn rename_data_borrowed(
&self,
src_volume: &str,
src_path: &str,
fi: &FileInfo,
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
<Self as DiskAPI>::rename_data(self, src_volume, src_path, fi.clone(), dst_volume, dst_path).await
}
}
async fn wait_for_startup_cleanup_signal(
startup_cleanup_ready: &AtomicU32,
startup_cleanup_notify: &Notify,
+27 -4
View File
@@ -55,6 +55,7 @@ pub fn part_transaction_path(part_path: &str) -> String {
use crate::cluster::rpc::RemoteDisk;
use crate::cluster::rpc::build_internode_data_transport_from_env;
use crate::disk::disk_store::DiskStoreRenameDataExt;
use crate::disk::disk_store::LocalDiskWrapper;
use crate::disk::health_state::RuntimeDriveHealthState;
use crate::disk::local::ScanGuard;
@@ -434,10 +435,8 @@ impl DiskAPI for Disk {
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
match self {
Disk::Local(local_disk) => local_disk.rename_data(src_volume, src_path, fi, dst_volume, dst_path).await,
Disk::Remote(remote_disk) => remote_disk.rename_data(src_volume, src_path, fi, dst_volume, dst_path).await,
}
self.rename_data_borrowed(src_volume, src_path, &fi, dst_volume, dst_path)
.await
}
#[tracing::instrument(level = "trace", skip_all)]
@@ -667,6 +666,30 @@ impl DiskAPI for Disk {
}
}
impl Disk {
pub(crate) async fn rename_data_borrowed(
&self,
src_volume: &str,
src_path: &str,
fi: &FileInfo,
dst_volume: &str,
dst_path: &str,
) -> Result<RenameDataResp> {
match self {
Disk::Local(local_disk) => {
local_disk
.rename_data_borrowed(src_volume, src_path, fi, dst_volume, dst_path)
.await
}
Disk::Remote(remote_disk) => {
remote_disk
.rename_data_borrowed(src_volume, src_path, fi, dst_volume, dst_path)
.await
}
}
}
}
impl Disk {
pub async fn ns_scanner_server_epoch(&self) -> Result<Option<Uuid>> {
match self {
@@ -46,6 +46,7 @@ use crate::diagnostics::get::{
GetObjectFailureReason, classify_disk_error, get_stage_timer_if_enabled, record_get_object_pipeline_failure,
record_get_object_pipeline_failure_for_path, record_get_stage_duration_if_enabled,
};
use crate::disk::disk_store::DiskStoreRenameDataExt;
use crate::disk::local::DELETE_DATA_DIR_MARKER_PREFIX;
use crate::disk::{
DataDirDeleteStatus, OldCurrentSize, PART_TRANSACTION_NEW_META, PART_TRANSACTION_OLD_META, PART_TRANSACTION_ROLLBACK,
@@ -3011,6 +3012,35 @@ impl RenameConvergence {
}
}
pub(in crate::set_disk) struct RenameDataCommit {
pub(in crate::set_disk) online_disks: Vec<Option<DiskStore>>,
pub(in crate::set_disk) convergence: RenameConvergence,
pub(in crate::set_disk) data_dir: Option<Uuid>,
pub(in crate::set_disk) cleanup_disks: Vec<Option<DiskStore>>,
pub(in crate::set_disk) old_current_size: Option<OldCurrentSize>,
pub(in crate::set_disk) committed_file_info: FileInfo,
}
type RenameDataLegacyTuple = (
Vec<Option<DiskStore>>,
RenameConvergence,
Option<Uuid>,
Vec<Option<DiskStore>>,
Option<OldCurrentSize>,
);
impl RenameDataCommit {
fn into_legacy_tuple(self) -> RenameDataLegacyTuple {
(
self.online_disks,
self.convergence,
self.data_dir,
self.cleanup_disks,
self.old_current_size,
)
}
}
impl SetDisks {
pub(in crate::set_disk) fn default_read_quorum(&self) -> usize {
self.set_drive_count - self.default_parity_count
@@ -3088,6 +3118,14 @@ impl SetDisks {
Ok(())
}
pub(in crate::set_disk) fn assign_rename_data_indexes(file_infos: &mut [FileInfo]) {
for (index, file_info) in file_infos.iter_mut().enumerate() {
if file_info.erasure.index == 0 {
file_info.erasure.index = index + 1;
}
}
}
pub(in crate::set_disk) async fn abort_quota_reservation_after_fence(
reservation: crate::bucket::quota::reservation::QuotaReservation,
disks: &[Option<DiskStore>],
@@ -3118,13 +3156,22 @@ impl SetDisks {
dst_bucket: &str,
dst_object: &str,
write_quorum: usize,
) -> disk::error::Result<(
Vec<Option<DiskStore>>,
RenameConvergence,
Option<Uuid>,
Vec<Option<DiskStore>>,
Option<OldCurrentSize>,
)> {
) -> disk::error::Result<RenameDataLegacyTuple> {
Self::rename_data_owned(disks, src_bucket, src_object, file_infos.to_vec(), dst_bucket, dst_object, write_quorum)
.await
.map(RenameDataCommit::into_legacy_tuple)
}
#[tracing::instrument(level = "debug", skip(disks, file_infos))]
pub(in crate::set_disk) async fn rename_data_owned(
disks: &[Option<DiskStore>],
src_bucket: &str,
src_object: &str,
file_infos: Vec<FileInfo>,
dst_bucket: &str,
dst_object: &str,
write_quorum: usize,
) -> disk::error::Result<RenameDataCommit> {
if let Some(file_info) = disks
.iter()
.zip(file_infos.iter())
@@ -3149,7 +3196,7 @@ impl SetDisks {
let disk_count = disks.len();
let fanout_disks = disks.to_vec();
let fanout_file_infos = file_infos.to_vec();
let fanout_file_infos = file_infos;
let fanout_src_bucket = src_bucket.clone();
let fanout_src_object = src_object.clone();
let fanout_dst_bucket = dst_bucket.clone();
@@ -3161,9 +3208,9 @@ impl SetDisks {
let fanout = tokio::spawn(async move {
let futures = fanout_disks
.into_iter()
.zip(fanout_file_infos)
.zip(fanout_file_infos.iter())
.enumerate()
.map(|(i, (disk, mut file_info))| {
.map(|(i, (disk, file_info))| {
let src_bucket = fanout_src_bucket.clone();
let src_object = fanout_src_object.clone();
let dst_object = fanout_dst_object.clone();
@@ -3180,11 +3227,15 @@ impl SetDisks {
};
let is_delete_marker = file_info.is_canonical_delete_marker();
if file_info.erasure.index == 0 {
file_info.erasure.index = i + 1;
}
if !is_delete_marker && !file_info.has_valid_erasure_geometry() {
let mut local_file_info;
let file_info = if file_info.erasure.index == 0 {
local_file_info = file_info.clone();
local_file_info.erasure.index = i + 1;
&local_file_info
} else {
file_info
};
if file_info.erasure.index == 0 || (!is_delete_marker && !file_info.has_valid_erasure_geometry()) {
return Err(DiskError::FileCorrupt);
}
@@ -3192,12 +3243,13 @@ impl SetDisks {
// A no-op immediately-ready future in production.
Self::rename_fanout_barrier(&dst_object, i, rename_fanout_barrier_phase::RENAME).await;
disk.rename_data(&src_bucket, &src_object, file_info, &dst_bucket, &dst_object)
disk.rename_data_borrowed(&src_bucket, &src_object, file_info, &dst_bucket, &dst_object)
.await
})
.catch_unwind()
});
join_all(futures).await
let results = join_all(futures).await;
(results, fanout_file_infos)
});
let mut disk_versions = vec![None; disk_count];
@@ -3205,7 +3257,7 @@ impl SetDisks {
let mut cleanup_data_dirs = vec![None; disk_count];
let mut old_current_sizes = vec![None; disk_count];
let results = fanout.await.map_err(|_| DiskError::Unexpected)?;
let (results, mut file_infos) = fanout.await.map_err(|_| DiskError::Unexpected)?;
for (idx, result) in results.iter().enumerate() {
match result {
@@ -3261,7 +3313,7 @@ impl SetDisks {
}
if let Some(disk) = disks[i].as_ref() {
let fi = file_infos[i].clone();
let fi = std::mem::take(&mut file_infos[i]);
let old_data_dir = data_dirs[i];
let disk = disk.clone();
let dst_bucket = dst_bucket.clone();
@@ -3384,6 +3436,8 @@ impl SetDisks {
let convergence = Self::classify_rename_convergence(&disk_versions, &errs);
let old_current_size = Self::reduce_common_old_current_size(&old_current_sizes, write_quorum);
let online_disks = Self::eval_disks(disks, &errs);
let committed_slot = online_disks.iter().position(Option::is_some).ok_or(DiskError::Unexpected)?;
let committed_file_info = std::mem::take(&mut file_infos[committed_slot]);
let cleanup_disks = if let Some(data_dir) = data_dir {
disks
.iter()
@@ -3401,7 +3455,14 @@ impl SetDisks {
vec![None; disks.len()]
};
Ok((online_disks, convergence, data_dir, cleanup_disks, old_current_size))
Ok(RenameDataCommit {
online_disks,
convergence,
data_dir,
cleanup_disks,
old_current_size,
committed_file_info,
})
}
/// rustfs/backlog#1009: reduce the per-disk observations of the
+3 -2
View File
@@ -13,6 +13,7 @@
// limitations under the License.
use super::super::*;
use crate::disk::disk_store::DiskStoreRenameDataExt;
use crate::io_support::bitrot::object_mmap_read_enabled;
use crate::storage_api_contracts::namespace::NamespaceLocking as _;
use tracing::trace;
@@ -1164,10 +1165,10 @@ impl SetDisks {
let rename_result = if should_fail_heal_rename(bucket, object, index) {
Err(DiskError::Unexpected)
} else {
disk.rename_data(
disk.rename_data_borrowed(
RUSTFS_META_TMP_BUCKET,
&tmp_id,
parts_metadata[index].clone(),
&parts_metadata[index],
bucket,
object,
)
+10 -6
View File
@@ -2527,11 +2527,12 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
// The trailing `_` drops the rename_data old-size backfill
// (rustfs/backlog#1009): CompleteMultipartUpload keeps its pre-commit
// `get_object_info` lookup, so the backfill has no consumer here yet.
let rename_result = SetDisks::rename_data(
Self::assign_rename_data_indexes(&mut parts_metadatas);
let rename_result = SetDisks::rename_data_owned(
&commit_disks,
RUSTFS_META_MULTIPART_BUCKET,
&commit_upload_id_path,
&parts_metadatas,
parts_metadatas,
&commit_bucket,
&commit_object,
write_quorum,
@@ -2550,10 +2551,15 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
if rename_result.is_ok() {
quota_reservation.commit().await;
}
let (online_disks, convergence, op_old_dir, cleanup_disks, _) = match rename_result {
let rename_commit = match rename_result {
Ok(result) => result,
Err(err) => return Err(err.into()),
};
let online_disks = rename_commit.online_disks;
let convergence = rename_commit.convergence;
let op_old_dir = rename_commit.data_dir;
let cleanup_disks = rename_commit.cleanup_disks;
let committed_file_info = rename_commit.committed_file_info;
// Detach admission before any post-commit await: client cancellation
// must not couple durable convergence repair to cleanup work.
@@ -2598,9 +2604,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
return Err(StorageError::Unexpected);
}
if let Some(committed_slot) = online_disks.iter().position(Option::is_some) {
fi = parts_metadatas[committed_slot].clone();
}
fi = committed_file_info;
let committed_dir = fi.data_dir.unwrap_or_default().to_string();
commit_set.record_capacity_scope_if_needed(commit_capacity_scope_token, &online_disks);
+10 -42
View File
@@ -97,10 +97,6 @@ fn duration_millis_f64(duration: std::time::Duration) -> f64 {
duration.as_secs_f64() * 1000.0
}
fn committed_response_metadata_slot<D>(committed_disks: &[Option<D>], fallback_slot: usize) -> usize {
committed_disks.iter().position(Option::is_some).unwrap_or(fallback_slot)
}
pub(in crate::set_disk::ops) fn assign_object_transaction_epoch(
shuffle_disks: &[Option<DiskStore>],
parts_metadatas: &mut [FileInfo],
@@ -239,38 +235,6 @@ mod duration_metrics_tests {
}
}
#[cfg(test)]
mod put_metadata_tests {
use super::*;
#[test]
fn committed_file_info_follows_exact_quorum_success_slot() {
let mut first_success = FileInfo::new("bucket/object", 2, 2);
first_success.name = "first-success".to_string();
let mut second_success = first_success.clone();
second_success.name = "second-success".to_string();
let mut parts_metadata = [FileInfo::default(), first_success, second_success, FileInfo::default()];
let committed_disks = [None, Some(()), Some(()), None];
assert_eq!(
committed_disks.iter().filter(|disk| disk.is_some()).count(),
2,
"fixture must meet exact quorum"
);
let selected_slot = committed_response_metadata_slot(&committed_disks, 3);
let selected = std::mem::take(&mut parts_metadata[selected_slot]);
assert_eq!(selected.name, "first-success");
assert_eq!(parts_metadata[1], FileInfo::default(), "selected metadata should move without cloning");
assert_eq!(parts_metadata[2].name, "second-success", "other committed metadata must remain available");
assert_eq!(
committed_response_metadata_slot::<()>(&[None, None, None, None], 3),
3,
"a violated post-commit success-mask invariant must not turn a durable PUT into an error"
);
}
}
fn is_restore_control_metadata(key: &str) -> bool {
key.eq_ignore_ascii_case(X_AMZ_RESTORE.as_str())
|| key.eq_ignore_ascii_case(rustfs_utils::http::headers::AMZ_RESTORE_EXPIRY_DAYS)
@@ -2236,11 +2200,12 @@ impl SetDisks {
return Err(err);
}
let rename_result = SetDisks::rename_data(
Self::assign_rename_data_indexes(&mut parts_metadatas);
let rename_result = SetDisks::rename_data_owned(
&commit_disks,
RUSTFS_META_TMP_BUCKET,
commit_tmp_dir.as_str(),
&parts_metadatas,
parts_metadatas,
&commit_bucket,
&commit_object,
write_quorum,
@@ -2259,7 +2224,7 @@ impl SetDisks {
if rename_result.is_ok() {
quota_reservation.commit().await;
}
let (online_disks, convergence, op_old_dir, cleanup_disks, old_current_size) = match rename_result {
let rename_commit = match rename_result {
Ok(commit) => commit,
Err(err) => {
if let Err(cleanup_err) = commit_set.delete_all(RUSTFS_META_TMP_BUCKET, &commit_tmp_dir).await {
@@ -2276,6 +2241,12 @@ impl SetDisks {
return Err(err.into());
}
};
let online_disks = rename_commit.online_disks;
let convergence = rename_commit.convergence;
let op_old_dir = rename_commit.data_dir;
let cleanup_disks = rename_commit.cleanup_disks;
let old_current_size = rename_commit.old_current_size;
let mut fi = rename_commit.committed_file_info;
// Do this before any post-commit await so request cancellation cannot
// bypass best-effort admission. A process crash before admission
// remains subject to the existing scanner reconciliation path.
@@ -2384,9 +2355,6 @@ impl SetDisks {
}
}
let committed_metadata_slot = committed_response_metadata_slot(&online_disks, response_metadata_slot);
let mut fi = std::mem::take(&mut parts_metadatas[committed_metadata_slot]);
if is_compressed {
record_compression_total_memory(actual_size as u64, w_size as u64).await;
}
+1 -1
View File
@@ -1049,7 +1049,7 @@ impl NodeService {
.rename_data(
&request.src_volume,
&request.src_path,
decoded_file_info.value,
&decoded_file_info.value,
&request.dst_volume,
&request.dst_path,
)
+3 -3
View File
@@ -1150,7 +1150,7 @@ pub(crate) trait StorageDiskRpcExt {
&self,
src_volume: &str,
src_path: &str,
file_info: rustfs_filemeta::FileInfo,
file_info: &rustfs_filemeta::FileInfo,
dst_volume: &str,
dst_path: &str,
) -> DiskResult<RenameDataResp>;
@@ -1301,11 +1301,11 @@ where
&self,
src_volume: &str,
src_path: &str,
file_info: rustfs_filemeta::FileInfo,
file_info: &rustfs_filemeta::FileInfo,
dst_volume: &str,
dst_path: &str,
) -> DiskResult<RenameDataResp> {
ecstore_disk::DiskAPI::rename_data(self, src_volume, src_path, file_info, dst_volume, dst_path).await
ecstore_disk::DiskAPI::rename_data(self, src_volume, src_path, file_info.clone(), dst_volume, dst_path).await
}
async fn list_dir(&self, origvolume: &str, volume: &str, dir_path: &str, count: i32) -> DiskResult<Vec<String>> {