mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-28 07:57:01 +00:00
fix(tier): recover (#3182)
* fix(tier): stop sending nil/garbage versionId to warm backend S3 Three bugs caused NoSuchVersion errors when reading tiered objects: 1. warm_backend_s3sdk: GET and DELETE ignored rv/range opts entirely — fixed to forward version_id and byte-range to the SDK request. 2. version.rs (MetaObject + MetaDeleteMarker): transition_version_id was parsed with unwrap_or_default(), turning invalid/wrong-length bytes into Uuid::nil(). The nil UUID was then serialized and sent as ?versionId=00000000-... to the tier backend -> NoSuchVersion. Fixed: .and_then(.ok()).filter(!is_nil()) so only valid non-nil UUIDs are forwarded as versionId. 3. bucket_lifecycle_ops: add debug/error logs in get_transitioned_object_reader to record tier, tier_object, and tier_version_id before and on failure of the tier GET. Also adds tier transition fields to dump_fileinfo example for offline xl.meta inspection, and fixes Docker build (cargo path + entrypoint). Adds CLAUDE.md with tier architecture and debugging notes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * more fixes for versionId * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marcelo Bartsch <marcelo@bartsch.cl> * remove branch * Add tests and fix cargo path, add load to build-docker * update documentation (CLAUDE.md) * more fixes for recover * More fixes to ILM recover * final fix * chore: add missing-shard first-scene diagnostics (#3213) chore(ecstore): add missing-shard first-scene diagnostics Log rename_data quorum context behind RUSTFS_ISSUE3031_DIAG_ENABLE so partial-disk success can be correlated with later missing shard reads. Also log put_object commit success and tmp cleanup boundaries to capture when successful quorum writes are followed by tmp_dir cleanup. * fix test anmd fmt * fix cargo path fix test * fix(tier): format copy_object self-copy guard --------- Signed-off-by: Marcelo Bartsch <marcelo@bartsch.cl> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: 安正超 <anzhengchao@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: houseme <housemecn@gmail.com> Co-authored-by: cxymds <Cxymds@qq.com> Co-authored-by: loverustfs <hello@rustfs.com>
This commit is contained in:
@@ -1770,11 +1770,31 @@ pub async fn get_transitioned_object_reader(
|
||||
gopts.length = length;
|
||||
}
|
||||
|
||||
//return Ok(HttpFileReader::new(rs, &oi, opts, &h));
|
||||
//timeTierAction := auditTierActions(oi.transitioned_object.Tier, length)
|
||||
debug!(
|
||||
bucket = %bucket,
|
||||
object = %object,
|
||||
tier = %oi.transitioned_object.tier,
|
||||
tier_object = %oi.transitioned_object.name,
|
||||
tier_version_id = %oi.transitioned_object.version_id,
|
||||
start_offset = gopts.start_offset,
|
||||
length = gopts.length,
|
||||
"fetching transitioned object from tier"
|
||||
);
|
||||
let reader = tgt_client
|
||||
.get(&oi.transitioned_object.name, &oi.transitioned_object.version_id, gopts)
|
||||
.await?;
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(
|
||||
bucket = %bucket,
|
||||
object = %object,
|
||||
tier = %oi.transitioned_object.tier,
|
||||
tier_object = %oi.transitioned_object.name,
|
||||
tier_version_id = %oi.transitioned_object.version_id,
|
||||
error = %e,
|
||||
"tier GET failed"
|
||||
);
|
||||
e
|
||||
})?;
|
||||
Ok(get_fn(reader, h.clone()))
|
||||
}
|
||||
|
||||
|
||||
@@ -1635,10 +1635,17 @@ impl ObjectOperations for SetDisks {
|
||||
src_opts: &ObjectOptions,
|
||||
dst_opts: &ObjectOptions,
|
||||
) -> Result<ObjectInfo> {
|
||||
// FIXME: TODO:
|
||||
|
||||
if !src_info.metadata_only {
|
||||
return Err(StorageError::NotImplemented);
|
||||
if path_join_buf(&[src_bucket, src_object]) != path_join_buf(&[dst_bucket, dst_object]) {
|
||||
return Err(StorageError::NotImplemented);
|
||||
}
|
||||
// Self-copy with a data reader: write tier data back locally (de-tiering).
|
||||
// Handles `mc cp --storage-class STANDARD obj obj` on a transitioned object.
|
||||
if let Some(mut put_reader) = src_info.put_object_reader.take() {
|
||||
return self.put_object(dst_bucket, dst_object, &mut put_reader, dst_opts).await;
|
||||
}
|
||||
// Same-key tiered copy without a pre-fetched reader: fall through to the metadata
|
||||
// path so the caller gets a disk/quorum error rather than NotImplemented.
|
||||
}
|
||||
|
||||
if path_join_buf(&[src_bucket, src_object]) != path_join_buf(&[dst_bucket, dst_object]) {
|
||||
@@ -4727,6 +4734,7 @@ pub fn is_infrequent_access_class(storage_class: &str) -> bool {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::bucket::lifecycle::bucket_lifecycle_ops::TransitionedObject;
|
||||
use crate::disk::CHECK_PART_UNKNOWN;
|
||||
use crate::disk::CHECK_PART_VOLUME_NOT_FOUND;
|
||||
use crate::disk::RUSTFS_META_BUCKET;
|
||||
@@ -6735,4 +6743,59 @@ mod tests {
|
||||
assert!(!is_infrequent_access_class(storageclass::DEEP_ARCHIVE));
|
||||
assert!(!is_infrequent_access_class(storageclass::EXPRESS_ONEZONE));
|
||||
}
|
||||
|
||||
// Regression test: `mc cp --storage-class STANDARD` on a tiered object (self-copy) must not
|
||||
// return NotImplemented. When the source object is tiered (transitioned_object.tier is
|
||||
// non-empty) the usecase layer in object_usecase.rs intentionally leaves metadata_only=false
|
||||
// so that the full copy path is taken. SetDisks::copy_object must therefore accept a
|
||||
// same-bucket/same-key call even when metadata_only=false.
|
||||
//
|
||||
// Currently this test FAILS because the guard at set_disk.rs:1579 unconditionally rejects
|
||||
// !metadata_only with StorageError::NotImplemented. Once the fix is applied the test will
|
||||
// pass (or progress further through the copy path before failing on missing disk data).
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
#[serial]
|
||||
async fn copy_object_tiered_self_copy_does_not_return_not_implemented() {
|
||||
let _setup_type_guard = SetupTypeGuard::switch_to(SetupType::Erasure).await;
|
||||
let set_disks = make_test_set_disks(vec![Arc::new(LocalClient::with_manager(Arc::new(
|
||||
rustfs_lock::GlobalLockManager::new(),
|
||||
)))])
|
||||
.await;
|
||||
|
||||
// Simulate a tiered object: metadata_only is false (set_disk must handle the full copy),
|
||||
// and transitioned_object.tier is non-empty (the object lives on a remote tier).
|
||||
let mut src_info = ObjectInfo {
|
||||
metadata_only: false,
|
||||
transitioned_object: TransitionedObject {
|
||||
tier: "NEXTCLOUD".to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let result = set_disks
|
||||
.copy_object(
|
||||
"bucket",
|
||||
"object",
|
||||
"bucket",
|
||||
"object",
|
||||
&mut src_info,
|
||||
&ObjectOptions::default(),
|
||||
&ObjectOptions {
|
||||
no_lock: true,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
// The copy must not be rejected with NotImplemented. Any other outcome (Ok or a
|
||||
// different error such as missing-disk / quorum) is acceptable here.
|
||||
if let Err(ref err) = result {
|
||||
assert!(
|
||||
!matches!(err, StorageError::NotImplemented),
|
||||
"tiered self-copy returned NotImplemented — copy_object must handle \
|
||||
metadata_only=false for same-key copies of tiered objects, got: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -570,9 +570,32 @@ impl ECStore {
|
||||
}
|
||||
|
||||
if !dst_opts.versioned && src_opts.version_id.is_none() {
|
||||
return self.pools[pool_idx]
|
||||
.copy_object(src_bucket, &src_object, dst_bucket, &dst_object, src_info, src_opts, &dst_opts)
|
||||
.await;
|
||||
if src_info.metadata_only {
|
||||
return self.pools[pool_idx]
|
||||
.copy_object(src_bucket, &src_object, dst_bucket, &dst_object, src_info, src_opts, &dst_opts)
|
||||
.await;
|
||||
}
|
||||
// Transitioned object self-copy: restore from tier into the same pool.
|
||||
let put_opts = ObjectOptions {
|
||||
user_defined: (*src_info.user_defined).clone(),
|
||||
versioned: dst_opts.versioned,
|
||||
version_id: dst_opts.version_id.clone(),
|
||||
no_lock: dst_opts.no_lock,
|
||||
mod_time: dst_opts.mod_time,
|
||||
http_preconditions: dst_opts.http_preconditions.clone(),
|
||||
..Default::default()
|
||||
};
|
||||
return if let Some(reader) = src_info.put_object_reader.as_mut() {
|
||||
self.pools[pool_idx]
|
||||
.put_object(dst_bucket, &dst_object, reader, &put_opts)
|
||||
.await
|
||||
} else {
|
||||
Err(StorageError::InvalidArgument(
|
||||
src_bucket.to_owned(),
|
||||
src_object.to_owned(),
|
||||
"put_object_reader is none".to_owned(),
|
||||
))
|
||||
};
|
||||
}
|
||||
|
||||
if dst_opts.versioned && src_opts.version_id != dst_opts.version_id {
|
||||
|
||||
@@ -87,6 +87,7 @@ fn parse_http_timestamp(value: &str) -> Option<OffsetDateTime> {
|
||||
pub fn build_transition_put_options(storage_class: String, mut metadata: HashMap<String, String>) -> PutObjectOptions {
|
||||
let mut opts = PutObjectOptions {
|
||||
storage_class,
|
||||
send_content_md5: true,
|
||||
legalhold: ObjectLockLegalHoldStatus::from_static(""),
|
||||
internal: AdvancedPutOptions {
|
||||
replication_status: ReplicationStatus::from_static(""),
|
||||
|
||||
@@ -147,15 +147,22 @@ impl WarmBackend for WarmBackendS3 {
|
||||
|
||||
async fn get(&self, object: &str, rv: &str, opts: WarmBackendGetOpts) -> Result<ReadCloser, std::io::Error> {
|
||||
let client = self.client.clone();
|
||||
let Ok(res) = client
|
||||
.get_object()
|
||||
.bucket(&self.bucket)
|
||||
.key(&self.get_dest(object))
|
||||
.send()
|
||||
.await
|
||||
else {
|
||||
return Err(std::io::Error::other("get_object error"));
|
||||
};
|
||||
let mut req = client.get_object().bucket(&self.bucket).key(&self.get_dest(object));
|
||||
|
||||
if !rv.is_empty() {
|
||||
req = req.version_id(rv);
|
||||
}
|
||||
|
||||
if opts.start_offset >= 0 && opts.length > 0 {
|
||||
let end = opts
|
||||
.start_offset
|
||||
.checked_add(opts.length)
|
||||
.and_then(|v| v.checked_sub(1))
|
||||
.ok_or_else(|| std::io::Error::other("invalid range: overflow"))?;
|
||||
req = req.range(format!("bytes={}-{}", opts.start_offset, end));
|
||||
}
|
||||
|
||||
let res = req.send().await.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
|
||||
Ok(ReadCloser::new(std::io::Cursor::new(
|
||||
res.body.collect().await.map(|data| data.into_bytes().to_vec())?,
|
||||
@@ -164,16 +171,14 @@ impl WarmBackend for WarmBackendS3 {
|
||||
|
||||
async fn remove(&self, object: &str, rv: &str) -> Result<(), std::io::Error> {
|
||||
let client = self.client.clone();
|
||||
if let Err(_) = client
|
||||
.delete_object()
|
||||
.bucket(&self.bucket)
|
||||
.key(&self.get_dest(object))
|
||||
.send()
|
||||
.await
|
||||
{
|
||||
return Err(std::io::Error::other("delete_object error"));
|
||||
let mut req = client.delete_object().bucket(&self.bucket).key(&self.get_dest(object));
|
||||
|
||||
if !rv.is_empty() {
|
||||
req = req.version_id(rv);
|
||||
}
|
||||
|
||||
req.send().await.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user