fix(lock): fence write commit on lock loss (#4406)

fix(lock): fence write commit on lock loss (backlog#899 Phase 2)

Phase 0+1 (#4388) made object write locks refreshable and marks the guard
lost when the heartbeat can no longer refresh a quorum, but does not act on
it. Under a partition a long write's lock can expire on an unreachable node
and be reclaimed, letting a third party re-acquire it; the original writer
keeps going and both commit -- a double write.

Expose the loss signal through NamespaceLockGuard::is_lock_lost() and
ObjectLockDiagGuard::is_lock_lost(), and fence the commit in put_object and
complete_multipart_upload: immediately before rename_data (the atomic commit
point), abort with a retryable NamespaceLockQuorumUnavailable (503) if the
lock was lost. In multipart the check precedes cleanup_multipart_path so a
lost lock leaves the upload intact and retryable. A write that already
reached rename_data Ok is durable and never aborted.

The loss criterion is unchanged (reacts to Phase 1's signal). Heal and the
long-GET read side are deferred follow-ups.
This commit is contained in:
Zhengchao An
2026-07-08 15:01:24 +08:00
committed by GitHub
parent f327707321
commit 1e6207c08e
5 changed files with 74 additions and 0 deletions
+7
View File
@@ -254,6 +254,13 @@ impl ObjectLockDiagGuard {
acquired_at: Instant::now(),
}
}
/// Whether the underlying namespace lock's heartbeat has observed a
/// refresh-quorum loss (backlog#899 Phase 2). Callers fence their commit
/// point on this so a stale lock holder does not race a double-write.
fn is_lock_lost(&self) -> bool {
self.guard.is_lock_lost()
}
}
impl Drop for ObjectLockDiagGuard {
@@ -1342,6 +1342,22 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
);
}
// Phase 2 (backlog#899): fence the commit on lock loss before any destructive
// step. If the refresh heartbeat has observed a refresh-quorum loss, another
// writer may have re-acquired this object's lock; proceeding would race a
// double-write. Abort with a retryable error *before* cleanup_multipart_path
// (which removes the source parts) and rename_data (which commits the object),
// so a lost lock leaves the upload intact and retryable.
if object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) {
return Err(StorageError::NamespaceLockQuorumUnavailable {
mode: "complete_multipart_upload_commit",
bucket: bucket.to_string(),
object: object.to_string(),
required: 1,
achieved: 0,
});
}
let complete_tail_stage_start = rustfs_io_metrics::put_stage_metrics_enabled().then(Instant::now);
self.cleanup_multipart_path(&parts).await;
+16
View File
@@ -864,6 +864,22 @@ impl crate::storage_api_contracts::object::ObjectIO for SetDisks {
object_lock_guard = Some(self.acquire_write_lock_diag("put_object_commit", bucket, object).await?);
}
// Phase 2 (backlog#899): fence the commit on lock loss. If the refresh
// heartbeat has observed a refresh-quorum loss, another writer may have
// re-acquired this object's lock; committing now would race a double-write.
// Abort with a retryable error *before* rename_data makes the new version
// durable — once rename_data returns Ok the write is committed and must
// never be aborted (that would violate "a committed write is not lost").
if object_lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) {
return Err(StorageError::NamespaceLockQuorumUnavailable {
mode: "put_object_commit",
bucket: bucket.to_string(),
object: object.to_string(),
required: 1,
achieved: 0,
});
}
let rename_stage_start = Instant::now();
let (online_disks, _, op_old_dir, cleanup_disks) = Self::rename_data(
&shuffle_disks,