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

fix(multipart): don't pre-delete the committed part before rename_part (backlog#853) (#4316)

Browse Source 
`rename_part` unconditionally ran `cleanup_multipart_path([part.N, part.N.meta])`
on every disk *before* the per-disk rename fan-out. The per-disk `rename_part`
already replaces `part.N` atomically (std::fs::rename) and rewrites `part.N.meta`,
so the pre-delete is redundant — and destructive: it removed an already-committed
(ACKed) part on all disks before the new rename landed, so a re-upload of the same
part that then failed write quorum destroyed the committed part outright.

Remove the pre-rename cleanup and rely on the atomic rename to overwrite in place;
the quorum-failure rollback below is unchanged. No behaviour change on the success
paths (first upload / retry both end with part.N replaced), verified by the
existing multipart suite.

Refs backlog#799 (B4). The remaining half of B4 — serializing concurrent
same-part uploads with an uploadId-scoped lock so two generations can't be mixed
across disks — is a larger concurrency change tracked as a follow-up on the issue.
This commit is contained in:
Zhengchao An
2026-07-06 22:08:50 +08:00
committed by GitHub
parent 5e73d59eb6
commit 8ffe230a89
1 changed files with 8 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ecstore/src/set_disk/core/io_primitives.rs
+8 -4
View File
@@ -2688,10 +2688,14 @@ impl SetDisks {
let dst_bucket = Arc::new(dst_bucket.to_string());
let dst_object = Arc::new(dst_object.to_string());
// Match MinIO's multipart overwrite semantics: clear any stale destination
// part payload and metadata before the new per-disk rename fan-out begins.
self.cleanup_multipart_path(&[dst_object.to_string(), format!("{dst_object}.meta")])
.await;
// Do NOT pre-delete the destination part before renaming: the per-disk
// `rename_part` replaces `part.N` atomically (std::fs::rename) and rewrites
// `part.N.meta`, so the pre-delete is redundant — and destructive. It
// opened a window where an already-committed (ACKed) part was removed on
// every disk before the new rename landed, so a re-upload that then failed
// quorum destroyed the committed part outright (backlog#853 / #799 B4).
// The atomic rename overwrites in place; on quorum failure below we roll
// the destination back.
let mut errs = Vec::with_capacity(disks.len());