mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-21 11:56:38 +00:00
fix: resolve release-blocking integration failures (#6320)
* fix: resolve release-blocking integration failures * fix: satisfy stable clippy lints * fix: satisfy Rust 1.98 CI lints
This commit is contained in:
@@ -1968,7 +1968,7 @@ impl TargetClient {
|
||||
bucket: &str,
|
||||
object: &str,
|
||||
version_id: Option<String>,
|
||||
) -> Result<HeadObjectOutput, SdkError<HeadObjectError>> {
|
||||
) -> Result<HeadObjectOutput, Box<SdkError<HeadObjectError>>> {
|
||||
// Announce the replication check so a RustFS target returns SSE-C
|
||||
// object metadata (etag/size) without the customer key the replication
|
||||
// worker cannot hold; otherwise SSE-C replicas never converge on HEAD.
|
||||
@@ -1981,8 +1981,7 @@ impl TargetClient {
|
||||
// object with an identical ETag, and the worker concludes the object
|
||||
// already converged — so it never actually replicates it.
|
||||
insert_header(&mut headers, SUFFIX_SOURCE_PROXY_REQUEST, "false");
|
||||
match self
|
||||
.client
|
||||
self.client
|
||||
.head_object()
|
||||
.bucket(bucket)
|
||||
.key(object)
|
||||
@@ -1999,10 +1998,7 @@ impl TargetClient {
|
||||
})
|
||||
.send()
|
||||
.await
|
||||
{
|
||||
Ok(res) => Ok(res),
|
||||
Err(e) => Err(e),
|
||||
}
|
||||
.map_err(Box::new)
|
||||
}
|
||||
|
||||
/// HEAD used by the read-proxy path (GET/HEAD of an object not yet
|
||||
@@ -2023,7 +2019,7 @@ impl TargetClient {
|
||||
range: Option<String>,
|
||||
part_number: Option<i32>,
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<HeadObjectOutput, SdkError<HeadObjectError>> {
|
||||
) -> Result<HeadObjectOutput, Box<SdkError<HeadObjectError>>> {
|
||||
let headers = proxy_outbound_headers(extra_headers);
|
||||
self.client
|
||||
.head_object()
|
||||
@@ -2036,6 +2032,7 @@ impl TargetClient {
|
||||
.map_request(move |req| apply_extra_headers(req, &headers))
|
||||
.send()
|
||||
.await
|
||||
.map_err(Box::new)
|
||||
}
|
||||
|
||||
/// GET used by the read-proxy path (MinIO `proxyGetToReplicationTarget`).
|
||||
@@ -2051,7 +2048,7 @@ impl TargetClient {
|
||||
range: Option<String>,
|
||||
part_number: Option<i32>,
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<GetObjectOutput, SdkError<GetObjectError>> {
|
||||
) -> Result<GetObjectOutput, Box<SdkError<GetObjectError>>> {
|
||||
let headers = proxy_outbound_headers(extra_headers);
|
||||
self.client
|
||||
.get_object()
|
||||
@@ -2064,6 +2061,7 @@ impl TargetClient {
|
||||
.map_request(move |req| apply_extra_headers(req, &headers))
|
||||
.send()
|
||||
.await
|
||||
.map_err(Box::new)
|
||||
}
|
||||
|
||||
/// GetObjectTagging for the tagging read-proxy path
|
||||
@@ -2073,7 +2071,7 @@ impl TargetClient {
|
||||
bucket: &str,
|
||||
object: &str,
|
||||
version_id: Option<String>,
|
||||
) -> Result<GetObjectTaggingOutput, SdkError<GetObjectTaggingError>> {
|
||||
) -> Result<GetObjectTaggingOutput, Box<SdkError<GetObjectTaggingError>>> {
|
||||
let headers = proxy_outbound_headers(HeaderMap::new());
|
||||
self.client
|
||||
.get_object_tagging()
|
||||
@@ -2084,6 +2082,7 @@ impl TargetClient {
|
||||
.map_request(move |req| apply_extra_headers(req, &headers))
|
||||
.send()
|
||||
.await
|
||||
.map_err(Box::new)
|
||||
}
|
||||
|
||||
/// PutObjectTagging for the tagging proxy path
|
||||
@@ -2094,7 +2093,7 @@ impl TargetClient {
|
||||
object: &str,
|
||||
version_id: Option<String>,
|
||||
tagging: SdkTagging,
|
||||
) -> Result<PutObjectTaggingOutput, SdkError<PutObjectTaggingError>> {
|
||||
) -> Result<PutObjectTaggingOutput, Box<SdkError<PutObjectTaggingError>>> {
|
||||
let headers = proxy_outbound_headers(HeaderMap::new());
|
||||
self.client
|
||||
.put_object_tagging()
|
||||
@@ -2106,6 +2105,7 @@ impl TargetClient {
|
||||
.map_request(move |req| apply_extra_headers(req, &headers))
|
||||
.send()
|
||||
.await
|
||||
.map_err(Box::new)
|
||||
}
|
||||
|
||||
/// DeleteObjectTagging for the tagging proxy path
|
||||
@@ -2115,7 +2115,7 @@ impl TargetClient {
|
||||
bucket: &str,
|
||||
object: &str,
|
||||
version_id: Option<String>,
|
||||
) -> Result<DeleteObjectTaggingOutput, SdkError<DeleteObjectTaggingError>> {
|
||||
) -> Result<DeleteObjectTaggingOutput, Box<SdkError<DeleteObjectTaggingError>>> {
|
||||
let headers = proxy_outbound_headers(HeaderMap::new());
|
||||
self.client
|
||||
.delete_object_tagging()
|
||||
@@ -2126,6 +2126,7 @@ impl TargetClient {
|
||||
.map_request(move |req| apply_extra_headers(req, &headers))
|
||||
.send()
|
||||
.await
|
||||
.map_err(Box::new)
|
||||
}
|
||||
|
||||
/// On success returns the version id the target assigned (from
|
||||
|
||||
@@ -2180,7 +2180,7 @@ pub async fn recover_manual_transition_jobs_once(
|
||||
if limit == 0 {
|
||||
return Err(Error::other("manual transition job recovery limit must be greater than zero"));
|
||||
}
|
||||
let list_limit = i32::try_from(limit).map_or(i32::MAX, |value| value);
|
||||
let list_limit = i32::try_from(limit).unwrap_or(i32::MAX);
|
||||
let page = api
|
||||
.clone()
|
||||
.list_objects_v2(
|
||||
@@ -2386,7 +2386,7 @@ async fn replay_manual_transition_pending_tasks(
|
||||
version_id: task.version_id,
|
||||
etag: task.etag,
|
||||
mod_time,
|
||||
size: task.size.map_or(0, |size| size),
|
||||
size: task.size.unwrap_or(0),
|
||||
is_latest: task.is_latest.unwrap_or(false),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -1016,7 +1016,7 @@ pub async fn recover_transition_transaction_records(
|
||||
return Err(Error::other("transition transaction recovery limit must be greater than zero"));
|
||||
}
|
||||
|
||||
let list_limit = i32::try_from(limit).map_or(i32::MAX, |value| value);
|
||||
let list_limit = i32::try_from(limit).unwrap_or(i32::MAX);
|
||||
let list = api
|
||||
.clone()
|
||||
.list_objects_v2(
|
||||
|
||||
@@ -76,7 +76,12 @@ impl QuotaChecker {
|
||||
|
||||
let current_usage = self.get_real_time_usage(bucket).await?;
|
||||
|
||||
let admission_size = if uses_durable_reservations { 0 } else { operation_size };
|
||||
// The reporting path projects this operation; storage mutations reserve it at commit.
|
||||
let admission_size = if uses_durable_reservations && !force_usage_calculation {
|
||||
0
|
||||
} else {
|
||||
operation_size
|
||||
};
|
||||
let expected_usage = match operation {
|
||||
QuotaOperation::PutObject | QuotaOperation::PostObject | QuotaOperation::CopyObject => {
|
||||
current_usage.saturating_add(admission_size)
|
||||
|
||||
@@ -214,7 +214,7 @@ async fn head_object_for_worker(
|
||||
target_bucket: &str,
|
||||
object: &str,
|
||||
version_id: Option<String>,
|
||||
) -> std::result::Result<HeadObjectOutput, SdkError<HeadObjectError>> {
|
||||
) -> std::result::Result<HeadObjectOutput, Box<SdkError<HeadObjectError>>> {
|
||||
target_client.head_object(target_bucket, object, version_id).await
|
||||
}
|
||||
|
||||
@@ -233,7 +233,7 @@ async fn mark_replication_target_offline_if_needed(target_client: &Arc<TargetCli
|
||||
async fn head_object_fallback(
|
||||
tgt_client: &TargetClient,
|
||||
object: &str,
|
||||
) -> std::result::Result<Option<HeadObjectOutput>, SdkError<HeadObjectError>> {
|
||||
) -> std::result::Result<Option<HeadObjectOutput>, Box<SdkError<HeadObjectError>>> {
|
||||
match head_object_for_worker(tgt_client, &tgt_client.bucket, object, None).await {
|
||||
Ok(oi) => Ok(Some(oi)),
|
||||
Err(e) if e.as_service_error().is_some_and(|se| se.is_not_found()) || has_raw_status(&e, 404) => Ok(None),
|
||||
@@ -1152,11 +1152,11 @@ fn spawn_resync_walk_task<S: ReplicationStorage>(
|
||||
/// updating the per-object status counters and returning the accounted size
|
||||
/// together with any verification error.
|
||||
async fn verify_resync_head_result(
|
||||
head_result: std::result::Result<HeadObjectOutput, SdkError<HeadObjectError>>,
|
||||
head_result: std::result::Result<HeadObjectOutput, Box<SdkError<HeadObjectError>>>,
|
||||
roi: &ReplicateObjectInfo,
|
||||
st: &mut TargetReplicationResyncStatus,
|
||||
target_client: &Arc<TargetClient>,
|
||||
) -> (i64, Option<SdkError<HeadObjectError>>) {
|
||||
) -> (i64, Option<Box<SdkError<HeadObjectError>>>) {
|
||||
match head_result {
|
||||
Ok(_) => {
|
||||
st.replicated_count += 1;
|
||||
@@ -1275,7 +1275,7 @@ async fn resync_worker_process_object<S: ReplicationStorage>(
|
||||
"Processed resync object"
|
||||
);
|
||||
}
|
||||
st.error = err.as_ref().and_then(resync_target_error_detail);
|
||||
st.error = err.as_ref().and_then(|err| resync_target_error_detail(err));
|
||||
|
||||
st
|
||||
}
|
||||
@@ -2467,7 +2467,7 @@ async fn replicate_delete_to_target(dobj: &DeletedObjectReplicationInfo, tgt_cli
|
||||
Ok(_) => {}
|
||||
Err(e) => {
|
||||
let non_retryable = matches!(
|
||||
&e,
|
||||
&*e,
|
||||
SdkError::ServiceError(service_err)
|
||||
if is_retryable_delete_replication_head_error(
|
||||
service_err.err().is_not_found(),
|
||||
|
||||
@@ -657,7 +657,7 @@ where
|
||||
prefix,
|
||||
marker,
|
||||
None,
|
||||
i32::try_from(limit).map_or(i32::MAX, |value| value),
|
||||
i32::try_from(limit).unwrap_or(i32::MAX),
|
||||
false,
|
||||
None,
|
||||
false,
|
||||
|
||||
@@ -1322,7 +1322,12 @@ impl crate::storage_api_contracts::object::ObjectIO for SetDisks {
|
||||
let object_info = prepared_object_info
|
||||
.unwrap_or_else(|| build_get_object_info(fi, bucket, object, opts.versioned || opts.version_suspended));
|
||||
let object_class = classify_get_codec_streaming_object_class(&range, &object_info, fi);
|
||||
let size_bucket = rustfs_io_metrics::get_object_size_bucket(object_info.size);
|
||||
let metrics_size = if stage_metrics_enabled {
|
||||
object_info.get_actual_size().unwrap_or(object_info.size)
|
||||
} else {
|
||||
object_info.size
|
||||
};
|
||||
let size_bucket = rustfs_io_metrics::get_object_size_bucket(metrics_size);
|
||||
record_get_stage_duration_if_enabled(GET_OBJECT_PATH_SET_DISK, GET_STAGE_OBJECT_INFO, object_info_stage_start);
|
||||
let metadata_elapsed = metadata_stage_start.elapsed().as_secs_f64();
|
||||
rustfs_io_metrics::record_get_object_metadata_phase_duration(metadata_elapsed);
|
||||
@@ -3766,7 +3771,7 @@ pub(crate) async fn complete_transition_upload<Remote, Producer>(
|
||||
producer: Producer,
|
||||
expected_size: u64,
|
||||
consumed: Arc<AtomicU64>,
|
||||
) -> std::result::Result<TransitionUploadCompletion, TransitionUploadFailure>
|
||||
) -> std::result::Result<TransitionUploadCompletion, Box<TransitionUploadFailure>>
|
||||
where
|
||||
Remote: Future<Output = std::result::Result<String, std::io::Error>>,
|
||||
Producer: Future<Output = Result<u64>>,
|
||||
@@ -3784,23 +3789,23 @@ where
|
||||
Err(_) => StorageError::Unexpected,
|
||||
Ok(Ok(_)) => StorageError::Io(remote_error),
|
||||
};
|
||||
return Err(TransitionUploadFailure { error, candidate: None });
|
||||
return Err(Box::new(TransitionUploadFailure { error, candidate: None }));
|
||||
}
|
||||
};
|
||||
let candidate = TransitionUploadCandidate::from_put_response(remote_version);
|
||||
let produced = match producer_result {
|
||||
Ok(Ok(produced)) => produced,
|
||||
Ok(Err(error)) => {
|
||||
return Err(TransitionUploadFailure {
|
||||
return Err(Box::new(TransitionUploadFailure {
|
||||
error,
|
||||
candidate: Some(candidate),
|
||||
});
|
||||
}));
|
||||
}
|
||||
Err(_) => {
|
||||
return Err(TransitionUploadFailure {
|
||||
return Err(Box::new(TransitionUploadFailure {
|
||||
error: StorageError::Unexpected,
|
||||
candidate: Some(candidate),
|
||||
});
|
||||
}));
|
||||
}
|
||||
};
|
||||
let consumed = consumed.load(Ordering::Acquire);
|
||||
@@ -3810,10 +3815,10 @@ where
|
||||
} else {
|
||||
StorageError::MoreData
|
||||
};
|
||||
return Err(TransitionUploadFailure {
|
||||
return Err(Box::new(TransitionUploadFailure {
|
||||
error,
|
||||
candidate: Some(candidate),
|
||||
});
|
||||
}));
|
||||
}
|
||||
Ok(TransitionUploadCompletion {
|
||||
candidate,
|
||||
@@ -7284,7 +7289,7 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
|
||||
}
|
||||
let gr = gr?;
|
||||
let reader = BufReader::new(gr.stream);
|
||||
let hash_reader = HashReader::from_stream(reader, gr.object_info.size, gr.object_info.size, None, None, false)?;
|
||||
let hash_reader = HashReader::from_stream(reader, gr.object_info.size, oi.get_actual_size()?, None, None, false)?;
|
||||
let mut p_reader = PutObjReader::new(hash_reader);
|
||||
return match self_.clone().put_object(bucket, object, &mut p_reader, &ropts).await {
|
||||
Ok(restored_info) => {
|
||||
@@ -8826,7 +8831,7 @@ mod transition_commit_failure_tests {
|
||||
use s3s::dto::RestoreRequest;
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
||||
|
||||
fn restore_operation_id_metadata(operation_id: Uuid) -> HashMap<String, String> {
|
||||
pub(super) fn restore_operation_id_metadata(operation_id: Uuid) -> HashMap<String, String> {
|
||||
let mut metadata = HashMap::new();
|
||||
rustfs_utils::http::metadata_compat::insert_str(
|
||||
&mut metadata,
|
||||
@@ -8836,7 +8841,7 @@ mod transition_commit_failure_tests {
|
||||
metadata
|
||||
}
|
||||
|
||||
fn restore_metadata(operation_id: Uuid, ongoing: bool) -> HashMap<String, String> {
|
||||
pub(super) fn restore_metadata(operation_id: Uuid, ongoing: bool) -> HashMap<String, String> {
|
||||
let mut metadata = restore_operation_id_metadata(operation_id);
|
||||
metadata.insert(s3s::header::X_AMZ_RESTORE.as_str().to_string(), format!("ongoing-request=\"{ongoing}\""));
|
||||
metadata
|
||||
@@ -10097,6 +10102,51 @@ mod transition_commit_failure_tests {
|
||||
.await
|
||||
.expect("operation B should replace operation A before final commit");
|
||||
|
||||
let mismatch = set_disks
|
||||
.finalize_restore_metadata(
|
||||
bucket,
|
||||
object,
|
||||
&set_disks
|
||||
.get_object_info(bucket, object, &ObjectOptions::default())
|
||||
.await
|
||||
.expect("operation B metadata should be readable"),
|
||||
&ObjectOptions {
|
||||
user_defined: restore_operation_id_metadata(operation_a),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect_err("operation A must not finalize operation B metadata");
|
||||
assert!(matches!(
|
||||
mismatch,
|
||||
Error::Io(ref error)
|
||||
if error.kind() == std::io::ErrorKind::Other
|
||||
&& error.to_string() == "restore operation id changed before metadata finalization"
|
||||
));
|
||||
let current = set_disks
|
||||
.get_object_info(bucket, object, &ObjectOptions::default())
|
||||
.await
|
||||
.expect("operation B metadata should remain after mismatched finalization");
|
||||
assert_eq!(
|
||||
rustfs_utils::http::metadata_compat::get_consistent_str(
|
||||
current.user_defined.as_ref(),
|
||||
rustfs_utils::http::metadata_compat::SUFFIX_RESTORE_OPERATION_ID,
|
||||
),
|
||||
Some(operation_b.to_string().as_str()),
|
||||
"mismatched finalization must not remove operation B"
|
||||
);
|
||||
assert!(
|
||||
parse_restore_obj_status(
|
||||
current
|
||||
.user_defined
|
||||
.get(s3s::header::X_AMZ_RESTORE.as_str())
|
||||
.expect("operation B restore header should remain pending"),
|
||||
)
|
||||
.expect("operation B restore header should parse")
|
||||
.on_going(),
|
||||
"mismatched finalization must not publish restore completion"
|
||||
);
|
||||
|
||||
let mut stale_restore_reader = PutObjReader::from_vec(b"stale A restored body".repeat(1024));
|
||||
let result = set_disks
|
||||
.put_object(
|
||||
@@ -10126,18 +10176,37 @@ mod transition_commit_failure_tests {
|
||||
|
||||
let mut matching_restore_reader = PutObjReader::from_vec(b"matching B restored body".repeat(1024));
|
||||
let operation_b_restore_metadata = restore_metadata(operation_b, false);
|
||||
set_disks
|
||||
let restored = set_disks
|
||||
.put_object(
|
||||
bucket,
|
||||
object,
|
||||
&mut matching_restore_reader,
|
||||
&ObjectOptions {
|
||||
user_defined: operation_b_restore_metadata,
|
||||
user_defined: operation_b_restore_metadata.clone(),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("matching operation B should be allowed to commit");
|
||||
set_disks
|
||||
.finalize_restore_metadata(
|
||||
bucket,
|
||||
object,
|
||||
&restored,
|
||||
&ObjectOptions {
|
||||
user_defined: restore_operation_id_metadata(operation_b),
|
||||
transition: TransitionOptions {
|
||||
restore_request: RestoreRequest {
|
||||
days: Some(1),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("matching operation B should finalize after its commit consumes the operation id");
|
||||
let restored = set_disks
|
||||
.get_object_info(bucket, object, &ObjectOptions::default())
|
||||
.await
|
||||
@@ -10503,13 +10572,16 @@ mod transition_commit_failure_tests {
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
mod transition_upload_integrity_tests {
|
||||
use super::hermetic_set_disks_support::{hermetic_set_disks, hermetic_set_disks_with_lockers};
|
||||
use super::transition_commit_failure_tests::{restore_metadata, restore_operation_id_metadata};
|
||||
use super::*;
|
||||
use crate::bucket::lifecycle::lifecycle::{TRANSITION_PENDING, TransitionOptions};
|
||||
use crate::disk::DiskAPI as _;
|
||||
use crate::layout::endpoints::SetupType;
|
||||
use crate::services::tier::test_util::register_mock_tier;
|
||||
use crate::set_disk::replication::RestoreFinalizeBarrier;
|
||||
use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations as _};
|
||||
use http::HeaderMap;
|
||||
use rustfs_filemeta::RestoreStatusOps as _;
|
||||
use rustfs_lock::client::local::LocalClient;
|
||||
use rustfs_lock::{LockClient, LockError, LockId, LockInfo, LockRequest, LockResponse, LockStats};
|
||||
use std::collections::HashSet;
|
||||
@@ -10655,6 +10727,162 @@ mod transition_upload_integrity_tests {
|
||||
}
|
||||
}
|
||||
|
||||
async fn write_committed_restore(
|
||||
set_disks: &Arc<SetDisks>,
|
||||
disk_stores: &[DiskStore],
|
||||
bucket: &str,
|
||||
object: &str,
|
||||
operation_id: Uuid,
|
||||
) -> ObjectInfo {
|
||||
for disk in disk_stores {
|
||||
disk.make_volume(bucket).await.expect("bucket volume should be created");
|
||||
}
|
||||
let mut source = PutObjReader::from_vec(b"restore source body".repeat(1024));
|
||||
set_disks
|
||||
.put_object(bucket, object, &mut source, &ObjectOptions::default())
|
||||
.await
|
||||
.expect("source object should be written");
|
||||
set_disks
|
||||
.put_object_metadata(
|
||||
bucket,
|
||||
object,
|
||||
&ObjectOptions {
|
||||
eval_metadata: Some(restore_metadata(operation_id, true)),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("pending restore metadata should be installed");
|
||||
|
||||
let mut restored_reader = PutObjReader::from_vec(b"restored body".repeat(1024));
|
||||
set_disks
|
||||
.put_object(
|
||||
bucket,
|
||||
object,
|
||||
&mut restored_reader,
|
||||
&ObjectOptions {
|
||||
user_defined: restore_metadata(operation_id, true),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("matching restore commit should consume its operation id")
|
||||
}
|
||||
|
||||
fn restore_finalize_options(operation_id: Uuid) -> ObjectOptions {
|
||||
ObjectOptions {
|
||||
user_defined: restore_operation_id_metadata(operation_id),
|
||||
transition: TransitionOptions {
|
||||
restore_request: s3s::dto::RestoreRequest {
|
||||
days: Some(1),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
async fn assert_committed_restore_remains_pending(set_disks: &Arc<SetDisks>, bucket: &str, object: &str) {
|
||||
let current = set_disks
|
||||
.get_object_info(bucket, object, &ObjectOptions::default())
|
||||
.await
|
||||
.expect("pending restore metadata should remain readable");
|
||||
assert!(
|
||||
restore_operation_id_from_metadata(current.user_defined.as_ref())
|
||||
.expect("operation id metadata should parse")
|
||||
.is_none(),
|
||||
"successful restore commit must have consumed the operation id"
|
||||
);
|
||||
assert!(
|
||||
rustfs_filemeta::parse_restore_obj_status(
|
||||
current
|
||||
.user_defined
|
||||
.get(s3s::header::X_AMZ_RESTORE.as_str())
|
||||
.expect("pending restore header should remain"),
|
||||
)
|
||||
.expect("restore header should parse")
|
||||
.on_going(),
|
||||
"failed finalization must not publish completion metadata"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
#[serial_test::serial]
|
||||
async fn restore_finalize_rejects_acquired_lock_loss_after_commit() {
|
||||
let refresh_calls = Arc::new(AtomicUsize::new(0));
|
||||
let lockers: Vec<Arc<dyn LockClient>> = (0..4)
|
||||
.map(|_| Arc::new(LockLostRefreshClient::new(Arc::clone(&refresh_calls))) as Arc<dyn LockClient>)
|
||||
.collect();
|
||||
let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks_with_lockers(4, 0, 2, lockers).await;
|
||||
let bucket = "restore-finalize-acquired-lock-lost-bucket";
|
||||
let object = "object.bin";
|
||||
let operation_id = Uuid::new_v4();
|
||||
let restored = write_committed_restore(&set_disks, &disk_stores, bucket, object, operation_id).await;
|
||||
let _setup_type_guard = SetupTypeGuard::switch_to(SetupType::DistErasure).await;
|
||||
let barrier = RestoreFinalizeBarrier::install(bucket, object);
|
||||
let finalize_set = Arc::clone(&set_disks);
|
||||
let finalize = tokio::spawn(async move {
|
||||
finalize_set
|
||||
.finalize_restore_metadata(bucket, object, &restored, &restore_finalize_options(operation_id))
|
||||
.await
|
||||
});
|
||||
barrier.wait_until_paused().await;
|
||||
tokio::time::advance(Duration::from_secs(11)).await;
|
||||
tokio::task::yield_now().await;
|
||||
assert!(refresh_calls.load(Ordering::SeqCst) > 0, "restore finalization lock must attempt renewal");
|
||||
barrier.release();
|
||||
|
||||
let error = finalize
|
||||
.await
|
||||
.expect("restore finalization task should join")
|
||||
.expect_err("lost acquired lock must reject restore finalization");
|
||||
assert!(matches!(
|
||||
error,
|
||||
Error::Io(ref error)
|
||||
if error.kind() == std::io::ErrorKind::Other
|
||||
&& error.to_string() == "restore finalization lock lost before metadata update"
|
||||
));
|
||||
assert_committed_restore_remains_pending(&set_disks, bucket, object).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial_test::serial]
|
||||
async fn restore_finalize_rejects_outer_fence_loss_after_metadata_read() {
|
||||
let (_temp_dirs, disk_stores, set_disks) = hermetic_set_disks(4).await;
|
||||
let bucket = "restore-finalize-outer-fence-lost-bucket";
|
||||
let object = "object.bin";
|
||||
let operation_id = Uuid::new_v4();
|
||||
let restored = write_committed_restore(&set_disks, &disk_stores, bucket, object, operation_id).await;
|
||||
let (fence, loss_handle) = NamespaceLockFence::loss_handle_for_test();
|
||||
let barrier = RestoreFinalizeBarrier::install(bucket, object);
|
||||
let finalize_set = Arc::clone(&set_disks);
|
||||
let finalize = tokio::spawn(async move {
|
||||
let mut opts = restore_finalize_options(operation_id);
|
||||
opts.no_lock = true;
|
||||
opts.namespace_lock_fence = Some(fence);
|
||||
finalize_set.finalize_restore_metadata(bucket, object, &restored, &opts).await
|
||||
});
|
||||
barrier.wait_until_paused().await;
|
||||
loss_handle.store(true, std::sync::atomic::Ordering::Release);
|
||||
barrier.release();
|
||||
|
||||
let error = finalize
|
||||
.await
|
||||
.expect("restore finalization task should join")
|
||||
.expect_err("lost outer fence must reject restore finalization");
|
||||
assert!(matches!(
|
||||
error,
|
||||
Error::NamespaceLockQuorumUnavailable {
|
||||
mode: "restore_finalize_metadata",
|
||||
required: 1,
|
||||
achieved: 0,
|
||||
..
|
||||
}
|
||||
));
|
||||
assert_committed_restore_remains_pending(&set_disks, bucket, object).await;
|
||||
}
|
||||
|
||||
async fn assert_local_source_intact(set_disks: &Arc<SetDisks>, bucket: &str, object: &str, payload: &[u8]) {
|
||||
let mut restored = Vec::new();
|
||||
set_disks
|
||||
|
||||
@@ -18,6 +18,78 @@ use rustfs_filemeta::RestoreStatusOps;
|
||||
use rustfs_utils::http::headers::{AMZ_RESTORE_EXPIRY_DAYS, AMZ_RESTORE_REQUEST_DATE};
|
||||
use s3s::dto::{RestoreStatus, Timestamp};
|
||||
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
struct RestoreFinalizeBarrierState {
|
||||
bucket: String,
|
||||
object: String,
|
||||
arrived: tokio::sync::Notify,
|
||||
release: tokio::sync::Notify,
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
static RESTORE_FINALIZE_BARRIER: std::sync::OnceLock<std::sync::Mutex<Option<Arc<RestoreFinalizeBarrierState>>>> =
|
||||
std::sync::OnceLock::new();
|
||||
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
pub(in crate::set_disk) struct RestoreFinalizeBarrier {
|
||||
state: Arc<RestoreFinalizeBarrierState>,
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
impl RestoreFinalizeBarrier {
|
||||
pub(in crate::set_disk) fn install(bucket: &str, object: &str) -> Self {
|
||||
let state = Arc::new(RestoreFinalizeBarrierState {
|
||||
bucket: bucket.to_string(),
|
||||
object: object.to_string(),
|
||||
arrived: tokio::sync::Notify::new(),
|
||||
release: tokio::sync::Notify::new(),
|
||||
});
|
||||
let mut slot = RESTORE_FINALIZE_BARRIER
|
||||
.get_or_init(|| std::sync::Mutex::new(None))
|
||||
.lock()
|
||||
.expect("restore finalize barrier mutex should not poison");
|
||||
assert!(slot.is_none(), "restore finalize barrier must be installed by one test at a time");
|
||||
*slot = Some(Arc::clone(&state));
|
||||
Self { state }
|
||||
}
|
||||
|
||||
pub(in crate::set_disk) async fn wait_until_paused(&self) {
|
||||
self.state.arrived.notified().await;
|
||||
}
|
||||
|
||||
pub(in crate::set_disk) fn release(&self) {
|
||||
self.state.release.notify_one();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
impl Drop for RestoreFinalizeBarrier {
|
||||
fn drop(&mut self) {
|
||||
let mut slot = RESTORE_FINALIZE_BARRIER
|
||||
.get_or_init(|| std::sync::Mutex::new(None))
|
||||
.lock()
|
||||
.expect("restore finalize barrier mutex should not poison");
|
||||
if slot.as_ref().is_some_and(|state| Arc::ptr_eq(state, &self.state)) {
|
||||
*slot = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
async fn maybe_pause_restore_finalize(bucket: &str, object: &str) {
|
||||
let barrier = RESTORE_FINALIZE_BARRIER
|
||||
.get_or_init(|| std::sync::Mutex::new(None))
|
||||
.lock()
|
||||
.expect("restore finalize barrier mutex should not poison")
|
||||
.as_ref()
|
||||
.filter(|barrier| barrier.bucket == bucket && barrier.object == object)
|
||||
.cloned();
|
||||
if let Some(barrier) = barrier {
|
||||
barrier.arrived.notify_one();
|
||||
barrier.release.notified().await;
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
struct RestoreCleanupIdentity {
|
||||
version_id: Option<Uuid>,
|
||||
@@ -80,7 +152,7 @@ impl SetDisks {
|
||||
.clone()
|
||||
.unwrap_or_else(|| get_raw_etag(obj_info.user_defined.as_ref()));
|
||||
let version_id = expected.version_id.map(|v| v.to_string());
|
||||
let _lock_guard = if !opts.no_lock {
|
||||
let lock_guard = if !opts.no_lock {
|
||||
Some(
|
||||
self.acquire_write_lock_diag("restore_finalize_metadata", bucket, object)
|
||||
.await?,
|
||||
@@ -99,13 +171,16 @@ impl SetDisks {
|
||||
.get_object_fileinfo_gated(bucket, object, &read_opts, false, false)
|
||||
.await?
|
||||
.into_owned();
|
||||
if let Some(expected_operation_id) = expected_operation_id {
|
||||
require_restore_operation_id(&fi.metadata, expected_operation_id)?;
|
||||
if let Some(expected_operation_id) = expected_operation_id
|
||||
&& restore_operation_id_from_metadata(&fi.metadata)?.is_some_and(|actual| actual != expected_operation_id)
|
||||
{
|
||||
return Err(Error::other("restore operation id changed before metadata finalization"));
|
||||
}
|
||||
if !expected.matches_file_info(&fi, &expected_etag) {
|
||||
return Err(Error::other("restored object changed before restore metadata finalization"));
|
||||
}
|
||||
ensure_restore_metadata_lock_held(bucket, object, opts, "restore_finalize_metadata")?;
|
||||
#[cfg(all(test, feature = "test-util"))]
|
||||
maybe_pause_restore_finalize(bucket, object).await;
|
||||
let restore_expiry =
|
||||
lifecycle::expected_expiry_time(OffsetDateTime::now_utc(), opts.transition.restore_request.days.unwrap_or(1));
|
||||
fi.metadata.insert(
|
||||
@@ -117,6 +192,10 @@ impl SetDisks {
|
||||
.to_string(),
|
||||
);
|
||||
self.invalidate_get_object_metadata_cache(bucket, object).await;
|
||||
ensure_restore_metadata_lock_held(bucket, object, opts, "restore_finalize_metadata")?;
|
||||
if lock_guard.as_ref().is_some_and(|guard| guard.is_lock_lost()) {
|
||||
return Err(Error::other("restore finalization lock lost before metadata update"));
|
||||
}
|
||||
self.update_object_meta_with_opts(
|
||||
bucket,
|
||||
object,
|
||||
|
||||
Reference in New Issue
Block a user