perf(ecstore): backfill rename_data old size and gate the PUT prelookup (#4598)

This commit is contained in:
Zhengchao An
2026-07-09 12:08:01 +08:00
committed by GitHub
parent 579cdf52dc
commit 4f999bb6b8
17 changed files with 1161 additions and 68 deletions
+350 -13
View File
@@ -18,7 +18,7 @@ use crate::disk::disk_store::get_object_disk_read_timeout;
use crate::disk::{
BUCKET_META_PREFIX, CHECK_PART_FILE_CORRUPT, CHECK_PART_FILE_NOT_FOUND, CHECK_PART_SUCCESS, CHECK_PART_UNKNOWN,
CHECK_PART_VOLUME_NOT_FOUND, CheckPartsResp, DeleteOptions, DiskAPI, DiskInfo, DiskInfoOptions, DiskLocation, DiskMetrics,
FileInfoVersions, FileReader, FileWriter, MmapCopyStageMetrics, RUSTFS_META_BUCKET, RUSTFS_META_TMP_BUCKET,
FileInfoVersions, FileReader, FileWriter, MmapCopyStageMetrics, OldCurrentSize, RUSTFS_META_BUCKET, RUSTFS_META_TMP_BUCKET,
RUSTFS_META_TMP_DELETED_BUCKET, ReadMultipleReq, ReadMultipleResp, ReadOptions, RenameDataResp, STORAGE_FORMAT_FILE,
STORAGE_FORMAT_FILE_BACKUP, UpdateMetadataOpts, VolumeInfo, WalkDirOptions, conv_part_err_to_int,
endpoint::Endpoint,
@@ -4076,6 +4076,50 @@ fn rename_data_versions_signature(meta: &FileMeta) -> Option<Vec<u8>> {
Some(signature)
}
/// rustfs/backlog#1009: observe the destination key's *current* (latest)
/// version in the dst `xl.meta` that `rename_data` already loaded, before
/// `add_version` commits the incoming one. Replicates the pre-PUT
/// `get_object_info` outcome bit for bit, as the per-disk `read_version`
/// pipeline (`get_file_info`) would report it:
///
/// - `dst_meta_existed == false` (no dst xl.meta on this disk): the lookup
/// errors FileNotFound → app-level `None` → `Absent`.
/// - Existing meta whose versions are all hidden free versions: the lookup
/// errors FileNotFound the same way → `Absent`.
/// - Existing meta with zero versions: `get_file_info` synthesizes a deleted
/// `FileInfo` with size 0 and the lookup returns `Ok` → `Present(0)`.
/// - A resolvable latest version — live object, delete marker, or a
/// purge-pending version flagged `deleted` — returns `Ok` with
/// `ObjectInfo.size == fi.size` (0 for markers) → `Present(fi.size)`.
/// Delete markers deliberately do NOT map to `Absent`: today's lookup
/// returns `Ok(size 0)` for them, and delete-marker creation never
/// decrements `objects_count`, so `Some(0)` is what keeps versioned
/// accounting bit-identical.
/// - A latest version that fails to decode — including the part-array length
/// guard that `all_parts=true` enables, the same flag the per-disk lookup
/// uses — yields `None` (unknown): the old lookup surfaced a per-disk error
/// there, so this disk must not vote in the set-level quorum reduction.
///
/// One deliberate divergence: `read_data` stays `false` (the lookup used
/// `true`), so a corrupt inline-data map that would have errored the old
/// lookup votes the version's own `size` here instead of abstaining. The
/// size field decodes independently of the inline data, so the vote carries
/// the same value healthy disks report, and skipping the lookup's inline
/// bytes clone keeps the observation allocation-light.
fn observe_old_current_size(dst_meta_existed: bool, xlmeta: &FileMeta) -> Option<OldCurrentSize> {
if !dst_meta_existed {
return Some(OldCurrentSize::Absent);
}
if xlmeta.versions.is_empty() {
return Some(OldCurrentSize::Present(0));
}
match xlmeta.into_fileinfo("", "", "", false, false, true) {
Ok(fi) => Some(OldCurrentSize::Present(fi.size)),
Err(rustfs_filemeta::Error::FileNotFound) => Some(OldCurrentSize::Absent),
Err(_) => None,
}
}
fn is_root_path(path: impl AsRef<Path>) -> bool {
path.as_ref().components().count() == 1 && path.as_ref().has_root()
}
@@ -5108,13 +5152,27 @@ impl DiskAPI for LocalDisk {
};
let mut xlmeta = FileMeta::new();
if let Some(dst_buf) = has_dst_buf.as_ref()
&& FileMeta::is_xl2_v1_format(dst_buf)
&& let Ok(nmeta) = FileMeta::load(dst_buf)
{
xlmeta = nmeta
// An existing dst xl.meta that fails to parse leaves `xlmeta` empty
// and gets overwritten by the commit below (pre-existing behavior);
// track that so the old-size observation reports unknown instead of
// a false `Absent` (rustfs/backlog#1009).
let mut dst_meta_unparsable = false;
if let Some(dst_buf) = has_dst_buf.as_ref() {
if FileMeta::is_xl2_v1_format(dst_buf)
&& let Ok(nmeta) = FileMeta::load(dst_buf)
{
xlmeta = nmeta
} else {
dst_meta_unparsable = true;
}
}
let old_current_size = if dst_meta_unparsable {
None
} else {
observe_old_current_size(has_dst_buf.is_some(), &xlmeta)
};
let mut skip_parent = dst_volume_dir.clone();
if has_dst_buf.as_ref().is_some()
&& let Some(parent) = dst_file_path.parent()
@@ -5325,6 +5383,7 @@ impl DiskAPI for LocalDisk {
Ok(RenameDataResp {
old_data_dir: has_old_data_dir,
sign: version_signature,
old_current_size,
})
} else {
// Inline: merge read + parse + write + rename into single spawn_blocking
@@ -5338,7 +5397,7 @@ impl DiskAPI for LocalDisk {
None
};
let (old_data_dir, version_signature) = tokio::task::spawn_blocking(move || {
let (old_data_dir, version_signature, old_current_size) = tokio::task::spawn_blocking(move || {
// Read existing xl.meta
let has_dst_buf = match std::fs::read(&dst) {
Ok(buf) => Some(Bytes::from(buf)),
@@ -5347,13 +5406,26 @@ impl DiskAPI for LocalDisk {
};
let mut xlmeta = FileMeta::new();
if let Some(ref buf) = has_dst_buf
&& FileMeta::is_xl2_v1_format(buf)
&& let Ok(nmeta) = FileMeta::load(buf)
{
xlmeta = nmeta
// Same as the non-inline branch: an unparsable existing dst
// xl.meta must surface as unknown, not `Absent`
// (rustfs/backlog#1009).
let mut dst_meta_unparsable = false;
if let Some(ref buf) = has_dst_buf {
if FileMeta::is_xl2_v1_format(buf)
&& let Ok(nmeta) = FileMeta::load(buf)
{
xlmeta = nmeta
} else {
dst_meta_unparsable = true;
}
}
let old_current_size = if dst_meta_unparsable {
None
} else {
observe_old_current_size(has_dst_buf.is_some(), &xlmeta)
};
let version_id = fi.version_id.unwrap_or_default();
let old_data_dir = xlmeta.find_unshared_data_dir_for_version(Some(version_id));
if let Some(d) = old_data_dir.as_ref() {
@@ -5450,7 +5522,11 @@ impl DiskAPI for LocalDisk {
}
}
Ok::<(Option<uuid::Uuid>, Option<Vec<u8>>), std::io::Error>((old_data_dir, version_signature))
Ok::<(Option<uuid::Uuid>, Option<Vec<u8>>, Option<OldCurrentSize>), std::io::Error>((
old_data_dir,
version_signature,
old_current_size,
))
})
.await
.map_err(DiskError::from)??;
@@ -5465,6 +5541,7 @@ impl DiskAPI for LocalDisk {
Ok(RenameDataResp {
old_data_dir,
sign: version_signature,
old_current_size,
})
}
}
@@ -7669,6 +7746,266 @@ mod test {
.join(STORAGE_FORMAT_FILE)
.exists()
);
// rustfs/backlog#1009: the overwritten live current version (size 1
// from `test_file_info`) must be surfaced through the backfill.
assert_eq!(resp.old_current_size, Some(OldCurrentSize::Present(1)));
}
/// rustfs/backlog#1009: `observe_old_current_size` must mirror the pre-PUT
/// `get_object_info` semantics bit for bit — latest version's
/// `ObjectInfo.size` (0 for a delete-marker latest, which that lookup
/// returns as `Ok`, not as not-found), missing key → `Absent` — and
/// `rename_data` must report it for both the inline and non-inline commit
/// branches.
mod old_current_size_backfill {
use super::*;
use tempfile::tempdir;
fn live_file_info(name: &str, version_id: Uuid, size: i64, mod_time: OffsetDateTime) -> FileInfo {
FileInfo {
name: name.to_string(),
version_id: Some(version_id),
size,
mod_time: Some(mod_time),
..Default::default()
}
}
fn delete_marker_file_info(name: &str, version_id: Uuid, mod_time: OffsetDateTime) -> FileInfo {
FileInfo {
name: name.to_string(),
version_id: Some(version_id),
deleted: true,
mod_time: Some(mod_time),
..Default::default()
}
}
#[test]
fn observe_reports_absent_for_missing_key() {
assert_eq!(observe_old_current_size(false, &FileMeta::default()), Some(OldCurrentSize::Absent));
}
/// An existing xl.meta with zero versions reads back through
/// `get_file_info` as a synthetic deleted FileInfo of size 0, so the
/// pre-PUT lookup reported `Some(0)`, not not-found.
#[test]
fn observe_reports_present_zero_for_existing_versionless_meta() {
assert_eq!(observe_old_current_size(true, &FileMeta::default()), Some(OldCurrentSize::Present(0)));
}
#[test]
fn observe_reports_latest_live_version_size() {
let now = OffsetDateTime::now_utc();
let mut meta = FileMeta::new();
meta.add_version(live_file_info("object", Uuid::new_v4(), 10, now - time::Duration::seconds(10)))
.expect("older live version should be added");
meta.add_version(live_file_info("object", Uuid::new_v4(), 42, now))
.expect("newer live version should be added");
assert_eq!(observe_old_current_size(true, &meta), Some(OldCurrentSize::Present(42)));
}
/// The pre-PUT lookup returns `Ok(size 0)` for a delete-marker latest
/// (RustFS's `SetDisks::get_object_info` does not convert markers to
/// not-found), and delete-marker creation never decrements
/// objects_count — so the backfill must report `Present(0)` here, not
/// `Absent`, to keep versioned accounting bit-identical.
#[test]
fn observe_reports_present_zero_for_delete_marker_latest() {
let now = OffsetDateTime::now_utc();
let mut meta = FileMeta::new();
meta.add_version(live_file_info("object", Uuid::new_v4(), 42, now - time::Duration::seconds(10)))
.expect("live version should be added");
meta.add_version(delete_marker_file_info("object", Uuid::new_v4(), now))
.expect("delete marker should be added");
assert_eq!(observe_old_current_size(true, &meta), Some(OldCurrentSize::Present(0)));
}
/// A latest version whose part arrays are corrupt (lengths disagree)
/// made the old per-disk lookup error out — this disk must abstain
/// (`None`), never vote. Pins the `all_parts=true` flag that enables
/// the part-array length guard.
#[test]
fn observe_abstains_for_corrupt_part_arrays() {
let now = OffsetDateTime::now_utc();
let mut fi = live_file_info("object", Uuid::new_v4(), 42, now);
fi.add_object_part(1, "etag".to_string(), 42, Some(now), 42, None, None);
let mut version = rustfs_filemeta::FileMetaVersion::from(fi);
version
.object
.as_mut()
.expect("object version should carry a MetaObject")
.part_sizes
.clear();
let mut meta = FileMeta::new();
meta.add_version_filemata(version)
.expect("corrupt-part version should still insert");
assert_eq!(observe_old_current_size(true, &meta), None);
}
async fn test_disk(dir: &tempfile::TempDir) -> LocalDisk {
let endpoint =
Endpoint::try_from(dir.path().to_str().expect("temp dir should be utf8")).expect("endpoint should parse");
let disk = LocalDisk::new(&endpoint, false).await.expect("local disk should be created");
ensure_test_volume(&disk, "bucket").await;
ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await;
disk
}
#[tokio::test]
async fn inline_rename_data_reports_absent_for_fresh_key() {
let dir = tempdir().expect("temp dir should be created");
let disk = test_disk(&dir).await;
let new_fi = test_file_info("object", Uuid::new_v4(), None, Some(Bytes::from_static(b"inline-new")));
let resp = disk
.rename_data(RUSTFS_META_TMP_BUCKET, "tmp-fresh", new_fi, "bucket", "object")
.await
.expect("inline rename_data should commit");
assert_eq!(resp.old_current_size, Some(OldCurrentSize::Absent));
}
#[tokio::test]
async fn inline_rename_data_reports_present_zero_for_delete_marker_latest() {
let dir = tempdir().expect("temp dir should be created");
let disk = test_disk(&dir).await;
let now = OffsetDateTime::now_utc();
let mut old_meta = FileMeta::new();
old_meta
.add_version(live_file_info("object", Uuid::new_v4(), 42, now - time::Duration::seconds(10)))
.expect("live version should be added");
old_meta
.add_version(delete_marker_file_info("object", Uuid::new_v4(), now))
.expect("delete marker should be added");
let dst_object_dir = dir.path().join("bucket").join("object");
fs::create_dir_all(&dst_object_dir).await.expect("dst dir should be created");
fs::write(
dst_object_dir.join(STORAGE_FORMAT_FILE),
old_meta.marshal_msg().expect("old metadata should encode"),
)
.await
.expect("old metadata should be written");
let new_fi = test_file_info("object", Uuid::new_v4(), None, Some(Bytes::from_static(b"inline-new")));
let resp = disk
.rename_data(RUSTFS_META_TMP_BUCKET, "tmp-marker", new_fi, "bucket", "object")
.await
.expect("inline rename_data should commit");
// The pre-PUT lookup returns Ok(size 0) for a marker latest, so
// the backfill must match it (see observe_old_current_size docs).
assert_eq!(resp.old_current_size, Some(OldCurrentSize::Present(0)));
}
#[tokio::test]
async fn inline_rename_data_reports_unknown_for_unparsable_dst_meta() {
let dir = tempdir().expect("temp dir should be created");
let disk = test_disk(&dir).await;
let dst_object_dir = dir.path().join("bucket").join("object");
fs::create_dir_all(&dst_object_dir).await.expect("dst dir should be created");
fs::write(dst_object_dir.join(STORAGE_FORMAT_FILE), b"not-an-xl-meta")
.await
.expect("garbage metadata should be written");
let new_fi = test_file_info("object", Uuid::new_v4(), None, Some(Bytes::from_static(b"inline-new")));
let resp = disk
.rename_data(RUSTFS_META_TMP_BUCKET, "tmp-garbage", new_fi, "bucket", "object")
.await
.expect("inline rename_data should commit");
assert_eq!(resp.old_current_size, None);
}
#[tokio::test]
async fn non_inline_rename_data_reports_absent_then_previous_size() {
let dir = tempdir().expect("temp dir should be created");
let disk = test_disk(&dir).await;
// First non-inline commit: fresh key must report Absent.
let first_data_dir = Uuid::new_v4();
let tmp_data_dir = dir
.path()
.join(RUSTFS_META_TMP_BUCKET)
.join("tmp-first")
.join(first_data_dir.to_string());
fs::create_dir_all(&tmp_data_dir)
.await
.expect("tmp data dir should be created");
fs::write(tmp_data_dir.join("part.1"), b"first")
.await
.expect("part should be written");
let mut first_fi = test_file_info("object", Uuid::new_v4(), Some(first_data_dir), None);
first_fi.size = 5;
let resp = disk
.rename_data(RUSTFS_META_TMP_BUCKET, "tmp-first", first_fi, "bucket", "object")
.await
.expect("first non-inline rename_data should commit");
assert_eq!(resp.old_current_size, Some(OldCurrentSize::Absent));
// Overwrite: the committed live version (size 5) must be reported.
let second_data_dir = Uuid::new_v4();
let tmp_data_dir = dir
.path()
.join(RUSTFS_META_TMP_BUCKET)
.join("tmp-second")
.join(second_data_dir.to_string());
fs::create_dir_all(&tmp_data_dir)
.await
.expect("tmp data dir should be created");
fs::write(tmp_data_dir.join("part.1"), b"second-longer")
.await
.expect("part should be written");
let mut second_fi = test_file_info("object", Uuid::new_v4(), Some(second_data_dir), None);
second_fi.size = 13;
let resp = disk
.rename_data(RUSTFS_META_TMP_BUCKET, "tmp-second", second_fi, "bucket", "object")
.await
.expect("second non-inline rename_data should commit");
assert_eq!(resp.old_current_size, Some(OldCurrentSize::Present(5)));
}
/// Twin of the inline unparsable-dst test: the `dst_meta_unparsable`
/// tracking is duplicated per branch, so the non-inline copy needs its
/// own regression coverage.
#[tokio::test]
async fn non_inline_rename_data_reports_unknown_for_unparsable_dst_meta() {
let dir = tempdir().expect("temp dir should be created");
let disk = test_disk(&dir).await;
let dst_object_dir = dir.path().join("bucket").join("object");
fs::create_dir_all(&dst_object_dir).await.expect("dst dir should be created");
fs::write(dst_object_dir.join(STORAGE_FORMAT_FILE), b"not-an-xl-meta")
.await
.expect("garbage metadata should be written");
let data_dir = Uuid::new_v4();
let tmp_data_dir = dir
.path()
.join(RUSTFS_META_TMP_BUCKET)
.join("tmp-garbage-noninline")
.join(data_dir.to_string());
fs::create_dir_all(&tmp_data_dir)
.await
.expect("tmp data dir should be created");
fs::write(tmp_data_dir.join("part.1"), b"payload")
.await
.expect("part should be written");
let mut new_fi = test_file_info("object", Uuid::new_v4(), Some(data_dir), None);
new_fi.size = 7;
let resp = disk
.rename_data(RUSTFS_META_TMP_BUCKET, "tmp-garbage-noninline", new_fi, "bucket", "object")
.await
.expect("non-inline rename_data should commit over garbage metadata");
assert_eq!(resp.old_current_size, None);
}
}
#[tokio::test]
+79
View File
@@ -839,10 +839,43 @@ pub struct DiskOption {
pub health_check: bool,
}
/// Per-disk observation of the destination key's *current* (latest) version
/// in the dst `xl.meta` that `rename_data` reads before it commits the
/// incoming version (rustfs/backlog#1009). Mirrors the pre-PUT
/// `get_object_info` outcome — which the app layer previously obtained with an
/// extra full-disk fanout — bit for bit: `Absent` iff that lookup would report
/// object-not-found (missing xl.meta, or only hidden free versions), and
/// `Present(size)` iff it would return `Ok` with `ObjectInfo.size == size`.
/// Note a delete-marker latest is `Present(0)`, not `Absent`: the lookup
/// returns markers as `Ok(size 0)` and delete-marker accounting never
/// decrements objects_count, so `Present(0)` is what keeps usage numbers
/// identical.
///
/// The parity target is the set-level (`SetDisks`) lookup. Two app-visible
/// lookup variants deviated from it and from each other — the multi-pool
/// `ECStore` path converts a delete-marker latest to not-found, and
/// directory-key lookups pin the nil version — and both deviations
/// over-incremented objects_count relative to delete-marker accounting, so
/// the backfill standardizes on the self-consistent set-level answer.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub enum OldCurrentSize {
/// The pre-PUT lookup would have reported object-not-found for this key.
Absent,
/// The pre-PUT lookup would have returned `Ok` with this `ObjectInfo.size`
/// (0 for a delete-marker latest).
Present(i64),
}
#[derive(Debug, Default, Serialize, Deserialize)]
pub struct RenameDataResp {
pub old_data_dir: Option<Uuid>,
pub sign: Option<Vec<u8>>,
/// `None` means unknown — the disk could not determine the previous
/// current version (pre-#1009 peer on the wire, or an existing dst
/// `xl.meta` that failed to parse). Consumers must treat unknown as
/// "cannot vote", never as `Absent`.
#[serde(default)]
pub old_current_size: Option<OldCurrentSize>,
}
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
@@ -1229,10 +1262,56 @@ mod tests {
let resp = RenameDataResp {
old_data_dir: Some(uuid),
sign: Some(signature.clone()),
old_current_size: Some(OldCurrentSize::Present(42)),
};
assert_eq!(resp.old_data_dir, Some(uuid));
assert_eq!(resp.sign, Some(signature));
assert_eq!(resp.old_current_size, Some(OldCurrentSize::Present(42)));
}
/// rustfs/backlog#1009: `old_current_size` must survive a named-msgpack
/// round trip (the internode RPC encoding) for every variant.
#[test]
fn test_rename_data_resp_old_current_size_msgpack_roundtrip() {
for old_current_size in [None, Some(OldCurrentSize::Absent), Some(OldCurrentSize::Present(1337))] {
let resp = RenameDataResp {
old_data_dir: Some(Uuid::new_v4()),
sign: Some(vec![0x01, 0x02, 0x03]),
old_current_size,
};
let encoded = rmp_serde::encode::to_vec_named(&resp).expect("named msgpack should encode");
let decoded: RenameDataResp = rmp_serde::decode::from_slice(&encoded).expect("named msgpack should decode");
assert_eq!(decoded.old_data_dir, resp.old_data_dir);
assert_eq!(decoded.sign, resp.sign);
assert_eq!(decoded.old_current_size, resp.old_current_size);
}
}
/// rustfs/backlog#1009: a payload from a peer that predates
/// `old_current_size` must decode with the field defaulting to `None`
/// (unknown), keeping mixed-version clusters wire-compatible.
#[test]
fn test_rename_data_resp_decodes_payload_without_old_current_size() {
#[derive(Serialize)]
struct LegacyRenameDataResp {
old_data_dir: Option<Uuid>,
sign: Option<Vec<u8>>,
}
let legacy = LegacyRenameDataResp {
old_data_dir: Some(Uuid::new_v4()),
sign: Some(vec![0x0a, 0x0b]),
};
let encoded = rmp_serde::encode::to_vec_named(&legacy).expect("legacy named msgpack should encode");
let decoded: RenameDataResp = rmp_serde::decode::from_slice(&encoded).expect("legacy payload should decode");
assert_eq!(decoded.old_data_dir, legacy.old_data_dir);
assert_eq!(decoded.sign, legacy.sign);
assert_eq!(decoded.old_current_size, None);
}
/// Test constants