fix(heal): harden replacement rebuild tracking (#5892)

* fix(heal): gate auto replacement formatting

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): require replacement target outcomes

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): bind resumes to replacement targets

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): fence healing marker ownership

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): cover replacement target completion

Co-Authored-By: heihutu <heihutu@gmail.com>

* docs(heal): clarify replacement recovery status

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): canonicalize replacement target checks

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): satisfy marker test module lint

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): scope automatic replacement format

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): require a mounted replacement target

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): avoid cloned ref slice in test

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): revalidate replacement before scanning

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): reset stale resume checkpoints

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): release scanner disk map before probing

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): persist replacement intent before format

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): fail closed on mountinfo read errors

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): fence replacement target identity

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): order replacement completion cleanup

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): atomically seal replacement completion

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): census replacement target shards

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): fence replacement recovery ownership

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): preserve replacement recovery anchors

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): satisfy replacement recovery lint gates

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): bind replacement identity to mount lease

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): cover durable replacement recovery states

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): validate persisted resume task identifiers

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): avoid blocking replacement marker CAS

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): report failed marker rollback

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): pin replacement resume schema compatibility

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): preserve durable recovery anchors

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): preserve public disk path semantics

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): use canonical replacement task ids

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): cover automatic replacement in 3x4 cluster

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): verify replacement target commits

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): persist replacement completion proof

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(heal): expose durable replacement status

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): bound durable replacement discovery

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): remove replacement readiness bypass

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): retry terminal replacement cleanup

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): isolate replacement intents from legacy resume

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): migrate legacy replacement intents at startup

Co-Authored-By: heihutu <heihutu@gmail.com>

* style(heal): apply strict clippy fix

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): prioritize active replacement recovery state

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): bind readiness to the admitted mount lease

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): atomically publish replacement intents

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): isolate replacement recovery directory

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): tolerate an empty recovery directory

Co-Authored-By: heihutu <heihutu@gmail.com>

* style(heal): remove redundant disk bytes conversion

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): reconcile proof-first replacement recovery

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): fence torn intent recovery

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): cover replacement migration conflicts

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): fence replacement lease mount identity

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(heal): cover missing replacement path admission

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): reject conflicting legacy completion proof

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): fall back to proc mount identity

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(admin): expose replacement recovery status

Surface the local durable replacement recovery snapshot in the background heal status response so operators can tell whether replacement cleanup is definitive or still pending.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): keep replacement status compatible

Keep the existing background heal status response wire-compatible while retaining the Linux mount lease cleanup needed for the replacement recovery branch.

Co-Authored-By: heihutu <heihutu@gmail.com>

* style(ecstore): match linux mount lease formatting

Keep Linux rustfmt output stable for the replacement mount lease comparison.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): qualify mount lease test constant

Use the disk module path for the format config constant in the Linux mount lease regression test.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): keep procfd mount roots directory-safe

Use a procfd path with an explicit directory component so Unix directory guards can open the replacement mount lease root with O_NOFOLLOW while preserving handle-relative I/O semantics.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): delete empty leased buckets via dirfd

Use the held mount lease fd as the parent for non-force empty bucket deletion on Linux so procfd-rooted paths do not get rejected as BucketNotEmpty. Also make the download-part OpenOptions truncate behavior explicit and keep fsync test recording stable across procfd canonicalization.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): scan leased bucket paths for emptiness

Use the local disk I/O root for bucket emptiness probes before non-force bucket deletion and table-bucket metadata checks. This keeps validation on the same mount instance as the subsequent local disk delete path.

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(ecstore): align lease path test probes

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): block unsafe replacement recovery restarts

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): defer blocked replacement candidates

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): retry transient replacement discovery

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): keep transient recovery errors retryable

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): block corrupt legacy replacement state

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): classify flat replacement intent corruption

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): keep transient resume loads retryable

