perf(quota): Skip expensive usage checks when no quota configured (#1749)

Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
thorntonmc
2026-02-08 09:37:53 -05:00
committed by GitHub
parent a574285ab2
commit 927f3a57d7
5 changed files with 52 additions and 17 deletions
+19 -3
View File
@@ -36,6 +36,18 @@ impl QuotaChecker {
bucket: &str, bucket: &str,
operation: QuotaOperation, operation: QuotaOperation,
operation_size: u64, operation_size: u64,
) -> Result<QuotaCheckResult, QuotaError> {
self.check_quota_with_usage_reporting(bucket, operation, operation_size, false)
.await
}
/// Check quota with option to force usage calculation even when no quota is configured
pub async fn check_quota_with_usage_reporting(
&self,
bucket: &str,
operation: QuotaOperation,
operation_size: u64,
force_usage_calculation: bool,
) -> Result<QuotaCheckResult, QuotaError> { ) -> Result<QuotaCheckResult, QuotaError> {
let start_time = Instant::now(); let start_time = Instant::now();
let quota_config = self.get_quota_config(bucket).await?; let quota_config = self.get_quota_config(bucket).await?;
@@ -43,7 +55,11 @@ impl QuotaChecker {
// If no quota limit is set, allow operation // If no quota limit is set, allow operation
let quota_limit = match quota_config.quota { let quota_limit = match quota_config.quota {
None => { None => {
let current_usage = self.get_real_time_usage(bucket).await?; let current_usage = if force_usage_calculation {
Some(self.get_real_time_usage(bucket).await?)
} else {
None // Skip expensive usage calculation when no quota and not forced for performance
};
return Ok(QuotaCheckResult { return Ok(QuotaCheckResult {
allowed: true, allowed: true,
current_usage, current_usage,
@@ -84,7 +100,7 @@ impl QuotaChecker {
let result = QuotaCheckResult { let result = QuotaCheckResult {
allowed, allowed,
current_usage, current_usage: Some(current_usage),
quota_limit: Some(quota_limit), quota_limit: Some(quota_limit),
operation_size, operation_size,
remaining, remaining,
@@ -165,7 +181,7 @@ mod tests {
async fn test_quota_check_no_limit() { async fn test_quota_check_no_limit() {
let result = QuotaCheckResult { let result = QuotaCheckResult {
allowed: true, allowed: true,
current_usage: 0, current_usage: None,
quota_limit: None, quota_limit: None,
operation_size: 1024, operation_size: 1024,
remaining: None, remaining: None,
+2 -1
View File
@@ -80,7 +80,8 @@ impl BucketQuota {
#[derive(Debug)] #[derive(Debug)]
pub struct QuotaCheckResult { pub struct QuotaCheckResult {
pub allowed: bool, pub allowed: bool,
pub current_usage: u64, /// current_usage: None when skipped for performance (no quota configured)
pub current_usage: Option<u64>,
/// quota_limit: None means unlimited /// quota_limit: None means unlimited
pub quota_limit: Option<u64>, pub quota_limit: Option<u64>,
pub operation_size: u64, pub operation_size: u64,
+2 -2
View File
@@ -422,7 +422,7 @@ impl Operation for CheckBucketQuotaHandler {
}; };
let result = quota_checker let result = quota_checker
.check_quota(&bucket, operation, request.operation_size) .check_quota_with_usage_reporting(&bucket, operation, request.operation_size, true)
.await .await
.map_err(|e| s3_error!(InternalError, "Failed to check quota: {}", e))?; .map_err(|e| s3_error!(InternalError, "Failed to check quota: {}", e))?;
@@ -431,7 +431,7 @@ impl Operation for CheckBucketQuotaHandler {
operation_type: request.operation_type, operation_type: request.operation_type,
operation_size: request.operation_size, operation_size: request.operation_size,
allowed: result.allowed, allowed: result.allowed,
current_usage: result.current_usage, current_usage: result.current_usage.unwrap_or(0),
quota_limit: result.quota_limit, quota_limit: result.quota_limit,
remaining_quota: result.remaining, remaining_quota: result.remaining,
}; };
+19 -8
View File
@@ -667,6 +667,7 @@ impl S3 for FS {
.map_err(ApiError::from)?; .map_err(ApiError::from)?;
// check quota after completing multipart upload // check quota after completing multipart upload
let mut quota_usage_calculated = false;
if let Some(metadata_sys) = rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get() { if let Some(metadata_sys) = rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get() {
let quota_checker = QuotaChecker::new(metadata_sys.clone()); let quota_checker = QuotaChecker::new(metadata_sys.clone());
@@ -682,15 +683,13 @@ impl S3 for FS {
S3ErrorCode::InvalidRequest, S3ErrorCode::InvalidRequest,
format!( format!(
"Bucket quota exceeded. Current usage: {} bytes, limit: {} bytes", "Bucket quota exceeded. Current usage: {} bytes, limit: {} bytes",
check_result.current_usage, check_result.current_usage.unwrap_or(0),
check_result.quota_limit.unwrap_or(0) check_result.quota_limit.unwrap_or(0)
), ),
)); ));
} }
// Update quota tracking after successful multipart upload // Track if usage was actually calculated (not just returned as None/0)
if rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get().is_some() { quota_usage_calculated = check_result.current_usage.is_some();
rustfs_ecstore::data_usage::increment_bucket_usage_memory(&bucket, obj_info.size as u64).await;
}
} }
Err(e) => { Err(e) => {
warn!("Quota check failed for bucket {}: {}, allowing operation", bucket, e); warn!("Quota check failed for bucket {}: {}, allowing operation", bucket, e);
@@ -698,6 +697,13 @@ impl S3 for FS {
} }
} }
// Only increment usage cache when quota checking actually calculated real usage.
// This prevents cache corruption: when quotas are disabled, the cache remains unset.
// When quotas are later enabled, the cache will miss and recalculate from backend.
if quota_usage_calculated {
rustfs_ecstore::data_usage::increment_bucket_usage_memory(&bucket, obj_info.size as u64).await;
}
// Invalidate cache for the completed multipart object // Invalidate cache for the completed multipart object
let manager = get_concurrency_manager(); let manager = get_concurrency_manager();
let mpu_bucket = bucket.clone(); let mpu_bucket = bucket.clone();
@@ -1036,6 +1042,7 @@ impl S3 for FS {
} }
// check quota for copy operation // check quota for copy operation
let mut quota_usage_calculated = false;
if let Some(metadata_sys) = rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get() { if let Some(metadata_sys) = rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get() {
let quota_checker = QuotaChecker::new(metadata_sys.clone()); let quota_checker = QuotaChecker::new(metadata_sys.clone());
@@ -1049,11 +1056,13 @@ impl S3 for FS {
S3ErrorCode::InvalidRequest, S3ErrorCode::InvalidRequest,
format!( format!(
"Bucket quota exceeded. Current usage: {} bytes, limit: {} bytes", "Bucket quota exceeded. Current usage: {} bytes, limit: {} bytes",
check_result.current_usage, check_result.current_usage.unwrap_or(0),
check_result.quota_limit.unwrap_or(0) check_result.quota_limit.unwrap_or(0)
), ),
)); ));
} }
// Track if usage was actually calculated (not just returned as None/0)
quota_usage_calculated = check_result.current_usage.is_some();
} }
Err(e) => { Err(e) => {
warn!("Quota check failed for bucket {}: {}, allowing operation", bucket, e); warn!("Quota check failed for bucket {}: {}, allowing operation", bucket, e);
@@ -1066,8 +1075,10 @@ impl S3 for FS {
.await .await
.map_err(ApiError::from)?; .map_err(ApiError::from)?;
// Update quota tracking after successful copy // Only increment usage cache when quota checking actually calculated real usage.
if rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get().is_some() { // This prevents cache corruption: when quotas are disabled, the cache remains unset.
// When quotas are later enabled, the cache will miss and recalculate from backend.
if quota_usage_calculated {
rustfs_ecstore::data_usage::increment_bucket_usage_memory(&bucket, oi.size as u64).await; rustfs_ecstore::data_usage::increment_bucket_usage_memory(&bucket, oi.size as u64).await;
} }
+10 -3
View File
@@ -133,6 +133,7 @@ impl Objects {
} }
// check quota for put operation // check quota for put operation
let mut quota_usage_calculated = false;
if let Some(size) = content_length if let Some(size) = content_length
&& let Some(metadata_sys) = rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get() && let Some(metadata_sys) = rustfs_ecstore::bucket::metadata_sys::GLOBAL_BucketMetadataSys.get()
{ {
@@ -148,11 +149,13 @@ impl Objects {
S3ErrorCode::InvalidRequest, S3ErrorCode::InvalidRequest,
format!( format!(
"Bucket quota exceeded. Current usage: {} bytes, limit: {} bytes", "Bucket quota exceeded. Current usage: {} bytes, limit: {} bytes",
check_result.current_usage, check_result.current_usage.unwrap_or(0),
check_result.quota_limit.unwrap_or(0) check_result.quota_limit.unwrap_or(0)
), ),
)); ));
} }
// Track if usage was actually calculated (not just returned as None/0)
quota_usage_calculated = check_result.current_usage.is_some();
} }
Err(e) => { Err(e) => {
warn!("Quota check failed for bucket {}: {}, allowing operation", bucket, e); warn!("Quota check failed for bucket {}: {}, allowing operation", bucket, e);
@@ -332,8 +335,12 @@ impl Objects {
.await .await
.map_err(ApiError::from)?; .map_err(ApiError::from)?;
// Fast in-memory update for immediate quota consistency // Only increment usage cache when quota checking actually calculated real usage.
rustfs_ecstore::data_usage::increment_bucket_usage_memory(&bucket, obj_info.size as u64).await; // This prevents cache corruption: when quotas are disabled, the cache remains unset.
// When quotas are later enabled, the cache will miss and recalculate from backend.
if quota_usage_calculated {
rustfs_ecstore::data_usage::increment_bucket_usage_memory(&bucket, obj_info.size as u64).await;
}
// Invalidate cache for the written object to prevent stale data // Invalidate cache for the written object to prevent stale data
let manager = get_concurrency_manager(); let manager = get_concurrency_manager();