From 270a003c55a57350229eed4bdd2f5b095e00ac50 Mon Sep 17 00:00:00 2001 From: houseme Date: Wed, 12 Aug 2026 19:29:36 +0800 Subject: [PATCH] fix(ecstore): attribute internal metadata GET metrics (#5983) --- .../src/set_disk/core/io_primitives.rs | 6 ++--- crates/ecstore/src/set_disk/ops/object.rs | 27 +++++++++++++++++-- crates/ecstore/src/set_disk/read.rs | 27 +++++++++++-------- crates/io-metrics/src/lib.rs | 8 ++++-- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/crates/ecstore/src/set_disk/core/io_primitives.rs b/crates/ecstore/src/set_disk/core/io_primitives.rs index c41385fae..4f1e4c2f3 100644 --- a/crates/ecstore/src/set_disk/core/io_primitives.rs +++ b/crates/ecstore/src/set_disk/core/io_primitives.rs @@ -221,7 +221,7 @@ impl MetadataFanoutDiagnostics { self.observations.iter().filter(|observation| observation.ignored).count() } - pub(in crate::set_disk) fn error_responses(&self) -> usize { + pub(in crate::set_disk) fn non_valid_responses(&self) -> usize { self.total_responses().saturating_sub(self.valid_responses()) } @@ -272,7 +272,7 @@ impl MetadataFanoutDiagnostics { self.total_responses(), self.valid_responses(), self.ignored_responses(), - self.error_responses(), + self.non_valid_responses(), ); for observation in &self.observations { rustfs_io_metrics::record_get_object_metadata_response(path, observation.outcome); @@ -6037,7 +6037,7 @@ mod tests { assert_eq!(diagnostics.total_responses(), 3); assert_eq!(diagnostics.valid_responses(), 1); assert_eq!(diagnostics.ignored_responses(), 1); - assert_eq!(diagnostics.error_responses(), 2); + assert_eq!(diagnostics.non_valid_responses(), 2); assert_eq!(diagnostics.first_response_latency(), Some(Duration::from_millis(10))); assert_eq!(diagnostics.first_valid_response_latency(), Some(Duration::from_millis(30))); assert_eq!(diagnostics.slowest_response_latency(), Some(Duration::from_millis(30))); diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index a7858041d..8ae8a7ef3 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -5553,7 +5553,7 @@ mod get_object_downstream_close_accounting_tests { let previous_gate = rustfs_io_metrics::get_stage_metrics_enabled(); rustfs_io_metrics::set_get_stage_metrics_enabled(true); - let (decode_failures, emit_failures) = metrics::with_local_recorder(&recorder, || { + let (decode_failures, emit_failures, legacy_fanout, internal_fanout) = metrics::with_local_recorder(&recorder, || { runtime.block_on(async { let (_temp_dirs, _disk_stores, set_disks) = hermetic_set_disks(4).await; let bucket = "get-downstream-close-accounting"; @@ -5621,6 +5621,14 @@ mod get_object_downstream_close_accounting_tests { ("reason", GetObjectFailureReason::DownstreamClosed.as_str()), ], ), + recorder.histogram_values( + "rustfs_io_get_object_metadata_fanout_total_responses", + &[("path", GET_OBJECT_PATH_LEGACY_DUPLEX)], + ), + recorder.histogram_values( + "rustfs_io_get_object_metadata_fanout_total_responses", + &[("path", GET_OBJECT_PATH_INTERNAL_META)], + ), ) }) }); @@ -5628,6 +5636,11 @@ mod get_object_downstream_close_accounting_tests { assert!(decode_failures > 0, "the producer must expose the downstream close at decode"); assert_eq!(emit_failures, 0, "downstream closure must not be counted as an emit failure"); + assert_eq!(legacy_fanout, vec![4.0], "ordinary object fanout must retain the legacy_duplex path"); + assert!( + internal_fanout.is_empty(), + "ordinary object fanout must not be attributed to internal_meta" + ); } #[test] @@ -5641,7 +5654,7 @@ mod get_object_downstream_close_accounting_tests { let previous_gate = rustfs_io_metrics::get_stage_metrics_enabled(); rustfs_io_metrics::set_get_stage_metrics_enabled(true); - let (internal_missing, legacy_unknown) = metrics::with_local_recorder(&recorder, || { + let (internal_missing, legacy_unknown, internal_fanout, legacy_fanout) = metrics::with_local_recorder(&recorder, || { runtime.block_on(async { let (_temp_dirs, _disk_stores, set_disks) = hermetic_set_disks(4).await; let options = ObjectOptions { @@ -5677,6 +5690,14 @@ mod get_object_downstream_close_accounting_tests { ("reason", GetObjectFailureReason::Unknown.as_str()), ], ), + recorder.histogram_values( + "rustfs_io_get_object_metadata_fanout_error_responses", + &[("path", GET_OBJECT_PATH_INTERNAL_META)], + ), + recorder.histogram_values( + "rustfs_io_get_object_metadata_fanout_error_responses", + &[("path", GET_OBJECT_PATH_LEGACY_DUPLEX)], + ), ) }) }); @@ -5687,6 +5708,8 @@ mod get_object_downstream_close_accounting_tests { legacy_unknown, 0, "internal metadata miss must not be attributed to legacy_duplex/unknown" ); + assert_eq!(internal_fanout, vec![4.0], "internal metadata fanout must retain its path label"); + assert!(legacy_fanout.is_empty(), "internal metadata fanout must not leak into legacy_duplex"); } } diff --git a/crates/ecstore/src/set_disk/read.rs b/crates/ecstore/src/set_disk/read.rs index 2f2488d66..9fcc41ba3 100644 --- a/crates/ecstore/src/set_disk/read.rs +++ b/crates/ecstore/src/set_disk/read.rs @@ -30,12 +30,12 @@ use crate::diagnostics::get::{ GET_METADATA_RESPONSE_CORRUPT, GET_METADATA_RESPONSE_DISK_NOT_FOUND, GET_METADATA_RESPONSE_ERROR, GET_METADATA_RESPONSE_IGNORED, GET_METADATA_RESPONSE_NOT_FOUND, GET_METADATA_RESPONSE_TIMEOUT, GET_METADATA_RESPONSE_VALID, GET_METADATA_RESPONSE_VERSION_NOT_FOUND, GET_OBJECT_PATH_CODEC_STREAMING, GET_OBJECT_PATH_DIRECT_MEMORY, - GET_OBJECT_PATH_LEGACY_DUPLEX, GET_OBJECT_PATH_SET_DISK, GET_STAGE_DECODE, GET_STAGE_METADATA_CACHE_LOOKUP, - GET_STAGE_METADATA_RESOLVE, GET_STAGE_RANGE, GET_STAGE_READER_SETUP, GET_STAGE_READER_SETUP_DROP_PENDING, - GET_STAGE_READER_SETUP_SCHEDULE, GET_STAGE_READER_SETUP_WAIT_QUORUM, GET_STAGE_READER_TASK_BITROT_READER_INIT, - GET_STAGE_READER_TASK_FILE_OPEN, GET_STAGE_READER_TASK_READER_CONSTRUCTION, GetObjectFailureReason, classify_disk_error, - get_stage_timer_if_enabled, mark_get_object_downstream_closed, record_get_object_pipeline_failure, - record_get_object_pipeline_failure_for_path, record_get_stage_duration_if_enabled, + GET_OBJECT_PATH_INTERNAL_META, GET_OBJECT_PATH_LEGACY_DUPLEX, GET_OBJECT_PATH_SET_DISK, GET_STAGE_DECODE, + GET_STAGE_METADATA_CACHE_LOOKUP, GET_STAGE_METADATA_RESOLVE, GET_STAGE_RANGE, GET_STAGE_READER_SETUP, + GET_STAGE_READER_SETUP_DROP_PENDING, GET_STAGE_READER_SETUP_SCHEDULE, GET_STAGE_READER_SETUP_WAIT_QUORUM, + GET_STAGE_READER_TASK_BITROT_READER_INIT, GET_STAGE_READER_TASK_FILE_OPEN, GET_STAGE_READER_TASK_READER_CONSTRUCTION, + GetObjectFailureReason, classify_disk_error, get_stage_timer_if_enabled, mark_get_object_downstream_closed, + record_get_object_pipeline_failure, record_get_object_pipeline_failure_for_path, record_get_stage_duration_if_enabled, }; use crate::erasure::coding::BitrotReader; use crate::io_support::bitrot::{ @@ -349,7 +349,12 @@ impl SetDisks { self.default_parity_count, ) .await?; - metadata_fanout_diagnostics.record(GET_OBJECT_PATH_LEGACY_DUPLEX); + let metadata_metrics_path = if crate::bucket::utils::is_meta_bucketname(bucket) { + GET_OBJECT_PATH_INTERNAL_META + } else { + GET_OBJECT_PATH_LEGACY_DUPLEX + }; + metadata_fanout_diagnostics.record(metadata_metrics_path); let metadata_fanout_complete = metadata_fanout_diagnostics.total_responses() >= disks.len(); // warn!("get_object_fileinfo parts_metadata {:?}", &parts_metadata); // warn!("get_object_fileinfo {}/{} errs {:?}", bucket, object, &errs); @@ -387,7 +392,7 @@ impl SetDisks { let (op_online_disks, fi, fileinfo_selection_quorum) = Self::select_valid_fileinfo(&disks, &parts_metadata, &errs, vid.as_str(), read_quorum, write_quorum)?; - metadata_fanout_diagnostics.record_quorum_candidate_latency(GET_OBJECT_PATH_LEGACY_DUPLEX, fileinfo_selection_quorum); + metadata_fanout_diagnostics.record_quorum_candidate_latency(metadata_metrics_path, fileinfo_selection_quorum); if errs.iter().any(|err| err.is_some()) { let version_id = resolved_read_repair_version_id(&fi, opts.version_id.as_deref()); submit_read_repair_heal( @@ -3415,7 +3420,7 @@ mod tests { ); assert_eq!(diagnostics.total_responses(), 9); assert_eq!(diagnostics.valid_responses(), 1); - assert_eq!(diagnostics.error_responses(), 8); + assert_eq!(diagnostics.non_valid_responses(), 8); } #[test] @@ -3430,7 +3435,7 @@ mod tests { ); assert_eq!(diagnostics.ignored_responses(), 2); - assert_eq!(diagnostics.error_responses(), 3); + assert_eq!(diagnostics.non_valid_responses(), 3); assert_eq!(diagnostics.observations[0].outcome, GET_METADATA_RESPONSE_DISK_NOT_FOUND); assert_eq!(diagnostics.observations[1].outcome, GET_METADATA_RESPONSE_IGNORED); assert_eq!(diagnostics.observations[2].outcome, GET_METADATA_RESPONSE_NOT_FOUND); @@ -3498,7 +3503,7 @@ mod tests { assert_eq!(diagnostics.total_responses(), 3); assert_eq!(diagnostics.valid_responses(), 3); - assert_eq!(diagnostics.error_responses(), 0); + assert_eq!(diagnostics.non_valid_responses(), 0); assert!( diagnostics .observations diff --git a/crates/io-metrics/src/lib.rs b/crates/io-metrics/src/lib.rs index 8eb8deec5..e933b46b3 100644 --- a/crates/io-metrics/src/lib.rs +++ b/crates/io-metrics/src/lib.rs @@ -795,8 +795,12 @@ pub fn record_get_object_metadata_cache_decision(path: &'static str, decision: & } /// Record aggregate metadata fanout shape for one GetObject metadata read. +/// +/// The legacy `metadata_fanout_error_responses` series records every non-valid +/// response, including not-found and ignored outcomes. Use +/// `metadata_response_total` outcome labels for failure attribution. #[inline(always)] -pub fn record_get_object_metadata_fanout_shape(path: &'static str, total: usize, valid: usize, ignored: usize, errors: usize) { +pub fn record_get_object_metadata_fanout_shape(path: &'static str, total: usize, valid: usize, ignored: usize, non_valid: usize) { if !get_stage_metrics_enabled() { return; } @@ -807,7 +811,7 @@ pub fn record_get_object_metadata_fanout_shape(path: &'static str, total: usize, histogram!("rustfs_io_get_object_metadata_fanout_ignored_responses", "path" => path) .record(metadata_fanout_count_to_f64(ignored)); histogram!("rustfs_io_get_object_metadata_fanout_error_responses", "path" => path) - .record(metadata_fanout_count_to_f64(errors)); + .record(metadata_fanout_count_to_f64(non_valid)); } /// Record a guarded metadata early-stop hit for GetObject.