chore(ecstore): drop the set_disk dead_code blanket (#6141)

* chore(ecstore): drop the set_disk dead_code blanket

Removing the blanket exposes 39 items; exactly one is deleted. The low share is a finding, not caution: unlike the disk root, where platform gating made local adjudication impossible, here the items were checked and nearly all of them are live.

Deleted: HealEntryResult, the only item with no reference anywhere.

What the checks turned up, in the order the warnings suggest deleting them:

SetDisks::rename_data looked like the head of a dead chain feeding into_legacy_tuple and RenameDataLegacyTuple. It is not: production goes through rename_data_owned, and rename_data itself has test callers at mod.rs:5809 and 5880. The chain below it is therefore live through the tests, and inferring "this is dead, so its callee is dead" would have removed three working items.

create_bitrot_readers_until_quorum, read_multiple_files and map_cleanup_join_result all have callers inside their files' test modules, so they only look dead in the lib target.

TransitionCommitBarrier and TransitionUploadedSaveProbe, with their install/wait_until_paused/release surfaces, are installed by tests behind #[cfg(all(test, feature = "test-util"))].

ctx.rs's SetDisksCtx accessors are the split seam left by the SetDisks god-object break-up (backlog#815).

heal_object_dir's two apparent references are comments, and they document an index-alignment contract that live code maintains for it, so they stay as they are.

Worth a maintainer decision: the metadata early-stop switch has a complete percentage-rollout facet — ENV_RUSTFS_GET_METADATA_EARLY_STOP_ROLLOUT_PCT, get_metadata_early_stop_rollout_pct and should_use_metadata_early_stop — with no caller, no test and no documentation, while its sibling enable flag is live. It is kept with an allow that says so rather than removed, since a rollout knob is a product call.

One placement note for anyone adding allows near heal code: check_logging_guardrails.sh requires #[instrument(level = "trace")] to sit immediately before async fn heal_object_dir, so the allow goes above the instrument attribute. Putting it between the two drops the guard's match count and fails the check.

Verification, four lanes warning-free: default, --tests, --features rio-v2 --tests, --features test-util --tests. cargo nextest run -p rustfs-ecstore 4096 passed; clippy --lib --tests -D warnings clean; make pre-commit exit 0.

Ref rustfs/backlog#1823 (step 2).

* chore(ecstore): fix duplicated and inaccurate dead_code reasons in set_disk

format_lock_error carried the same #[allow] twice. Five items in the
locking/heal roots were labelled 'asserted by this file's tests' while
having no reference at all - heal_object_dir's only two references are
comments, as this branch's own notes point out. Say what each item
actually is instead, so the next reader does not assume test coverage
that is not there.

Ref rustfs/backlog#1823.

* chore(ecstore): correct the bounded_spare_disk_index dead_code reason

The mod.rs copy is an unused test fixture, not something this module's
tests assert; the namesake that is exercised lives in the io_primitives
test module.

Ref rustfs/backlog#1823.
This commit is contained in:
Zhengchao An
2026-08-16 21:38:46 +08:00
committed by GitHub
parent 1eef0de003
commit f1f86ee9d0
9 changed files with 128 additions and 10 deletions
+1
View File
@@ -704,6 +704,7 @@ pub(crate) async fn create_bitrot_reader_from_bytes_with_stage_metrics(
}
#[allow(clippy::too_many_arguments)]
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub fn create_deferred_bitrot_reader(
inline_data: Option<Bytes>,
disk: Option<DiskStore>,
@@ -180,11 +180,13 @@ pub(in crate::set_disk) enum GetCodecStreamingReaderBuildOutcome {
Fallback(GetCodecStreamingFallbackReason),
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) struct MultipartCodecStreamingReader {
pub(in crate::set_disk) readers: VecDeque<Box<dyn AsyncRead + Unpin + Send + Sync>>,
}
impl MultipartCodecStreamingReader {
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) fn new(readers: Vec<Box<dyn AsyncRead + Unpin + Send + Sync>>) -> Self {
Self {
readers: VecDeque::from(readers),
@@ -1836,6 +1838,7 @@ pub(in crate::set_disk) async fn create_bitrot_readers_until_quorum_all_shards(
}
#[allow(clippy::too_many_arguments)]
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) async fn create_bitrot_readers_until_quorum(
files: &[FileInfo],
disks: &[Option<DiskStore>],
@@ -2126,6 +2129,7 @@ pub(in crate::set_disk) async fn create_data_block_bitrot_readers(
setup
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) async fn collect_read_multiple_results<F>(
tasks: Vec<F>,
read_quorum: usize,
@@ -2955,6 +2959,7 @@ impl SetDisks {
(meta_file_infos, errs)
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) async fn read_multiple_files(
disks: &[Option<DiskStore>],
req: ReadMultipleReq,
@@ -3134,6 +3139,7 @@ pub(in crate::set_disk) struct RenameDataCommit {
pub(in crate::set_disk) committed_file_info: FileInfo,
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
type RenameDataLegacyTuple = (
Vec<Option<DiskStore>>,
RenameConvergence,
@@ -3143,6 +3149,7 @@ type RenameDataLegacyTuple = (
);
impl RenameDataCommit {
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn into_legacy_tuple(self) -> RenameDataLegacyTuple {
(
self.online_disks,
@@ -3261,6 +3268,7 @@ impl SetDisks {
#[tracing::instrument(level = "debug", skip(disks, file_infos))]
#[allow(clippy::type_complexity)]
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) async fn rename_data(
disks: &[Option<DiskStore>],
src_bucket: &str,
@@ -5073,6 +5081,7 @@ fn is_cleanup_not_found(e: &DiskError) -> bool {
/// normalized to `DiskNotFound`: a panic is not a "disk absent" condition and
/// must not be silently swallowed as an ignorable error (fixes the historical
/// `Unexpected`/`DiskNotFound` misclassification).
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn map_cleanup_join_result(joined: std::result::Result<Option<DiskError>, tokio::task::JoinError>) -> Option<DiskError> {
match joined {
Ok(res) => res,
@@ -5297,6 +5306,7 @@ pub(in crate::set_disk) mod rename_fanout_barrier_phase {
/// The per-disk old-data-dir cleanup phase of the commit fan-out.
pub const CLEANUP: &str = "cleanup";
/// The per-disk `read_version` phase of metadata read fan-out.
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub const READ_VERSION: &str = "read_version";
}
+28
View File
@@ -42,12 +42,20 @@ impl<'a> SetDisksCtx<'a> {
}
/// The borrowed core, for state not yet fronted by a typed accessor.
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn core(&self) -> &'a SetDisks {
self.core
}
// --- Immutable topology / config (fixed after construction) ---
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn set_index(&self) -> usize {
self.core.set_index
}
@@ -56,14 +64,26 @@ impl<'a> SetDisksCtx<'a> {
self.core.pool_index
}
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn set_drive_count(&self) -> usize {
self.core.set_drive_count
}
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn default_parity_count(&self) -> usize {
self.core.default_parity_count
}
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn set_endpoints(&self) -> &'a [Endpoint] {
&self.core.set_endpoints
}
@@ -72,6 +92,10 @@ impl<'a> SetDisksCtx<'a> {
&self.core.format
}
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn locker_owner(&self) -> &'a str {
&self.core.locker_owner
}
@@ -84,6 +108,10 @@ impl<'a> SetDisksCtx<'a> {
// --- Locker trio ---
#[allow(
dead_code,
reason = "SetDisks split seam (backlog#815) with no caller in this port (backlog#1823)"
)]
pub(crate) fn lockers(&self) -> &'a [Arc<dyn LockClient>] {
&self.core.lockers
}
+30 -10
View File
@@ -39,7 +39,6 @@
//! - `metadata.rs`, `replication.rs`, `shard_source.rs` — supporting helpers.
// #730: SetDisks still hosts staged read/heal/write migration helpers.
#![allow(dead_code)]
#![allow(unused_imports)]
#![allow(unused_variables)]
@@ -624,7 +623,9 @@ fn adaptive_duplex_buffer_size(object_size: i64) -> usize {
// Each flag has a corresponding `*_ROLLOUT_PCT` for percentage-based gradual rollout.
// ============================================================================
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
const DISK_ONLINE_TIMEOUT: Duration = Duration::from_secs(1);
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
const DISK_HEALTH_CACHE_TTL: Duration = Duration::from_millis(750);
const GET_OBJECT_METADATA_CACHE_TTL: Duration = Duration::from_secs(2); // Increased from 250ms to 2s
const DEFAULT_GET_OBJECT_METADATA_CACHE_MAX_ENTRIES: usize = 4096; // Increased from 1024 to 4096
@@ -698,7 +699,15 @@ const ENV_RUSTFS_GET_METADATA_EARLY_STOP_ENABLE: &str = "RUSTFS_GET_METADATA_EAR
// the env var to `false` to fall back to full-wait metadata fanout.
const DEFAULT_RUSTFS_GET_METADATA_EARLY_STOP_ENABLE: bool = true;
#[allow(
dead_code,
reason = "percentage-rollout facet of the metadata early-stop switch; its predicate has no caller while the sibling enable flag is live (backlog#1823)"
)]
const ENV_RUSTFS_GET_METADATA_EARLY_STOP_ROLLOUT_PCT: &str = "RUSTFS_GET_METADATA_EARLY_STOP_ROLLOUT_PCT";
#[allow(
dead_code,
reason = "percentage-rollout facet of the metadata early-stop switch; its predicate has no caller while the sibling enable flag is live (backlog#1823)"
)]
const DEFAULT_RUSTFS_GET_METADATA_EARLY_STOP_ROLLOUT_PCT: u32 = 100;
const ENV_RUSTFS_GET_METADATA_VERSION_EARLY_STOP_ENABLE: &str = "RUSTFS_GET_METADATA_VERSION_EARLY_STOP_ENABLE";
@@ -910,6 +919,10 @@ mod prepared_get_object_metadata_tests {
.expect("test should find an object whose initial fanout covers both data shards")
}
#[allow(
dead_code,
reason = "test fixture no assertion in this module uses today; the live namesake lives in io_primitives tests (backlog#1823)"
)]
fn bounded_spare_disk_index(bucket: &str, object: &str) -> usize {
*bounded_metadata_fanout_order(bucket, object, 4, 2)
.get(3)
@@ -1709,6 +1722,10 @@ fn is_multipart_reader_setup_prefetch_enabled() -> bool {
}
}
#[allow(
dead_code,
reason = "percentage-rollout facet of the metadata early-stop switch; its predicate has no caller while the sibling enable flag is live (backlog#1823)"
)]
fn get_metadata_early_stop_rollout_pct() -> u32 {
static CACHED: OnceLock<u32> = OnceLock::new();
*CACHED.get_or_init(|| {
@@ -1748,6 +1765,10 @@ fn should_use_codec_streaming(config: GetCodecStreamingConfig, bucket: &str, obj
}
/// Should this specific request use metadata early-stop?
#[allow(
dead_code,
reason = "percentage-rollout facet of the metadata early-stop switch; its predicate has no caller while the sibling enable flag is live (backlog#1823)"
)]
pub fn should_use_metadata_early_stop(bucket: &str, object: &str) -> bool {
let base = is_get_metadata_early_stop_enabled();
let pct = get_metadata_early_stop_rollout_pct();
@@ -2181,6 +2202,7 @@ fn classify_get_codec_streaming_object_class(
GetCodecStreamingObjectClass::PlainSinglePart
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn is_get_small_object_direct_memory_eligible_with_threshold(
range: &Option<HTTPRangeSpec>,
object_info: &ObjectInfo,
@@ -2786,6 +2808,7 @@ pub struct SetDisks {
/// Stable namespace shared by every object lock created for this set.
set_lock_namespace: Arc<str>,
pub format: FormatV3,
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
disk_health_cache: Arc<RwLock<Vec<Option<DiskHealthEntry>>>>,
get_object_metadata_cache: moka::future::Cache<GetObjectMetadataCacheKey, Arc<GetObjectMetadataCacheEntry>>,
get_object_metadata_cache_hash_builder: std::collections::hash_map::RandomState,
@@ -3061,11 +3084,13 @@ struct GetObjectMetadataCacheEntry {
#[derive(Clone, Debug)]
struct DiskHealthEntry {
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
last_check: Instant,
online: bool,
}
impl DiskHealthEntry {
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn cached_value(&self) -> Option<bool> {
if self.last_check.elapsed() <= DISK_HEALTH_CACHE_TTL {
Some(self.online)
@@ -3659,6 +3684,7 @@ fn multipart_put_large_batch_min_size_bytes() -> usize {
})
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn classify_small_write_path(is_inline_buffer: bool, object_size: i64, block_size: usize) -> SmallWritePath {
if should_use_inline_small_fast_path(is_inline_buffer, object_size, block_size) {
SmallWritePath::Inline
@@ -4237,6 +4263,7 @@ fn check_object_lock_retention_update(bucket: &str, object: &str, obj_info: &Obj
///
/// Fail closed: when bucket metadata cannot be resolved the check stays on, so
/// object-lock protection is never skipped because of a metadata lookup miss.
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(crate) fn object_lock_delete_check_required(bucket_meta: Option<&crate::bucket::metadata::BucketMetadata>) -> bool {
bucket_meta.is_none_or(|meta| meta.object_locking())
}
@@ -4512,15 +4539,6 @@ impl Hash for ObjProps {
}
}
#[derive(Default, Clone, Debug)]
pub struct HealEntryResult {
pub bytes: usize,
pub success: bool,
pub skipped: bool,
pub entry_done: bool,
pub name: String,
}
fn is_object_dangling(
meta_arr: &[FileInfo],
errs: &[Option<DiskError>],
@@ -5297,6 +5315,7 @@ pub fn is_valid_storage_class(storage_class: &str) -> bool {
}
/// Returns true if the storage class is a cold storage tier that requires special handling
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub fn is_cold_storage_class(storage_class: &str) -> bool {
matches!(
storage_class,
@@ -5305,6 +5324,7 @@ pub fn is_cold_storage_class(storage_class: &str) -> bool {
}
/// Returns true if the storage class is an infrequent access tier
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub fn is_infrequent_access_class(storage_class: &str) -> bool {
matches!(
storage_class,
+4
View File
@@ -1716,6 +1716,10 @@ impl SetDisks {
Ok((result, None))
}
#[allow(
dead_code,
reason = "lock-taking wrapper over the live heal_object_dir_locked; only comments reference it (backlog#1823)"
)]
#[tracing::instrument(level = "trace", skip(self), fields(bucket = %bucket, object = %object))]
pub(in crate::set_disk) async fn heal_object_dir(
&self,
@@ -66,6 +66,7 @@ impl crate::storage_api_contracts::namespace::NamespaceLocking for SetDisks {
}
impl SetDisks {
#[allow(dead_code, reason = "lock diagnostics formatter with no caller in this port (backlog#1823)")]
pub(in crate::set_disk) fn format_lock_error(&self, bucket: &str, object: &str, mode: &str, err: &LockResult) -> String {
match err {
LockResult::Timeout => {
@@ -79,6 +80,7 @@ impl SetDisks {
}
}
#[allow(dead_code, reason = "lock diagnostics formatter with no caller in this port (backlog#1823)")]
pub(in crate::set_disk) fn format_lock_error_from_error(
&self,
bucket: &str,
@@ -143,6 +145,7 @@ impl SetDisks {
disks
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(in crate::set_disk) async fn get_online_disks(&self) -> Vec<Option<DiskStore>> {
let snapshot = self.drive_membership_snapshot().await;
let mut disks = snapshot.strict_online_candidates().into_iter().map(Some).collect::<Vec<_>>();
@@ -153,6 +156,10 @@ impl SetDisks {
disks
}
#[allow(
dead_code,
reason = "local-only sibling of the test-covered get_online_disks; no caller in this port (backlog#1823)"
)]
pub(in crate::set_disk) async fn get_online_local_disks(&self) -> Vec<Option<DiskStore>> {
let snapshot = self.drive_membership_snapshot().await;
let mut disks = snapshot
@@ -432,6 +439,10 @@ impl SetDisks {
Ok((disk, fm))
}
#[allow(
dead_code,
reason = "MinIO-parity healing-disk accessor with no caller in this port (backlog#1823)"
)]
pub(in crate::set_disk) async fn get_online_disk_with_healing(
&self,
incl_healing: bool,
@@ -440,6 +451,10 @@ impl SetDisks {
Ok((new_disks, healing > 0))
}
#[allow(
dead_code,
reason = "reached only from get_online_disk_with_healing, itself uncalled in this port (backlog#1823)"
)]
pub(in crate::set_disk) async fn get_online_disk_with_healing_and_info(
&self,
incl_healing: bool,
@@ -415,6 +415,7 @@ fn reduce_quorum_part_numbers(object_parts: Vec<Vec<String>>, read_quorum: usize
/// never returned, but flips `is_truncated` to `true` and yields a
/// `next_upload_id_marker` pointing at the last returned upload so the caller can
/// resume paging.
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn paginate_upload_page(remaining: &[MultipartInfo], max_uploads: usize) -> (Vec<MultipartInfo>, bool, Option<String>) {
let is_truncated = remaining.len() > max_uploads;
let page: Vec<MultipartInfo> = remaining.iter().take(max_uploads).cloned().collect();
@@ -557,6 +558,7 @@ impl SetDisks {
}
#[tracing::instrument(level = "debug", skip(self))]
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
pub(super) async fn check_upload_id_exists(
&self,
bucket: &str,
+36
View File
@@ -3507,6 +3507,10 @@ struct TransitionUploadedSaveProbeState {
}
#[cfg(test)]
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
struct TransitionUploadedSaveProbe {
state: Arc<TransitionUploadedSaveProbeState>,
}
@@ -3517,6 +3521,10 @@ static TRANSITION_UPLOADED_SAVE_PROBE: std::sync::OnceLock<std::sync::Mutex<Opti
#[cfg(test)]
impl TransitionUploadedSaveProbe {
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
fn install(bucket: &str, object: &str) -> Self {
let state = Arc::new(TransitionUploadedSaveProbeState {
bucket: bucket.to_string(),
@@ -3533,6 +3541,10 @@ impl TransitionUploadedSaveProbe {
Self { state }
}
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
fn attempts(&self) -> usize {
self.state.attempts.load(std::sync::atomic::Ordering::Acquire)
}
@@ -3738,6 +3750,10 @@ struct TransitionCommitBarrierState {
}
#[cfg(test)]
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
struct TransitionCommitBarrier {
state: Arc<TransitionCommitBarrierState>,
}
@@ -3748,14 +3764,26 @@ static TRANSITION_COMMIT_BARRIER: std::sync::OnceLock<std::sync::Mutex<Option<Ar
#[cfg(test)]
impl TransitionCommitBarrier {
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
fn install_before_lock_lost_check(bucket: &str, object: &str) -> Self {
Self::install_at(bucket, object, TransitionCommitPause::BeforeLockLost)
}
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
fn install(bucket: &str, object: &str) -> Self {
Self::install_at(bucket, object, TransitionCommitPause::BeforeLeaseValidation)
}
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
fn install_after_lease_check(bucket: &str, object: &str) -> Self {
Self::install_at(bucket, object, TransitionCommitPause::AfterLeaseValidation)
}
@@ -3778,12 +3806,20 @@ impl TransitionCommitBarrier {
Self { state }
}
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
async fn wait_until_paused(&self) {
tokio::time::timeout(Duration::from_secs(30), self.state.arrived.notified())
.await
.expect("transition should reach the deterministic commit barrier");
}
#[allow(
dead_code,
reason = "installed by set_disk tests behind `--features test-util` (backlog#1823)"
)]
fn release(&self) {
self.state.release.notify_one();
}
+2
View File
@@ -116,6 +116,7 @@ impl SetDisks {
.then_some(GET_METADATA_CACHE_REASON_DIST_ERASURE)
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
async fn cached_get_object_fileinfo(&self, bucket: &str, object: &str) -> Option<Arc<GetObjectMetadataCacheEntry>> {
match self.lookup_cached_get_object_fileinfo(bucket, object).await {
MetadataCacheLookup::Hit(entry) => Some(entry),
@@ -1826,6 +1827,7 @@ fn get_object_metadata_cache_request_bypass_reason(bucket: &str, opts: &ObjectOp
.then_some(GET_METADATA_CACHE_REASON_META_BUCKET)
}
#[allow(dead_code, reason = "asserted by this file's tests (backlog#1823)")]
fn is_get_object_metadata_cache_request_eligible(bucket: &str, opts: &ObjectOptions, read_data: bool) -> bool {
get_object_metadata_cache_request_bypass_reason(bucket, opts, read_data).is_none()
}