mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-10 23:26:53 +00:00
fix(migration): decrypt MinIO IAM & server config on drop-in migration (#4358)
* fix(migration): decrypt MinIO IAM & server config on drop-in migration MinIO encrypts IAM identity/service-account files and the server config at rest with a key derived from the root credentials. The drop-in migration paths read those blobs from the legacy `.minio.sys` bucket and parsed them as plaintext JSON, so any encrypted blob failed to parse and was silently skipped with "incompatible format". This is why users migrating from MinIO kept their buckets/objects/policies but lost users and access keys (#2212). The IAM load path already knows how to decrypt these blobs (RustFS master keys plus MinIO-compatible legacy keys derived from the root credentials), but that logic lived behind a private method and was never used by the migration paths. Expose it as `rustfs_iam::try_decrypt_iam_blob` and inject it into both migration paths via a `LegacyBlobDecryptFn` callback (ecstore cannot depend on the IAM crate, so the closure is wired in the binary crate). When a blob cannot be decrypted the raw bytes are used as-is, preserving the previous plaintext-only behavior with no regression. Also improve object-layer migration observability without changing control flow: `try_migrate_format` now distinguishes "no legacy format" (a normal fresh install) from "legacy format present but incompatible", and the caller logs a loud error before initializing a fresh format that would leave the existing MinIO objects unreadable. Topology/version skip reasons are promoted from debug to warn. Fixes a pre-existing test isolation race by marking `test_recovery_falls_back_to_default_config_when_blob_stays_corrupt` serial, since it reads a process-wide env var toggled by a sibling test. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(migration): box FormatV3 in LegacyFormatOutcome to satisfy clippy Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): stabilize concurrent multipart resend lock timeout concurrent_resend_same_part_commits_one_generation spawns 6 same-part resends whose cross-disk commits serialize on the per-uploadId commit lock. Under the full nextest suite the parallel disk load pushes those serialized commits past the small default lock-acquire timeout (5s), producing a spurious `Lock(Timeout ...)` unrelated to the property under test (observed on CI at 5.775s vs ~0.5s in isolation). Raise RUSTFS_OBJECT_LOCK_ACQUIRE_TIMEOUT to the production default (30s) for the concurrent-commit section via temp_env, so the regression guard reflects correctness (exactly one intact generation) rather than disk latency under CI load. The meaningful assertions are unchanged, and #[serial] keeps the process-wide env override isolated. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(lock): bound fast-lock notification wait to prevent lost-wakeup stall The real cause of the concurrent_resend_same_part_commits_one_generation failures was a lost wakeup in the fast-lock slow path, not disk latency: raising the acquire timeout to 30s only delayed the failure (it then timed out at 30s), proving a genuine stall rather than overload. In acquire_lock_slow_path a waiter that reaches the notification phase did a single `timeout(remaining, wait_for_write())` spanning the whole acquire budget, and treated that wait's elapse as a hard `Timeout`. But the release path only notifies when `writer_waiters > 0`, so if the holder releases in the gap after the waiter's `try_acquire` fails and before it registers as a waiter, no notification (and no stored permit, since the pooled `Notify` is gated) is produced. The waiter then blocks until the deadline even though the lock is free and stays free — a spurious lock-acquire timeout. The shared process-wide notify pool makes it worse: a wakeup can be consumed by a waiter of a different lock hashing to the same slot. Bound each notification wait (NOTIFY_WAIT_CAP = 50ms) and, on elapse, loop back and re-`try_acquire` instead of returning `Timeout`; the deadline check at the top of the loop is the single source of truth for timing out. A lost/stolen wakeup now degrades to bounded re-polling (acquire within ~50ms of the lock becoming free) instead of stalling for the whole timeout. Correctness (mutual exclusion) is unchanged — acquisition still only happens via `try_acquire_*`. Add a regression test that reproduces the stall (holder + late waiter across many keys): it times out without the fix and passes in ~1s with it. Revert the earlier acquire-timeout workaround in the multipart test now that the underlying stall is fixed, so it runs under the default timeout again. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -169,6 +169,15 @@ impl LockShard {
|
||||
|
||||
let mut retry_count = 0u32;
|
||||
const MAX_RETRIES: u32 = 10;
|
||||
// Upper bound for a single notification wait. The notification pool is
|
||||
// shared process-wide (a fixed set of `Notify` slots hashed by lock), so a
|
||||
// wakeup meant for this lock can be consumed by a waiter of a different
|
||||
// lock that hashes to the same slot, and this lock's waiter would then
|
||||
// sleep until the full deadline. Capping each wait turns that lost-wakeup
|
||||
// into bounded re-polling: on cap elapse we loop and re-`try_acquire`,
|
||||
// only returning `Timeout` once the real deadline passes. The notification
|
||||
// still delivers prompt wakeups in the common (no-collision) case.
|
||||
const NOTIFY_WAIT_CAP: Duration = Duration::from_millis(50);
|
||||
|
||||
loop {
|
||||
// Get or create object state
|
||||
@@ -217,23 +226,25 @@ impl LockShard {
|
||||
}
|
||||
}
|
||||
|
||||
// If we've exhausted quick retries or have little time left, use notification wait
|
||||
// If we've exhausted quick retries or have little time left, use a
|
||||
// notification wait, but bounded by NOTIFY_WAIT_CAP so a lost/stolen
|
||||
// wakeup cannot strand this waiter until the deadline.
|
||||
let wait = remaining.min(NOTIFY_WAIT_CAP);
|
||||
let wait_result = match request.mode {
|
||||
LockMode::Shared => {
|
||||
let _waiter_guard = WaiterCounterGuard::new(state.clone(), LockMode::Shared);
|
||||
timeout(remaining, state.optimized_notify.wait_for_read()).await
|
||||
timeout(wait, state.optimized_notify.wait_for_read()).await
|
||||
}
|
||||
LockMode::Exclusive => {
|
||||
let _waiter_guard = WaiterCounterGuard::new(state.clone(), LockMode::Exclusive);
|
||||
timeout(remaining, state.optimized_notify.wait_for_write()).await
|
||||
timeout(wait, state.optimized_notify.wait_for_write()).await
|
||||
}
|
||||
};
|
||||
|
||||
if wait_result.is_err() {
|
||||
self.metrics.record_timeout();
|
||||
return Err(LockResult::Timeout);
|
||||
}
|
||||
|
||||
// A capped-wait elapse is not a real timeout: loop back and re-try the
|
||||
// acquisition. The deadline check at the top of the loop is the single
|
||||
// source of truth for returning `Timeout`.
|
||||
let _ = wait_result;
|
||||
retry_count += 1;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -410,6 +410,66 @@ mod fast_lock_tests {
|
||||
}
|
||||
}
|
||||
|
||||
// Regression for the fast-lock lost-wakeup (backlog#853 follow-up).
|
||||
//
|
||||
// A waiter that enters the notification wait can miss its wakeup: the release
|
||||
// path only calls `notify_one` when `writer_waiters > 0`, so if the holder
|
||||
// releases in the narrow gap after the waiter's `try_acquire` fails but before
|
||||
// it registers as a waiter, no notification (and no stored permit) is produced.
|
||||
// The waiter then blocks until the acquire deadline even though the lock is free
|
||||
// and stays free — surfacing as a spurious `LockResult::Timeout`.
|
||||
//
|
||||
// Each key here has one long holder plus one waiter that starts slightly later,
|
||||
// so the waiter is pushed past the early backoff phase into the notification
|
||||
// wait and there is no re-contention after the single release. With the fix
|
||||
// (bounded notification wait + re-poll) the waiter acquires within ~50ms of the
|
||||
// release; without it, the missed wakeup strands the waiter until timeout.
|
||||
// Many independent keys make hitting the narrow race reliable.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn write_lock_waiter_is_not_stranded_by_missed_wakeup() {
|
||||
let manager = Arc::new(create_test_manager());
|
||||
const KEYS: usize = 64;
|
||||
// Long enough to push the waiter past the ~850ms backoff phase into the
|
||||
// notification wait, where the missed-wakeup bug lives.
|
||||
const HOLDER_HOLD: Duration = Duration::from_millis(950);
|
||||
|
||||
let mut handles = Vec::new();
|
||||
for k in 0..KEYS {
|
||||
let key = ObjectKey::new("bucket", format!("object-{k}"));
|
||||
|
||||
// Holder: grabs the lock immediately and holds it across the waiter's
|
||||
// backoff-to-notification transition, then releases exactly once.
|
||||
let holder_mgr = manager.clone();
|
||||
let holder_key = key.clone();
|
||||
handles.push(tokio::spawn(async move {
|
||||
let mut guard = holder_mgr
|
||||
.acquire_write_lock(holder_key, "holder")
|
||||
.await
|
||||
.expect("holder should acquire immediately");
|
||||
sleep(HOLDER_HOLD).await;
|
||||
assert!(guard.release());
|
||||
}));
|
||||
|
||||
// Waiter: starts a touch later so the holder wins the lock first, then
|
||||
// must survive the whole hold and acquire promptly after the release.
|
||||
let waiter_mgr = manager.clone();
|
||||
let waiter_key = key.clone();
|
||||
handles.push(tokio::spawn(async move {
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
let request = ObjectLockRequest::new_write(waiter_key, "waiter").with_acquire_timeout(Duration::from_secs(5));
|
||||
let mut guard = waiter_mgr
|
||||
.acquire_lock(request)
|
||||
.await
|
||||
.expect("waiter must acquire after the holder releases, not time out on a missed wakeup");
|
||||
assert!(guard.release());
|
||||
}));
|
||||
}
|
||||
|
||||
for handle in handles {
|
||||
handle.await.expect("lock task should not panic");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_lock_timeout() {
|
||||
let manager = create_test_manager();
|
||||
|
||||
Reference in New Issue
Block a user