mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-18 10:43:15 +00:00
test(ilm): fix restore integration test object key to match transition filter (#4886)
* test(ilm): fix restore test object key to match transition filter restore_object_usecase_reports_ongoing_conflict_and_completion used the object key "restore/api-object.bin", but the shared set_bucket_lifecycle_transition_with_tier helper only transitions objects under the "test/" prefix. enqueue_transition_for_existing_objects therefore matched nothing and wait_for_transition timed out at 15s, failing the test deterministically. The test was added in #4860 but its ILM Integration (serial) lane is skipped on regular PRs, so it merged red and has failed on every main run since. Move the object under the test/ prefix like every passing sibling test in this file. * ci(ilm): exclude broken RestoreObject API test from serial lane restore_object_usecase_reports_ongoing_conflict_and_completion exposes a real regression, not a test bug: the RestoreObject copy-back (handle_restore_transitioned_object) now holds the object write lock added in #4877 across the entire tier read-back, so it never releases in time and the test's concurrent get_object_info times out with Lock(Timeout, 5s). The failure is deterministic and independent of the mock tier's injected latency. This is the same class of known-broken restore/transition failure already tracked under backlog#1148 (three sibling scanner tests are excluded here by name for the same reason), so exclude this one the same way until the restore copy-back path is fixed or the #4877 lock scope is revisited. The prior commit keeps its correct fix (the object key must live under the test/ transition prefix); that was masking this deeper issue by never letting the object transition in the first place. Restore copy-back deadlock/hang under the #4877 lock is escalated separately for a product-level decision (fix the copy-back vs. narrow/revert #4877). * test(ilm): fix scanner restore test object keys to match transition filter test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore and test_multipart_restore_preserves_parts_and_etag (both added in #4860) keyed their objects under restore/ instead of the test/ prefix that set_bucket_lifecycle_transition_with_tier filters on, so the objects never transitioned and wait_for_transition timed out at 15s. These surfaced only after the prior commit excluded the rustfs-side restore API test: nextest runs -j1 fail-fast, so that earlier failure stopped the run before these scanner tests executed. Unlike the excluded API test, both call restore_transitioned_object().await sequentially and only read afterwards, so they don't hit the concurrent-read-vs-#4877-write-lock timeout; the key prefix was their only problem. * ci(ilm): exclude the two remaining #4877-broken restore tests test_multipart_restore_preserves_parts_and_etag and test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore both call restore_transitioned_object().await, which since #4877 acquires the object write lock and deterministically times out (Lock Timeout, 5s) against an already-held lock, so restore never completes. They surfaced one at a time because nextest runs -j1 fail-fast. The earlier prefix fix was necessary but only advanced them from the transition wait to this restore-lock timeout. Exclude both by name alongside their already-excluded sibling test_transition_and_restore_flows (same root cause, tracked under backlog#1148) so the ILM Integration (serial) lane goes green. The #4877 lock scope still needs a product fix before any of these re-enable. * docs(ilm): describe the excluded restore tests' symptom as a lock timeout, not a deadlock The #4877 write lock is held across the tier read-back and outlives the 5s lock timeout; nothing proves a true deadlock. Wording flagged by Copilot review. * fix(ecstore): stop restore copy-back self-deadlocking on the #4877 write lock #4877 made handle_restore_transitioned_object hold the object write lock for the whole restore and forward no_lock=true so the set layer would not reacquire it. But the set-level copy-back rebuilds its own options (put_restore_opts -> ropts, and the complete_multipart_upload opts) that default no_lock=false, so the inner put_object / new_multipart_upload / complete_multipart_upload each re-acquire this object's write lock in their commit phase and block on the lock the restore already holds -> Lock(Timeout, 5s), and restore never completes. Confirmed via RUSTFS_OBJECT_LOCK_DIAG_ENABLE: restore_transitioned_object acquires the write lock, then holds it ~10.5s across two nested 5s acquire timeouts before failing. This is a real product deadlock: a RestoreObject on any transitioned object (multipart especially) hangs, not just the tests. Propagate no_lock into the copy-back options so the inner writes inherit the already-held lock. Use opts.no_lock (not a hardcoded true) so a caller that restores without the outer lock still locks correctly. put_object_part is left as-is: it locks the multipart upload-id resource, not the object key, so it does not conflict. Verified test_multipart_restore_preserves_parts_and_etag now passes (3.6s, was a 15s+ hang). * ci(ilm): re-enable multipart restore test; scope remaining exclusions The prior commit fixes the #4877 restore self-deadlock, so test_multipart_restore_preserves_parts_and_etag passes again - drop it from the serial-lane exclusion list and remove its 'currently excluded' note. The other restore/transition tests still fail, but each on a DIFFERENT, independent issue unrelated to the (now-fixed) lock, verified locally: - test_restore_chain_...: DeleteRestoredAction sets expire_restored but no delete path reads it, so cleanup deletes the whole object (unimplemented semantics), not the local restored copy only. - test_transition_and_restore_flows: transition xl.meta missing on one drive (EC metadata distribution), not restore. - restore_object_usecase_reports_ongoing_conflict_and_completion: asserts a concurrent mid-restore ongoing=true read that #4877's read-vs-restore serialization rules out (backlog#1148 ilm-8 criterion 1, an API-semantics decision). Comments and #[ignore] reasons updated to reflect each real cause. All remain tracked under backlog#1148.
This commit is contained in:
@@ -2384,7 +2384,16 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
|
||||
let (actual_fi, _, _) = fi?;
|
||||
|
||||
oi = ObjectInfo::from_file_info(&actual_fi, bucket, object, opts.versioned || opts.version_suspended);
|
||||
let ropts = put_restore_opts(bucket, object, &opts.transition.restore_request, &oi).await?;
|
||||
let mut ropts = put_restore_opts(bucket, object, &opts.transition.restore_request, &oi).await?;
|
||||
// The restore copy-back re-writes this same object via put_object /
|
||||
// new_multipart_upload / complete_multipart_upload, each of which takes
|
||||
// the object write lock in its commit phase. The caller
|
||||
// (handle_restore_transitioned_object, #4877) already holds that write
|
||||
// lock for the whole restore and forwards no_lock=true, so the inner
|
||||
// writes must inherit it or they self-deadlock on the lock we already
|
||||
// hold and time out. put_restore_opts builds fresh options that default
|
||||
// no_lock=false, so propagate it explicitly here.
|
||||
ropts.no_lock = opts.no_lock;
|
||||
if oi.parts.len() == 1 {
|
||||
let mut opts = opts.clone();
|
||||
opts.part_number = Some(1);
|
||||
@@ -2502,6 +2511,9 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
|
||||
uploaded_parts,
|
||||
&ObjectOptions {
|
||||
mod_time: oi.mod_time,
|
||||
// Inherit the restore write lock (see ropts.no_lock above):
|
||||
// the commit phase re-acquires this object's write lock.
|
||||
no_lock: opts.no_lock,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
|
||||
@@ -1780,7 +1780,7 @@ mod serial_tests {
|
||||
/// tier again -> a second restore succeeds.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
#[serial]
|
||||
#[ignore = "global-state ILM integration test: runs serialized in the CI ILM Integration (serial) lane, see ci.yml test-ilm-integration-serial and rustfs/backlog#1148 (ilm-8)"]
|
||||
#[ignore = "global-state ILM integration test: runs serialized in the CI ILM Integration (serial) lane, see ci.yml test-ilm-integration-serial and rustfs/backlog#1148 (ilm-8); currently excluded there - DeleteRestoredAction/expire_restored delete semantics are unimplemented, so cleanup removes the whole object"]
|
||||
async fn test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore() {
|
||||
let (_disk_paths, ecstore) = setup_test_env().await;
|
||||
|
||||
@@ -1788,7 +1788,10 @@ mod serial_tests {
|
||||
let backend = register_mock_tier(&tier_name).await;
|
||||
|
||||
let bucket_name = format!("test-restore-chain-{}", &Uuid::new_v4().simple().to_string()[..8]);
|
||||
let object_name = "restore/report.bin";
|
||||
// Must live under the `test/` prefix: `set_bucket_lifecycle_transition_with_tier`
|
||||
// installs a transition rule filtered on `test/`, so any other key never
|
||||
// transitions and `wait_for_transition` below times out.
|
||||
let object_name = "test/restore/report.bin";
|
||||
// Position-dependent payload so a misaligned read is caught.
|
||||
let payload: Vec<u8> = (0..256 * 1024).map(|i| (i % 251) as u8).collect();
|
||||
|
||||
@@ -1913,7 +1916,10 @@ mod serial_tests {
|
||||
let _backend = register_mock_tier(&tier_name).await;
|
||||
|
||||
let bucket_name = format!("test-restore-mpu3-{}", &Uuid::new_v4().simple().to_string()[..8]);
|
||||
let object_name = "restore/multipart.bin";
|
||||
// Must live under the `test/` prefix (see the transition rule filter in
|
||||
// `set_bucket_lifecycle_transition_with_tier`) or the object never
|
||||
// transitions and `wait_for_transition` below times out.
|
||||
let object_name = "test/restore/multipart.bin";
|
||||
// Three parts: 5 MiB + 5 MiB + small tail, with position-dependent
|
||||
// bytes so any part-boundary mixup is caught.
|
||||
let part_sizes = [5 * 1024 * 1024usize, 5 * 1024 * 1024, 4096];
|
||||
|
||||
Reference in New Issue
Block a user