mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-13 00:26:53 +00:00
Fix/resolve pr 1710 (#1743)
This commit is contained in:
@@ -375,6 +375,7 @@ static CONCURRENCY_MANAGER: LazyLock<ConcurrencyManager> = LazyLock::new(Concurr
|
||||
pub struct GetObjectGuard {
|
||||
/// Track when the request started for metrics collection.
|
||||
/// Used to calculate end-to-end request latency in the Drop implementation.
|
||||
#[allow(dead_code)]
|
||||
start_time: Instant,
|
||||
/// Reference to the concurrency manager for cleanup operations.
|
||||
/// The underscore prefix indicates this is used implicitly (for type safety).
|
||||
@@ -399,6 +400,7 @@ impl GetObjectGuard {
|
||||
///
|
||||
/// Useful for logging or metrics collection during request processing.
|
||||
/// Called automatically in the Drop implementation for duration tracking.
|
||||
#[allow(dead_code)]
|
||||
pub fn elapsed(&self) -> Duration {
|
||||
self.start_time.elapsed()
|
||||
}
|
||||
@@ -441,8 +443,11 @@ impl Drop for GetObjectGuard {
|
||||
}
|
||||
|
||||
// Record Prometheus metrics for monitoring and alerting
|
||||
#[cfg(feature = "metrics")]
|
||||
{
|
||||
// We strictly disable metrics recording in unit tests (cfg(test)) to prevent
|
||||
// Thread Local Storage (TLS) destruction order issues causing panics.
|
||||
// In production (not(test)), we use a panic check as a safety guard.
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
if !std::thread::panicking() {
|
||||
use metrics::{counter, histogram};
|
||||
// Track total completed requests for throughput calculation
|
||||
counter!("rustfs.get.object.requests.completed").increment(1);
|
||||
@@ -475,7 +480,7 @@ pub fn get_concurrency_aware_buffer_size(file_size: i64, base_buffer_size: usize
|
||||
let concurrent_requests = ACTIVE_GET_REQUESTS.load(Ordering::Relaxed);
|
||||
|
||||
// Record concurrent request metrics
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::gauge;
|
||||
gauge!("rustfs.concurrent.get.requests").set(concurrent_requests as f64);
|
||||
@@ -885,7 +890,7 @@ impl HotObjectCache {
|
||||
cached.access_count.fetch_add(1, Ordering::Relaxed);
|
||||
self.hit_count.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::counter;
|
||||
counter!("rustfs.object.cache.hits").increment(1);
|
||||
@@ -897,7 +902,7 @@ impl HotObjectCache {
|
||||
None => {
|
||||
self.miss_count.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::counter;
|
||||
counter!("rustfs.object.cache.misses").increment(1);
|
||||
@@ -929,7 +934,7 @@ impl HotObjectCache {
|
||||
|
||||
self.cache.insert(key.clone(), cached_obj).await;
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::{counter, gauge};
|
||||
counter!("rustfs.object.cache.insertions").increment(1);
|
||||
@@ -1086,7 +1091,7 @@ impl HotObjectCache {
|
||||
cached.data.increment_access();
|
||||
self.hit_count.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::counter;
|
||||
counter!("rustfs_object_response_cache_hits").increment(1);
|
||||
@@ -1098,7 +1103,7 @@ impl HotObjectCache {
|
||||
None => {
|
||||
self.miss_count.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::counter;
|
||||
counter!("rustfs_object_response_cache_misses").increment(1);
|
||||
@@ -1135,7 +1140,7 @@ impl HotObjectCache {
|
||||
|
||||
self.response_cache.insert(key.clone(), cached_internal).await;
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::{counter, gauge};
|
||||
counter!("rustfs_object_response_cache_insertions").increment(1);
|
||||
@@ -1158,7 +1163,7 @@ impl HotObjectCache {
|
||||
self.cache.invalidate(key).await;
|
||||
self.response_cache.invalidate(key).await;
|
||||
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::counter;
|
||||
counter!("rustfs_object_cache_invalidations").increment(1);
|
||||
@@ -1340,7 +1345,7 @@ impl ConcurrencyManager {
|
||||
}
|
||||
|
||||
// Record histogram metric for Prometheus
|
||||
#[cfg(feature = "metrics")]
|
||||
#[cfg(all(feature = "metrics", not(test)))]
|
||||
{
|
||||
use metrics::histogram;
|
||||
histogram!("rustfs.disk.permit.wait.duration.seconds").record(wait_duration.as_secs_f64());
|
||||
@@ -1675,8 +1680,10 @@ pub(crate) fn reset_active_get_requests() {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use serial_test::serial;
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_concurrent_request_tracking() {
|
||||
reset_active_get_requests();
|
||||
assert_eq!(GetObjectGuard::concurrent_requests(), 0);
|
||||
@@ -1722,6 +1729,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_hot_object_cache() {
|
||||
let cache = HotObjectCache::new();
|
||||
let test_data = vec![1u8; 1024];
|
||||
@@ -1738,6 +1746,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_cache_eviction() {
|
||||
let cache = HotObjectCache::new();
|
||||
|
||||
@@ -1757,6 +1766,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_cache_reject_large_objects() {
|
||||
let cache = HotObjectCache::new();
|
||||
let large_data = vec![0u8; 11 * MI_B]; // Larger than max_object_size
|
||||
@@ -1777,6 +1787,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_disk_read_permits() {
|
||||
let manager = ConcurrencyManager::new();
|
||||
|
||||
@@ -1814,6 +1825,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_hot_keys_tracking() {
|
||||
let manager = ConcurrencyManager::new();
|
||||
|
||||
@@ -1834,6 +1846,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_batch_operations() {
|
||||
let manager = ConcurrencyManager::new();
|
||||
|
||||
@@ -1851,6 +1864,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_cache_clear() {
|
||||
let manager = ConcurrencyManager::new();
|
||||
|
||||
@@ -1868,6 +1882,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_warm_cache() {
|
||||
let manager = ConcurrencyManager::new();
|
||||
|
||||
|
||||
@@ -93,6 +93,7 @@ mod tests {
|
||||
CachedGetObject, ConcurrencyManager, GetObjectGuard, get_advanced_buffer_size, get_concurrency_aware_buffer_size,
|
||||
};
|
||||
use rustfs_config::{KI_B, MI_B};
|
||||
use serial_test::serial;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use tokio::time::{Instant, sleep};
|
||||
@@ -117,6 +118,7 @@ mod tests {
|
||||
/// uses `ACTIVE_GET_REQUESTS` to determine optimal buffer sizes. A leaked
|
||||
/// counter would cause permanently reduced buffer sizes, degrading performance.
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_concurrent_request_tracking() {
|
||||
// Start with current baseline (may not be zero if other tests are running)
|
||||
let initial = GetObjectGuard::concurrent_requests();
|
||||
@@ -191,6 +193,7 @@ mod tests {
|
||||
/// ACTIVE_GET_REQUESTS is a global atomic counter. The test uses widened
|
||||
/// tolerances to account for this.
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_adaptive_buffer_sizing() {
|
||||
let file_size = 32 * MI_B as i64; // 32MB file (matches issue #911 test case)
|
||||
let base_buffer = 256 * KI_B; // 256KB base buffer (typical for S3-like workloads)
|
||||
@@ -527,6 +530,7 @@ mod tests {
|
||||
|
||||
/// Test advanced buffer sizing with file patterns
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_advanced_buffer_sizing() {
|
||||
crate::storage::concurrency::reset_active_get_requests();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user