perf(get,heal): fix GET hot-path overhead and heal checkpoint scaling (#4237)

perf(get,heal): land verified fixes for backlog #800-#804

Five fixes from the GET-path performance audit and scanner/heal
completeness audit (rustfs/backlog#800..#804), each verified locally:

- backlog#800 (heal checkpoint O(N^2)): ResumeCheckpoint object sets are
  now HashSet (Vec::contains was O(n) per healed object; 1.5ms at n=1M
  vs 11ns measured), and per-object checkpoint/resume-state persistence
  is batched (1000 mutations / 5s) instead of rewriting the whole file
  per object. complete_page() prunes the sets at page boundaries so
  memory stays bounded; positions still persist unconditionally and
  legacy Vec-format checkpoints still deserialize.

- backlog#801 (DiskInfo.healing never set): erasure-set heal now writes
  a healing marker (.rustfs.sys/healing.bin) on the disks it rebuilds
  (endpoints plumbed via HealRequest/HealTask.heal_endpoints from the
  auto disk scanner) and clears it on success. LocalDisk::disk_info
  surfaces the marker, so scanner heal coordination, lock selection and
  admin/metrics healing counts see the rebuild.

- backlog#802 (cache probe after data read): new GetObjectBodyCacheHook
  in ecstore lets the app-layer object data cache serve the body inside
  get_object_reader, after metadata quorum resolution (etag known) but
  before the erasure shard read/decode. Previously a hit still paid the
  full disk read. Hook is None/no-op when the cache is disabled.

- backlog#803 (GET hot-path redundant work): ObjectInfo is cloned for
  event notification only when an event will actually be built (GET and
  HEAD paths; events are currently suppressed so the clone was pure
  waste); get_opts/put_opts/del_opts resolve bucket versioning with one
  metadata-sys lookup instead of two; skip_verify_bitrot and
  get_lock_acquire_timeout env reads are cached via OnceLock; the
  io-priority metric is no longer double-counted; GetObject input fields
  are cloned selectively instead of cloning the whole input.

- backlog#804 (disk permit starvation): the disk-read permit wait is now
  bounded (RUSTFS_OBJECT_DISK_PERMIT_WAIT_TIMEOUT, default 5s, 0 =
  previous unbounded behavior); on timeout the GET proceeds without a
  permit and the bypass is counted. DiskReadPermitReader also releases
  the permit at body EOF instead of holding it until the client drops
  the stream.

Verification: make pre-commit; cargo clippy -D warnings on the four
changed crates; full rustfs lib suite (2096 tests) green; rustfs-heal
lib suite green with new unit tests for checkpoint pruning/legacy
format/throttle, permit EOF release, and the cache hook (hit + SSE
skip). The heal_integration_test and one set_disk listing test fail
identically on unmodified main (pre-existing global-state ordering
flakes, verified via git stash A/B).

Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
Zhengchao An
2026-07-03 21:42:27 +08:00
committed by GitHub
parent 769549dd2c
commit 92402a3bde
22 changed files with 690 additions and 90 deletions
+1 -3
View File
@@ -677,9 +677,7 @@ impl ErasureSetHealer {
}
*current_object_index = global_obj_idx;
checkpoint_manager
.update_position(bucket_index, *current_object_index)
.await?;
checkpoint_manager.complete_page(bucket_index, *current_object_index).await?;
gauge!(
"rustfs_heal_page_concurrency_current",
"set" => set_disk_id.to_string()
+1
View File
@@ -2371,6 +2371,7 @@ impl HealManager {
HealPriority::Low,
);
req.source = HealRequestSource::AutoHeal;
req.heal_endpoints = vec![ep.to_string()];
let config = config.read().await;
let mut queue = heal_queue.lock().await;
let admission = Self::admit_request_to_queue(&mut queue, req, &config, "auto_scan");
+62 -3
View File
@@ -24,9 +24,10 @@ pub mod task;
pub mod utils;
use storage_api::owner::{
ECSTORE_BUCKET_META_PREFIX, ECSTORE_DATA_USAGE_CACHE_NAME, ECSTORE_RUSTFS_META_BUCKET, EcstoreDeleteOptions, EcstoreDiskAPI,
EcstoreDiskBytes, EcstoreDiskError, EcstoreDiskResult, EcstoreDiskStore, EcstoreEndpoint, EcstoreErrorType,
EcstoreStorageError, EcstoreStore, ObjectIO, ObjectOperations, ecstore_local_disk_map_read,
ECSTORE_BUCKET_META_PREFIX, ECSTORE_DATA_USAGE_CACHE_NAME, ECSTORE_HEALING_MARKER_PATH, ECSTORE_RUSTFS_META_BUCKET,
EcstoreDeleteOptions, EcstoreDiskAPI, EcstoreDiskBytes, EcstoreDiskError, EcstoreDiskResult, EcstoreDiskStore,
EcstoreEndpoint, EcstoreErrorType, EcstoreStorageError, EcstoreStore, ObjectIO, ObjectOperations,
ecstore_local_disk_map_read,
};
#[cfg(test)]
use storage_api::owner::{EcstoreDiskOption, ecstore_new_disk};
@@ -69,6 +70,64 @@ pub async fn clear_unclean_shutdown_markers() {
}
}
/// Per-disk healing marker path (inside `RUSTFS_META_BUCKET`), mirrored from
/// ecstore so both sides agree on where `DiskInfo.healing` is derived from.
pub(crate) const HEALING_MARKER_PATH: &str = ECSTORE_HEALING_MARKER_PATH;
/// Write the healing marker on the local disks matching `endpoints` so their
/// `DiskInfo.healing` reports true while the erasure-set heal rebuilds them.
pub(crate) async fn set_healing_markers(endpoints: &[String], set_disk_id: &str) {
apply_healing_markers(endpoints, Some(set_disk_id)).await;
}
/// Remove the healing markers written by [`set_healing_markers`].
pub(crate) async fn clear_healing_markers(endpoints: &[String]) {
apply_healing_markers(endpoints, None).await;
}
async fn apply_healing_markers(endpoints: &[String], set_disk_id: Option<&str>) {
if endpoints.is_empty() {
return;
}
let local_disk_map = local_disk_map_read().await;
for disk in local_disk_map.values().flatten() {
let endpoint = EcstoreDiskAPI::endpoint(disk.as_ref()).to_string();
if !endpoints.iter().any(|candidate| candidate == &endpoint) {
continue;
}
let result = match set_disk_id {
Some(set_disk_id) => {
EcstoreDiskAPI::write_all(
disk.as_ref(),
RUSTFS_META_BUCKET,
HEALING_MARKER_PATH,
EcstoreDiskBytes::copy_from_slice(set_disk_id.as_bytes()),
)
.await
}
None => match EcstoreDiskAPI::delete(
disk.as_ref(),
RUSTFS_META_BUCKET,
HEALING_MARKER_PATH,
EcstoreDeleteOptions::default(),
)
.await
{
Err(DiskError::FileNotFound) => Ok(()),
other => other,
},
};
if let Err(err) = result {
tracing::warn!(
endpoint = %endpoint,
action = if set_disk_id.is_some() { "set" } else { "clear" },
error = ?err,
"failed to update healing marker"
);
}
}
}
pub(crate) type DiskError = EcstoreDiskError;
pub(crate) type DiskResult<T> = EcstoreDiskResult<T>;
pub(crate) type DiskStore = EcstoreDiskStore;
+170 -30
View File
@@ -14,9 +14,10 @@
use crate::{Error, Result};
use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use std::path::Path;
use std::sync::Arc;
use std::time::{SystemTime, UNIX_EPOCH};
use std::sync::{Arc, Mutex};
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
use tokio::sync::RwLock;
use tracing::{debug, warn};
use uuid::Uuid;
@@ -33,6 +34,39 @@ const RESUME_STATE_FILE: &str = "ahm_resume_state.json";
const RESUME_PROGRESS_FILE: &str = "ahm_progress.json";
const RESUME_CHECKPOINT_FILE: &str = "ahm_checkpoint.json";
/// Persistence throttle for per-object bookkeeping: flush after this many
/// buffered mutations or once the interval elapses, whichever comes first.
/// Object heal is idempotent, so a crash re-heals at most one throttle window.
const PERSIST_EVERY_MUTATIONS: usize = 1000;
const PERSIST_INTERVAL: Duration = Duration::from_secs(5);
/// Tracks buffered mutations between persisted snapshots.
#[derive(Debug)]
struct PersistThrottle {
pending: usize,
last_save: Instant,
}
impl PersistThrottle {
fn new() -> Self {
Self {
pending: 0,
last_save: Instant::now(),
}
}
/// Record one mutation; returns true when the batch should be flushed.
fn record(&mut self) -> bool {
self.pending += 1;
self.pending >= PERSIST_EVERY_MUTATIONS || self.last_save.elapsed() >= PERSIST_INTERVAL
}
fn mark_saved(&mut self) {
self.pending = 0;
self.last_save = Instant::now();
}
}
/// Helper function to convert Path to &str, returning an error if conversion fails
fn path_to_str(path: &Path) -> Result<&str> {
path.to_str()
@@ -168,6 +202,7 @@ impl ResumeState {
pub struct ResumeManager {
disk: DiskStore,
state: Arc<RwLock<ResumeState>>,
throttle: Mutex<PersistThrottle>,
}
impl ResumeManager {
@@ -183,6 +218,7 @@ impl ResumeManager {
let manager = Self {
disk,
state: Arc::new(RwLock::new(state)),
throttle: Mutex::new(PersistThrottle::new()),
};
// save initial state
@@ -210,6 +246,7 @@ impl ResumeManager {
Ok(Self {
disk,
state: Arc::new(RwLock::new(state)),
throttle: Mutex::new(PersistThrottle::new()),
})
}
@@ -235,15 +272,31 @@ impl ResumeManager {
let mut state = self.state.write().await;
state.update_progress(processed, successful, failed, skipped);
drop(state);
self.save_state().await
self.save_state_throttled().await
}
/// set current item
/// Set current item. Called once per healed object, so persistence is
/// throttled: the in-memory state always updates, but the snapshot is only
/// written every `PERSIST_EVERY_MUTATIONS` calls or `PERSIST_INTERVAL`.
pub async fn set_current_item(&self, bucket: Option<String>, object: Option<String>) -> Result<()> {
let mut state = self.state.write().await;
state.set_current_item(bucket, object);
drop(state);
self.save_state().await
let should_save = self.throttle.lock().map(|mut throttle| throttle.record()).unwrap_or(true);
if !should_save {
return Ok(());
}
self.save_state_throttled().await
}
async fn save_state_throttled(&self) -> Result<()> {
let result = self.save_state().await;
if result.is_ok()
&& let Ok(mut throttle) = self.throttle.lock()
{
throttle.mark_saved();
}
result
}
/// complete bucket
@@ -251,7 +304,7 @@ impl ResumeManager {
let mut state = self.state.write().await;
state.complete_bucket(bucket);
drop(state);
self.save_state().await
self.save_state_throttled().await
}
/// mark task completed
@@ -259,7 +312,7 @@ impl ResumeManager {
let mut state = self.state.write().await;
state.mark_completed();
drop(state);
self.save_state().await
self.save_state_throttled().await
}
/// set error message
@@ -376,12 +429,15 @@ pub struct ResumeCheckpoint {
pub current_bucket_index: usize,
/// current object index
pub current_object_index: usize,
/// processed objects
pub processed_objects: Vec<String>,
/// Objects healed since the last completed page. HashSet: with the
/// previous Vec the per-object `contains` was O(n) and made large-bucket
/// heals O(N²). Only spans the in-flight page — completed pages are
/// covered by `current_object_index`, so `complete_page` prunes the sets.
pub processed_objects: HashSet<String>,
/// failed objects
pub failed_objects: Vec<String>,
pub failed_objects: HashSet<String>,
/// skipped objects
pub skipped_objects: Vec<String>,
pub skipped_objects: HashSet<String>,
}
impl ResumeCheckpoint {
@@ -391,9 +447,9 @@ impl ResumeCheckpoint {
checkpoint_time: SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs(),
current_bucket_index: 0,
current_object_index: 0,
processed_objects: Vec::new(),
failed_objects: Vec::new(),
skipped_objects: Vec::new(),
processed_objects: HashSet::new(),
failed_objects: HashSet::new(),
skipped_objects: HashSet::new(),
}
}
@@ -404,21 +460,25 @@ impl ResumeCheckpoint {
}
pub fn add_processed_object(&mut self, object: String) {
if !self.processed_objects.contains(&object) {
self.processed_objects.push(object);
}
self.processed_objects.insert(object);
}
pub fn add_failed_object(&mut self, object: String) {
if !self.failed_objects.contains(&object) {
self.failed_objects.push(object);
}
self.failed_objects.insert(object);
}
pub fn add_skipped_object(&mut self, object: String) {
if !self.skipped_objects.contains(&object) {
self.skipped_objects.push(object);
}
self.skipped_objects.insert(object);
}
/// Advance past a fully-processed page: objects below `object_index` are
/// skipped by position on resume, so the per-object sets no longer need
/// their entries and would otherwise grow with the whole bucket.
pub fn complete_page(&mut self, bucket_index: usize, object_index: usize) {
self.update_position(bucket_index, object_index);
self.processed_objects.clear();
self.skipped_objects.clear();
self.failed_objects.clear();
}
}
@@ -426,6 +486,7 @@ impl ResumeCheckpoint {
pub struct CheckpointManager {
disk: DiskStore,
checkpoint: Arc<RwLock<ResumeCheckpoint>>,
throttle: Mutex<PersistThrottle>,
}
impl CheckpointManager {
@@ -435,6 +496,7 @@ impl CheckpointManager {
let manager = Self {
disk,
checkpoint: Arc::new(RwLock::new(checkpoint)),
throttle: Mutex::new(PersistThrottle::new()),
};
// save initial checkpoint
@@ -462,6 +524,7 @@ impl CheckpointManager {
Ok(Self {
disk,
checkpoint: Arc::new(RwLock::new(checkpoint)),
throttle: Mutex::new(PersistThrottle::new()),
})
}
@@ -487,31 +550,59 @@ impl CheckpointManager {
let mut checkpoint = self.checkpoint.write().await;
checkpoint.update_position(bucket_index, object_index);
drop(checkpoint);
self.save_checkpoint().await
self.save_checkpoint_throttled().await
}
/// add processed object
/// Advance past a completed page and prune the per-object sets, then persist.
pub async fn complete_page(&self, bucket_index: usize, object_index: usize) -> Result<()> {
let mut checkpoint = self.checkpoint.write().await;
checkpoint.complete_page(bucket_index, object_index);
drop(checkpoint);
self.save_checkpoint_throttled().await
}
/// Add a processed object. Called once per healed object, so persistence
/// is batched (`PERSIST_EVERY_MUTATIONS` / `PERSIST_INTERVAL`); positions
/// and page boundaries still persist unconditionally.
pub async fn add_processed_object(&self, object: String) -> Result<()> {
let mut checkpoint = self.checkpoint.write().await;
checkpoint.add_processed_object(object);
drop(checkpoint);
self.save_checkpoint().await
self.save_checkpoint_if_due().await
}
/// add failed object
/// add failed object (batched, see `add_processed_object`)
pub async fn add_failed_object(&self, object: String) -> Result<()> {
let mut checkpoint = self.checkpoint.write().await;
checkpoint.add_failed_object(object);
drop(checkpoint);
self.save_checkpoint().await
self.save_checkpoint_if_due().await
}
/// add skipped object
/// add skipped object (batched, see `add_processed_object`)
pub async fn add_skipped_object(&self, object: String) -> Result<()> {
let mut checkpoint = self.checkpoint.write().await;
checkpoint.add_skipped_object(object);
drop(checkpoint);
self.save_checkpoint().await
self.save_checkpoint_if_due().await
}
async fn save_checkpoint_if_due(&self) -> Result<()> {
let should_save = self.throttle.lock().map(|mut throttle| throttle.record()).unwrap_or(true);
if !should_save {
return Ok(());
}
self.save_checkpoint_throttled().await
}
async fn save_checkpoint_throttled(&self) -> Result<()> {
let result = self.save_checkpoint().await;
if result.is_ok()
&& let Ok(mut throttle) = self.throttle.lock()
{
throttle.mark_saved();
}
result
}
/// cleanup checkpoint
@@ -732,6 +823,55 @@ mod tests {
assert!(state.completed_buckets.contains(&"bucket1".to_string()));
}
#[test]
fn test_checkpoint_object_sets_dedupe_and_prune() {
let mut checkpoint = ResumeCheckpoint::new("task".to_string());
checkpoint.add_processed_object("bucket/a".to_string());
checkpoint.add_processed_object("bucket/a".to_string());
checkpoint.add_skipped_object("bucket/b".to_string());
checkpoint.add_failed_object("bucket/c".to_string());
assert_eq!(checkpoint.processed_objects.len(), 1);
assert!(checkpoint.processed_objects.contains("bucket/a"));
checkpoint.complete_page(2, 2000);
assert_eq!(checkpoint.current_bucket_index, 2);
assert_eq!(checkpoint.current_object_index, 2000);
assert!(checkpoint.processed_objects.is_empty());
assert!(checkpoint.skipped_objects.is_empty());
assert!(checkpoint.failed_objects.is_empty());
}
#[test]
fn test_checkpoint_loads_legacy_vec_format() {
// Checkpoints written before the HashSet migration stored the object
// lists as JSON arrays (possibly with duplicates); they must still load.
let legacy = r#"{
"task_id": "t1",
"checkpoint_time": 1700000000,
"current_bucket_index": 1,
"current_object_index": 42,
"processed_objects": ["a", "b", "a"],
"failed_objects": [],
"skipped_objects": ["c"]
}"#;
let checkpoint: ResumeCheckpoint = serde_json::from_str(legacy).unwrap();
assert_eq!(checkpoint.current_object_index, 42);
assert_eq!(checkpoint.processed_objects.len(), 2);
assert!(checkpoint.processed_objects.contains("a"));
assert!(checkpoint.skipped_objects.contains("c"));
}
#[test]
fn test_persist_throttle_batches_until_threshold() {
let mut throttle = PersistThrottle::new();
for _ in 0..PERSIST_EVERY_MUTATIONS - 1 {
assert!(!throttle.record(), "must not flush below the mutation threshold");
}
assert!(throttle.record(), "must flush at the mutation threshold");
throttle.mark_saved();
assert!(!throttle.record(), "counter must reset after a save");
}
#[tokio::test]
async fn test_resume_utils() {
let task_id1 = ResumeUtils::generate_task_id();
+5 -4
View File
@@ -17,7 +17,8 @@ pub(crate) use rustfs_ecstore::api::disk::endpoint::Endpoint as EcstoreEndpoint;
pub(crate) use rustfs_ecstore::api::disk::error::{DiskError as EcstoreDiskError, Result as EcstoreDiskResult};
pub(crate) use rustfs_ecstore::api::disk::{
BUCKET_META_PREFIX as ECSTORE_BUCKET_META_PREFIX, Bytes as EcstoreDiskBytes, DeleteOptions as EcstoreDeleteOptions,
DiskAPI as EcstoreDiskAPI, DiskStore as EcstoreDiskStore, RUSTFS_META_BUCKET as ECSTORE_RUSTFS_META_BUCKET,
DiskAPI as EcstoreDiskAPI, DiskStore as EcstoreDiskStore, HEALING_MARKER_PATH as ECSTORE_HEALING_MARKER_PATH,
RUSTFS_META_BUCKET as ECSTORE_RUSTFS_META_BUCKET,
};
#[cfg(test)]
pub(crate) use rustfs_ecstore::api::disk::{DiskOption as EcstoreDiskOption, new_disk as ecstore_new_disk};
@@ -30,9 +31,9 @@ pub(crate) mod owner {
pub(crate) use super::storage_contracts::{ObjectIO, ObjectOperations};
pub(crate) use super::{
ECSTORE_BUCKET_META_PREFIX, ECSTORE_DATA_USAGE_CACHE_NAME, ECSTORE_RUSTFS_META_BUCKET, EcstoreDeleteOptions,
EcstoreDiskAPI, EcstoreDiskBytes, EcstoreDiskError, EcstoreDiskResult, EcstoreDiskStore, EcstoreEndpoint,
EcstoreErrorType, EcstoreStorageError, EcstoreStore, ecstore_local_disk_map_read,
ECSTORE_BUCKET_META_PREFIX, ECSTORE_DATA_USAGE_CACHE_NAME, ECSTORE_HEALING_MARKER_PATH, ECSTORE_RUSTFS_META_BUCKET,
EcstoreDeleteOptions, EcstoreDiskAPI, EcstoreDiskBytes, EcstoreDiskError, EcstoreDiskResult, EcstoreDiskStore,
EcstoreEndpoint, EcstoreErrorType, EcstoreStorageError, EcstoreStore, ecstore_local_disk_map_read,
};
#[cfg(test)]
+20
View File
@@ -198,6 +198,11 @@ pub struct HealRequest {
pub force_start: bool,
/// Number of recoverable retry attempts already scheduled for this request.
pub retry_attempts: u32,
/// Endpoints of the disks being rebuilt by an erasure-set heal. Used to
/// write per-disk healing markers so `DiskInfo.healing` reflects reality;
/// empty when the trigger doesn't know the specific disks (admin API,
/// unclean-shutdown verification).
pub heal_endpoints: Vec<String>,
/// Created time
pub created_at: SystemTime,
/// Queue admission time used for scheduler delay metrics
@@ -215,6 +220,7 @@ impl HealRequest {
source: HealRequestSource::Internal,
force_start: false,
retry_attempts: 0,
heal_endpoints: Vec::new(),
created_at: now,
enqueued_at: now,
}
@@ -267,6 +273,8 @@ pub struct HealTask {
pub source: HealRequestSource,
/// Number of recoverable retry attempts already scheduled for this task.
pub retry_attempts: u32,
/// Endpoints of the disks being rebuilt (see `HealRequest::heal_endpoints`).
pub heal_endpoints: Vec<String>,
/// Task status
pub status: Arc<RwLock<HealTaskStatus>>,
/// Progress tracking
@@ -298,6 +306,7 @@ impl HealTask {
priority: request.priority,
source: request.source,
retry_attempts: request.retry_attempts,
heal_endpoints: request.heal_endpoints,
status: Arc::new(RwLock::new(HealTaskStatus::Pending)),
progress: Arc::new(RwLock::new(HealProgress::new())),
result_items: Arc::new(RwLock::new(Vec::new())),
@@ -320,6 +329,7 @@ impl HealTask {
source: self.source,
force_start: false,
retry_attempts: self.retry_attempts.saturating_add(1),
heal_endpoints: self.heal_endpoints.clone(),
created_at: self.created_at,
enqueued_at: SystemTime::now(),
}
@@ -2038,6 +2048,10 @@ impl HealTask {
progress.update_progress(1, 4, 0, 0);
}
// The rebuilt disks are formatted now: mark them as healing so
// DiskInfo.healing reflects the rebuild until it completes.
super::set_healing_markers(&self.heal_endpoints, &set_disk_id).await;
// Step 2: Get disk for resume functionality
debug!(
target: "rustfs::heal::task",
@@ -2152,6 +2166,12 @@ impl HealTask {
);
let result = erasure_healer.heal_erasure_set(&buckets, &set_disk_id).await;
// Keep the markers on failure: the resume state also persists, and the
// next run of this set heal re-marks and eventually clears them.
if result.is_ok() {
super::clear_healing_markers(&self.heal_endpoints).await;
}
{
let mut progress = self.progress.write().await;
progress.update_progress(4, 4, 0, 0);