Classify malformed legacy replacement state as blocking corruption while preserving disk and transient load failures for retry. This avoids permanently blocking replacement recovery on temporary storage errors.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): avoid latching transient legacy publishes

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): retry blocked legacy migrations

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(heal): defer blocked startup recoveries

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): preserve disk sync limiter across lease roots

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
Co-authored-by: zhi22915 <qiuzgang@gmail.com>
This commit is contained in:
houseme
2026-08-10 08:32:47 +08:00
committed by GitHub
parent 10a1d6b6e6
commit f17ea7f146
28 changed files with 7060 additions and 442 deletions
+2 -2
View File
@@ -326,8 +326,8 @@ pub mod disk {
pub use crate::disk::local::ScanGuard;
pub use crate::disk::{
BATCH_READ_VERSION_MAX_ITEMS, BUCKET_META_PREFIX, BatchReadVersionItem, BatchReadVersionReq, BatchReadVersionResp,
CheckPartsResp, DeleteOptions, Disk, DiskAPI, DiskInfo, DiskInfoOptions, DiskLocation, DiskOption, DiskStore,
FileInfoVersions, FileReader, FileWriter, HEALING_MARKER_PATH, NsScannerOpenRequest, OldCurrentSize,
CheckPartsResp, ConditionalFileUpdate, DeleteOptions, Disk, DiskAPI, DiskInfo, DiskInfoOptions, DiskLocation, DiskOption,
DiskStore, FileInfoVersions, FileReader, FileWriter, HEALING_MARKER_PATH, NsScannerOpenRequest, OldCurrentSize,
PartTransactionAction, RUSTFS_META_BUCKET, ReadMultipleReq, ReadMultipleResp, ReadOptions, RenameDataResp,
STORAGE_FORMAT_FILE, SnapshotLeaseToken, UpdateMetadataOpts, VolumeInfo, WalkDirOptions, new_disk,
validate_batch_read_version_item_count,
@@ -57,7 +57,7 @@ fn build_part_path(file_path: &Path) -> PathBuf {
async fn open_download_part_file(file_part_path: &Path) -> io::Result<tokio::fs::File> {
let mut options = OpenOptions::new();
options.create(true).read(true).write(true);
options.create(true).truncate(false).read(true).write(true);
#[cfg(not(windows))]
options.mode(0o600);
@@ -784,7 +784,11 @@ impl PeerS3Client for LocalPeerS3Client {
if opts.force_if_empty && !opts.force {
for disk in local_disks.iter() {
if has_xlmeta_files(&disk.path().join(bucket)).await.map_err(Error::Io)? {
let Some(bucket_path) = disk.get_bucket_path_for_io_if_local(bucket) else {
continue;
};
let bucket_path = bucket_path?;
if has_xlmeta_files(&bucket_path).await.map_err(Error::Io)? {
return Err(Error::VolumeNotEmpty);
}
}
+45 -6
View File
@@ -1058,17 +1058,23 @@ impl crate::storage_api_contracts::heal::HealOperations for Sets {
for (i, set) in new_format_sets.iter().enumerate() {
for (j, fm) in set.iter().enumerate() {
if let Some(fm) = fm {
res.after.drives[i * self.set_drive_count + j].uuid = fm.erasure.this.to_string();
res.after.drives[i * self.set_drive_count + j].state = DriveState::Ok.to_string();
tmp_new_formats[i * self.set_drive_count + j] = Some(fm.clone());
}
}
}
// Save new formats `format.json` on unformatted disks.
for (fm, disk) in tmp_new_formats.iter_mut().zip(disks.iter()) {
if fm.is_some() && disk.is_some() && save_format_file(disk, fm).await.is_err() {
let _ = disk.as_ref().unwrap().close().await;
*fm = None;
for (index, (fm, disk)) in tmp_new_formats.iter_mut().zip(disks.iter()).enumerate() {
if fm.is_some() && disk.is_some() {
if let Err(err) = save_format_file(disk, fm).await {
if let Some(disk) = disk.as_ref() {
let _ = disk.close().await;
}
return Ok((res, Some(err.into())));
}
if let Some(saved_format) = fm.as_ref() {
res.after.drives[index].uuid = saved_format.erasure.this.to_string();
res.after.drives[index].state = DriveState::Ok.to_string();
}
}
}
@@ -2206,6 +2212,39 @@ mod tests {
);
}
#[tokio::test]
#[serial]
async fn replacement_format_only_writes_the_requested_slot() {
let (_dirs, _ref_format, sets) = setup_heal_format_sets(1, false).await;
let target = sets.endpoints.endpoints.as_ref()[1].to_string();
let untouched = sets.endpoints.endpoints.as_ref()[2].to_string();
let set = set_level_heal_view(&sets).await;
let (result, error) = set
.heal_replacement_format(false, std::slice::from_ref(&target))
.await
.expect("target-scoped replacement format should run");
assert!(error.is_none(), "target format must not report an error: {error:?}");
assert!(
result
.after
.drives
.iter()
.any(|drive| drive.endpoint == target && drive.state == DriveState::Ok.to_string()),
"requested replacement slot must be formatted"
);
let untouched_format = std::path::Path::new(&sets.endpoints.endpoints.as_ref()[2].get_file_path())
.join(crate::disk::RUSTFS_META_BUCKET)
.join(crate::disk::FORMAT_CONFIG_FILE);
assert!(
!tokio::fs::try_exists(untouched_format)
.await
.expect("untouched replacement format path should be inspectable"),
"unrequested slot {untouched} must remain unformatted"
);
}
fn instance_ctx_test_pool_endpoints() -> (FormatV3, PoolEndpoints) {
let format = FormatV3::new(1, 2);
let endpoints = vec![
+33
View File
@@ -152,6 +152,7 @@ const DISK_OPERATION_NAMES: &[&str] = &[
"read_parts",
"read_multiple",
"write_all",
"compare_and_update_file",
"read_all",
];
@@ -1092,6 +1093,18 @@ impl LocalDiskWrapper {
self.disk.get_object_path(volume, path)
}
pub(crate) fn get_object_path_for_io(&self, volume: &str, path: &str) -> crate::disk::error::Result<std::path::PathBuf> {
self.disk.get_object_path_for_io(volume, path)
}
pub(crate) fn get_bucket_path_for_io(&self, volume: &str) -> crate::disk::error::Result<std::path::PathBuf> {
self.disk.get_bucket_path_for_io(volume)
}
pub fn replacement_mount_lease_root(&self) -> Option<std::path::PathBuf> {
self.disk.replacement_mount_lease_root()
}
pub fn runtime_state(&self) -> RuntimeDriveHealthState {
self.health.runtime_state()
}
@@ -1639,6 +1652,10 @@ impl LocalDiskWrapper {
#[async_trait::async_trait]
impl DiskAPI for LocalDiskWrapper {
fn has_replacement_mount_lease(&self) -> bool {
self.disk.has_replacement_mount_lease()
}
async fn read_metadata(&self, volume: &str, path: &str) -> Result<Bytes> {
self.track_disk_health_with_op_and_timeout_action(
"read_metadata",
@@ -2140,6 +2157,22 @@ impl DiskAPI for LocalDiskWrapper {
.await
}
async fn compare_and_update_file(
&self,
volume: &str,
path: &str,
expected: Option<Bytes>,
replacement: Option<Bytes>,
) -> Result<crate::disk::ConditionalFileUpdate> {
self.track_disk_health_mutation(
"compare_and_update_file",
DiskMetricMutation::Write,
|| async { self.disk.compare_and_update_file(volume, path, expected, replacement).await },
get_max_timeout_duration(),
)
.await
}
async fn read_all(&self, volume: &str, path: &str) -> Result<Bytes> {
self.track_disk_health_with_op(
"read_all",
File diff suppressed because it is too large Load Diff
+89
View File
@@ -115,6 +115,15 @@ pub enum PartTransactionAction {
Rollback,
}
/// Result of an owner-aware file mutation. The disk applies the mutation only
/// while the current contents match the supplied expected value.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum ConditionalFileUpdate {
Updated,
Missing,
Mismatch,
}
#[derive(Clone, Copy, Debug)]
pub struct MmapCopyStageMetrics {
pub(crate) path: &'static str,
@@ -557,6 +566,26 @@ impl DiskAPI for Disk {
}
}
async fn compare_and_update_file(
&self,
volume: &str,
path: &str,
expected: Option<Bytes>,
replacement: Option<Bytes>,
) -> Result<ConditionalFileUpdate> {
match self {
Disk::Local(local_disk) => local_disk.compare_and_update_file(volume, path, expected, replacement).await,
Disk::Remote(remote_disk) => remote_disk.compare_and_update_file(volume, path, expected, replacement).await,
}
}
fn has_replacement_mount_lease(&self) -> bool {
match self {
Disk::Local(local_disk) => local_disk.has_replacement_mount_lease(),
Disk::Remote(remote_disk) => remote_disk.has_replacement_mount_lease(),
}
}
#[tracing::instrument(level = "trace", skip_all)]
async fn read_all(&self, volume: &str, path: &str) -> Result<Bytes> {
match self {
@@ -695,6 +724,34 @@ impl Disk {
Disk::Remote(_) => None,
}
}
pub(crate) fn get_object_path_for_io_if_local(
&self,
volume: &str,
path: &str,
) -> Option<crate::disk::error::Result<std::path::PathBuf>> {
match self {
Disk::Local(w) => Some(w.get_object_path_for_io(volume, path)),
Disk::Remote(_) => None,
}
}
pub(crate) fn get_bucket_path_for_io_if_local(&self, volume: &str) -> Option<crate::disk::error::Result<std::path::PathBuf>> {
match self {
Disk::Local(w) => Some(w.get_bucket_path_for_io(volume)),
Disk::Remote(_) => None,
}
}
/// Return the descriptor-rooted mount path admitted for automatic
/// replacement, or `None` when the configured endpoint no longer names
/// that held mount instance.
pub fn replacement_mount_lease_root(&self) -> Option<PathBuf> {
match self {
Disk::Local(local_disk) => local_disk.replacement_mount_lease_root(),
Disk::Remote(_) => None,
}
}
}
pub async fn new_disk(ep: &Endpoint, opt: &DiskOption) -> Result<DiskStore> {
@@ -860,6 +917,24 @@ pub trait DiskAPI: Debug + Send + Sync + 'static {
// CleanAbandonedData
async fn write_all(&self, volume: &str, path: &str, data: Bytes) -> Result<()>;
async fn read_all(&self, volume: &str, path: &str) -> Result<Bytes>;
/// Atomically replace or remove a small control file only when its current
/// contents match `expected`. Implementations that cannot provide this
/// cross-process guarantee must fail closed instead of emulating it with a
/// read-then-write sequence.
async fn compare_and_update_file(
&self,
_volume: &str,
_path: &str,
_expected: Option<Bytes>,
_replacement: Option<Bytes>,
) -> Result<ConditionalFileUpdate> {
Err(DiskError::MethodNotAllowed)
}
/// Whether local I/O is rooted at a held mount descriptor. Auto-replacement
/// refuses destructive work when this is false.
fn has_replacement_mount_lease(&self) -> bool {
false
}
async fn disk_info(&self, opts: &DiskInfoOptions) -> Result<DiskInfo>;
fn start_scan(&self) -> ScanGuard;
}
@@ -1612,6 +1687,7 @@ mod tests {
let endpoint = Endpoint::try_from(test_dir).unwrap();
let local_disk = LocalDisk::new(&endpoint, false).await.unwrap();
let expected_object_path = local_disk.root.join("test-bucket/test-object");
let disk = Disk::Local(Box::new(LocalDiskWrapper::new(Arc::new(local_disk), false)));
// Test basic methods
@@ -1626,6 +1702,19 @@ mod tests {
// Test path method
let path = disk.path();
assert!(path.exists());
let object_path = disk
.get_object_path_if_local("test-bucket", "test-object")
.expect("local disk should expose an object path")
.expect("object path should resolve");
assert_eq!(object_path, expected_object_path);
assert!(!object_path.starts_with("/proc/self/fd/"));
#[cfg(target_os = "linux")]
assert!(
disk.get_object_path_for_io_if_local("test-bucket", "test-object")
.expect("local disk should expose an I/O object path")
.expect("I/O object path should resolve")
.starts_with("/proc/self/fd/")
);
// Test disk location
let location = disk.get_disk_location();
+27 -10
View File
@@ -84,11 +84,22 @@ pub(crate) mod fsync_dir_recorder {
static RECORDED: Mutex<Vec<PathBuf>> = Mutex::new(Vec::new());
pub(crate) fn record(dir: &Path) {
RECORDED.lock().expect("fsync dir recorder poisoned").push(dir.to_path_buf());
let mut recorded = RECORDED.lock().expect("fsync dir recorder poisoned");
recorded.push(dir.to_path_buf());
if let Ok(canonical) = dir.canonicalize()
&& canonical != dir
{
recorded.push(canonical);
}
}
pub(crate) fn was_fsynced(dir: &Path) -> bool {
RECORDED.lock().expect("fsync dir recorder poisoned").iter().any(|p| p == dir)
let canonical = dir.canonicalize().ok();
RECORDED
.lock()
.expect("fsync dir recorder poisoned")
.iter()
.any(|p| p == dir || canonical.as_ref().is_some_and(|canonical| p == canonical))
}
}
@@ -330,7 +341,7 @@ pub(crate) mod file_sync_probe {
use tokio::sync::Notify;
use tokio::time::timeout;
static ROOT: RwLock<Option<PathBuf>> = RwLock::new(None);
static ROOTS: RwLock<Vec<PathBuf>> = RwLock::new(Vec::new());
static BLOCK_MUTEX: Mutex<()> = Mutex::new(());
static BLOCK_CONDVAR: Condvar = Condvar::new();
static ACTIVE_CHANGED: Notify = Notify::const_new();
@@ -364,7 +375,7 @@ pub(crate) mod file_sync_probe {
fn drop(&mut self) {
release();
FAIL_ON_ATTEMPT.store(usize::MAX, Ordering::SeqCst);
*ROOT.write().expect("file sync probe lock poisoned") = None;
ROOTS.write().expect("file sync probe lock poisoned").clear();
}
}
@@ -377,7 +388,13 @@ pub(crate) mod file_sync_probe {
let _guard = BLOCK_MUTEX.lock().expect("file sync probe blocker poisoned");
BLOCK.store(block, Ordering::SeqCst);
}
*ROOT.write().expect("file sync probe lock poisoned") = Some(root.to_path_buf());
let mut roots = vec![root.to_path_buf()];
if let Ok(canonical) = root.canonicalize()
&& canonical != root
{
roots.push(canonical);
}
*ROOTS.write().expect("file sync probe lock poisoned") = roots;
ProbeGuard
}
@@ -398,11 +415,11 @@ pub(crate) mod file_sync_probe {
}
pub(super) fn enter(path: &Path) -> Option<ActiveGuard> {
let enabled = ROOT
.read()
.expect("file sync probe lock poisoned")
.as_ref()
.is_some_and(|root| path.starts_with(root));
let canonical_path = path.canonicalize().ok();
let enabled =
ROOTS.read().expect("file sync probe lock poisoned").iter().any(|root| {
path.starts_with(root) || canonical_path.as_ref().is_some_and(|canonical| canonical.starts_with(root))
});
if !enabled {
return None;
}
@@ -1439,7 +1439,7 @@ async fn try_create_bitrot_readers_via_batch_pread(
if let Some(disk) = disk_op.as_ref() {
let data_dir = files[idx].data_dir.unwrap_or_default();
let path_str = format!("{object}/{data_dir}/part.{part_number}");
match disk.get_object_path_if_local(bucket, &path_str) {
match disk.get_object_path_for_io_if_local(bucket, &path_str) {
Some(Ok(p)) => batch_items.push((idx, p, adj_off, adj_len)),
_ => return None,
}
+177 -16
View File
@@ -331,6 +331,85 @@ fn warn_heal_writer_failures(
}
impl SetDisks {
/// Read back one healed version from every explicitly admitted replacement
/// target. This is intentionally separate from the normal heal result: a
/// successful result describes the transaction attempt, while automatic
/// replacement completion needs physical evidence that survives a crash
/// before its checkpoint is persisted.
pub(crate) async fn replacement_targets_have_version(
&self,
bucket: &str,
object: &str,
version_id: &str,
targets: &[String],
) -> disk::error::Result<bool> {
let disks = self.get_disks_internal().await;
let mut target_disks = Vec::with_capacity(targets.len());
for target in targets {
let Some(index) = self.set_endpoints.iter().position(|endpoint| endpoint.to_string() == *target) else {
return Ok(false);
};
let Some(disk) = disks.get(index).and_then(Option::as_ref) else {
return Ok(false);
};
target_disks.push(disk.clone());
}
let read_options = ReadOptions {
incl_free_versions: false,
read_data: true,
healing: true,
};
let checks = target_disks.into_iter().map(|disk| {
let read_options = read_options.clone();
async move {
let file_info = match disk.read_version("", bucket, object, version_id, &read_options).await {
Ok(file_info) => file_info,
Err(
DiskError::DiskNotFound
| DiskError::VolumeNotFound
| DiskError::FileNotFound
| DiskError::FileVersionNotFound
| DiskError::PathNotFound,
) => return Ok(false),
Err(err) => return Err(err),
};
if !file_info_is_valid_for_metadata(&file_info) {
return Ok(false);
}
if !version_id.is_empty() && file_info.version_id.as_ref().map(ToString::to_string).as_deref() != Some(version_id)
{
return Ok(false);
}
if file_info.is_canonical_delete_marker() || file_info.is_remote() {
return Ok(true);
}
if (file_info.data.is_some() || file_info.size == 0) && !file_info.parts.is_empty() {
return Ok(true);
}
let check = match disk.check_parts(bucket, object, &file_info).await {
Ok(check) => check,
Err(
DiskError::DiskNotFound
| DiskError::VolumeNotFound
| DiskError::FileNotFound
| DiskError::FileVersionNotFound
| DiskError::PathNotFound,
) => return Ok(false),
Err(err) => return Err(err),
};
Ok(!check.results.is_empty() && check.results.iter().all(|result| *result == CHECK_PART_SUCCESS))
}
});
Ok(futures::future::try_join_all(checks)
.await?
.into_iter()
.all(|committed| committed))
}
#[tracing::instrument(level = "trace", skip(self, opts), fields(bucket = %bucket, object = %object, version_id = %version_id))]
pub(in crate::set_disk) async fn heal_object(
&self,
@@ -1711,19 +1790,35 @@ impl SetDisks {
}
}
// Heal operation family: the storage-api `HealOperations` contract stays
// implemented `for SetDisks` (contract bounds unchanged) but now lives beside
// its inherent helpers in the `set_disk::ops::heal` module. Bodies are moved
// unchanged; `get_pool_and_set` reads the core through `SetDisksCtx` to keep
// the Heal family aligned with the borrow pattern from #816.
#[async_trait::async_trait]
impl crate::storage_api_contracts::heal::HealOperations for SetDisks {
type Error = Error;
type HealResultItem = HealResultItem;
type HealOptions = HealOpts;
impl SetDisks {
pub(crate) async fn heal_replacement_format(
&self,
dry_run: bool,
targets: &[String],
) -> Result<(HealResultItem, Option<Error>)> {
if targets.is_empty() {
return Err(Error::other("replacement format requires at least one target"));
}
#[tracing::instrument(skip(self))]
async fn heal_format(&self, dry_run: bool) -> Result<(HealResultItem, Option<Error>)> {
let mut target_slots = Vec::with_capacity(targets.len());
for target in targets {
let Some(slot) = self.set_endpoints.iter().position(|endpoint| endpoint.to_string() == *target) else {
return Err(Error::other("replacement format target does not belong to the set"));
};
if target_slots.contains(&slot) {
return Err(Error::other("replacement format target is duplicated"));
}
target_slots.push(slot);
}
self.heal_format_for_slots(dry_run, Some(&target_slots)).await
}
async fn heal_format_for_slots(
&self,
dry_run: bool,
target_slots: Option<&[usize]>,
) -> Result<(HealResultItem, Option<Error>)> {
let disks = self.disks.read().await.clone();
let (formats, errs) = load_format_erasure_all(&disks, true).await;
if errs.iter().any(|err| {
@@ -1785,21 +1880,43 @@ impl crate::storage_api_contracts::heal::HealOperations for SetDisks {
if !dry_run {
for (disk_idx, err) in errs.iter().enumerate() {
if !matches!(err, Some(DiskError::UnformattedDisk)) {
if !matches!(err, Some(DiskError::UnformattedDisk))
|| target_slots.is_some_and(|slots| !slots.contains(&disk_idx))
{
continue;
}
let mut new_format = ref_format.clone();
new_format.erasure.this = ref_format.erasure.sets[self.set_index][disk_idx];
if save_format_file(&disks[disk_idx], &Some(new_format.clone())).await.is_ok() {
result.after.drives[disk_idx].uuid = new_format.erasure.this.to_string();
result.after.drives[disk_idx].state = DriveState::Ok.to_string();
match save_format_file(&disks[disk_idx], &Some(new_format.clone())).await {
Ok(()) => {
result.after.drives[disk_idx].uuid = new_format.erasure.this.to_string();
result.after.drives[disk_idx].state = DriveState::Ok.to_string();
}
Err(err) => return Ok((result, Some(err.into()))),
}
}
}
Ok((result, None))
}
}
// Heal operation family: the storage-api `HealOperations` contract stays
// implemented `for SetDisks` (contract bounds unchanged) but now lives beside
// its inherent helpers in the `set_disk::ops::heal` module. Bodies are moved
// unchanged; `get_pool_and_set` reads the core through `SetDisksCtx` to keep
// the Heal family aligned with the borrow pattern from #816.
#[async_trait::async_trait]
impl crate::storage_api_contracts::heal::HealOperations for SetDisks {
type Error = Error;
type HealResultItem = HealResultItem;
type HealOptions = HealOpts;
#[tracing::instrument(skip(self))]
async fn heal_format(&self, dry_run: bool) -> Result<(HealResultItem, Option<Error>)> {
self.heal_format_for_slots(dry_run, None).await
}
#[tracing::instrument(skip(self))]
async fn heal_bucket(&self, bucket: &str, opts: &HealOpts) -> Result<HealResultItem> {
@@ -2397,6 +2514,50 @@ mod heal_result_report_tests {
}
}
#[tokio::test]
async fn replacement_target_readback_requires_the_committed_shard() {
let (temp_dirs, disks, set) = hermetic_set_disks_isolated(4).await;
let bucket = "replacement-target-readback";
let object = "object.bin";
for disk in &disks {
disk.make_volume(bucket).await.expect("bucket volume should be created");
}
let mut reader = PutObjReader::from_vec(vec![0x5a; 1024 * 1024]);
set.put_object(bucket, object, &mut reader, &ObjectOptions::default())
.await
.expect("source object should be written");
let source = disks[2]
.read_version("", bucket, object, "", &ReadOptions::default())
.await
.expect("source metadata should be readable");
let data_dir = source.data_dir.expect("non-inline source should have a data directory");
let targets = vec![set.set_endpoints[0].to_string(), set.set_endpoints[1].to_string()];
assert!(
set.replacement_targets_have_version(bucket, object, "", &targets)
.await
.expect("healthy target shards should be readable")
);
tokio::fs::remove_file(
temp_dirs[1]
.path()
.join(bucket)
.join(object)
.join(data_dir.to_string())
.join("part.1"),
)
.await
.expect("target shard should be removed after the initial commit");
assert!(
!set.replacement_targets_have_version(bucket, object, "", &targets)
.await
.expect("missing target shard should be observable")
);
}
#[tokio::test]
async fn format_heal_cached_layout_rejects_a_disk_from_another_slot() {
let mut _temp_dirs = Vec::new();
+8 -2
View File
@@ -52,7 +52,10 @@ fn validate_table_bucket_delete_allowed(
async fn table_catalog_metadata_exists(ctx: &crate::runtime::instance::InstanceContext, bucket: &str) -> Result<bool> {
let local_disks = runtime_sources::local_disks_in(ctx).await;
for disk in local_disks.iter() {
let catalog_path = disk.path().join(bucket).join(BUCKET_TABLE_RESERVED_PREFIX);
let Some(bucket_path) = disk.get_bucket_path_for_io_if_local(bucket) else {
continue;
};
let catalog_path = bucket_path?.join(BUCKET_TABLE_RESERVED_PREFIX);
if has_xlmeta_files(&catalog_path).await? {
return Ok(true);
}
@@ -727,7 +730,10 @@ impl ECStore {
if !opts.force {
let local_disks = runtime_sources::local_disks_in(&self.ctx).await;
for disk in local_disks.iter() {
let bucket_path = disk.path().join(bucket);
let Some(bucket_path) = disk.get_bucket_path_for_io_if_local(bucket) else {
continue;
};
let bucket_path = bucket_path?;
if has_xlmeta_files(&bucket_path).await? {
return Err(StorageError::BucketNotEmpty(bucket.to_string()));
}
+50
View File
@@ -97,6 +97,56 @@ impl ECStore {
Ok((r, None))
}
#[instrument(skip(self, targets), fields(pool_index, set_index, target_count = targets.len()))]
pub async fn heal_replacement_format(
&self,
dry_run: bool,
pool_index: usize,
set_index: usize,
targets: &[String],
) -> Result<(HealResultItem, Option<Error>)> {
let pool = self
.pools
.get(pool_index)
.ok_or_else(|| invalid_heal_pool_index(pool_index, self.pools.len()))?;
let set = pool.disk_set.get(set_index).cloned().ok_or_else(|| {
StorageError::InvalidArgument(
"heal".to_string(),
"set".to_string(),
format!("invalid heal set index {set_index} for pool {pool_index}"),
)
})?;
set.heal_replacement_format(dry_run, targets).await
}
#[instrument(skip(self, targets), fields(pool_index, set_index, target_count = targets.len()))]
pub async fn replacement_targets_have_version(
&self,
bucket: &str,
object: &str,
version_id: &str,
pool_index: usize,
set_index: usize,
targets: &[String],
) -> Result<bool> {
let pool = self
.pools
.get(pool_index)
.ok_or_else(|| invalid_heal_pool_index(pool_index, self.pools.len()))?;
let set = pool.disk_set.get(set_index).cloned().ok_or_else(|| {
StorageError::InvalidArgument(
"heal".to_string(),
"set".to_string(),
format!("invalid heal set index {set_index} for pool {pool_index}"),
)
})?;
set.replacement_targets_have_version(bucket, object, version_id, targets)
.await
.map_err(Into::into)
}
#[instrument(skip(self))]
pub(super) async fn handle_heal_bucket(&self, bucket: &str, opts: &HealOpts) -> Result<HealResultItem> {
let res = self.peer_sys.heal_bucket(bucket, opts).await?;