From fc3ce21d8533438fa27af9d932e603c726dc57e0 Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 5 Nov 2026 13:56:57 +0100 Subject: [PATCH] feat: add envvar RC_MULTIPART_THRESHOLD --- README.md | 8 ++++ crates/cli/src/commands/cp.rs | 31 ++++++------- crates/cli/tests/multipart_threshold.rs | 48 +++++++++++++++++++ crates/s3/src/client.rs | 62 +++++++++++++++++++++---- crates/s3/src/multipart.rs | 11 +++++ 5 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 crates/cli/tests/multipart_threshold.rs diff --git a/README.md b/README.md index 7e0fd60..795647c 100644 --- a/README.md +++ b/README.md @@ -555,6 +555,14 @@ rc completions powershell >> $PROFILE ## Configuration +Set `RC_MULTIPART_THRESHOLD` to the upload threshold in bytes, for example +`RC_MULTIPART_THRESHOLD=134217728 rc cp ./archive.tar local/backups/` for 128 MiB. +Files larger than the threshold use multipart uploads; files at the threshold +use a single PUT. The default is 64 MiB (67,108,864 bytes). If the setting is +unset, invalid, zero, or above 5 GiB (5,368,709,120 bytes), it falls back to the +default. This setting does not change multipart chunk sizes or the server-side +copy threshold. + Backends with a lower multipart ceiling can use `rc --max-parts 1000 cp ...` or `RC_MAX_PARTS=1000 rc mirror ...`. The default is 10,000; accepted values are 1 through 10,000. Uploads and server-side multipart copies increase their diff --git a/crates/cli/src/commands/cp.rs b/crates/cli/src/commands/cp.rs index 4fd2313..a7c2884 100644 --- a/crates/cli/src/commands/cp.rs +++ b/crates/cli/src/commands/cp.rs @@ -15,6 +15,7 @@ use rc_core::{ normalize_relative_key, parse_path, relative_local_path_from_key, }; use rc_s3::S3Client; +use rc_s3::multipart::multipart_threshold; use serde::Serialize; use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt; @@ -1521,9 +1522,9 @@ fn validate_storage_class_plan( )) })?; let multipart = match &item.payload { - CpOperation::LocalToRemote { .. } => size > MULTIPART_THRESHOLD, + CpOperation::LocalToRemote { .. } => size > multipart_threshold(), CpOperation::RemoteToRemote { source, target, .. } if source.alias != target.alias => { - size > MULTIPART_THRESHOLD + size > multipart_threshold() } CpOperation::RemoteToRemote { .. } => rc_core::requires_multipart_copy(size), CpOperation::RemoteToLocal { .. } => false, @@ -1780,7 +1781,7 @@ async fn perform_cross_alias_remote_copy( .size_bytes .and_then(|size| u64::try_from(size).ok()) .ok_or_else(|| Error::InvalidPath(format!("Source size is unavailable: {source}")))?; - if args.storage_class.is_some() && planned_size > MULTIPART_THRESHOLD { + if args.storage_class.is_some() && planned_size > multipart_threshold() { return Err(Error::UnsupportedFeature( "RustFS beta.10 does not persist storage class for multipart uploads".to_string(), )); @@ -2933,8 +2934,6 @@ async fn copy_local_to_s3_prepared( } } -/// Multipart upload threshold: files larger than this size use multipart upload. -const MULTIPART_THRESHOLD: u64 = rc_s3::multipart::DEFAULT_PART_SIZE; /// Download progress threshold: avoid flicker for tiny downloads while surfacing meaningful waits. const DOWNLOAD_PROGRESS_THRESHOLD: u64 = 4 * 1024 * 1024; @@ -3004,7 +3003,7 @@ async fn upload_file( ); } }; - if args.storage_class.is_some() && file_size > MULTIPART_THRESHOLD { + if args.storage_class.is_some() && file_size > multipart_threshold() { return formatter.fail( ExitCode::UnsupportedFeature, "RustFS beta.10 does not persist storage class for multipart uploads", @@ -3031,10 +3030,10 @@ async fn upload_file( ); // Show progress bar for large files - let progress = if file_size > MULTIPART_THRESHOLD { + let progress = if file_size > multipart_threshold() { tracing::debug!( file_size, - threshold = MULTIPART_THRESHOLD, + threshold = multipart_threshold(), "Using multipart upload for large file" ); Some(ProgressBar::new(formatter.output_config(), file_size)) @@ -3087,7 +3086,7 @@ fn select_upload_content_type<'a>( guessed_type: Option<&'a str>, file_size: u64, ) -> Option<&'a str> { - if file_size > MULTIPART_THRESHOLD { + if file_size > multipart_threshold() { explicit_type } else { explicit_type.or(guessed_type) @@ -3583,7 +3582,7 @@ async fn copy_s3_to_s3_prepared( if src.alias == dst.alias { rc_core::requires_multipart_copy(size) } else { - size > MULTIPART_THRESHOLD + size > multipart_threshold() } }) { @@ -4031,7 +4030,7 @@ mod tests { #[test] fn test_select_upload_content_type_uses_guess_for_small_files() { let selected = - select_upload_content_type(None, Some("text/plain"), MULTIPART_THRESHOLD - 1); + select_upload_content_type(None, Some("text/plain"), multipart_threshold() - 1); assert_eq!(selected, Some("text/plain")); } @@ -4039,14 +4038,14 @@ mod tests { #[test] fn test_select_upload_content_type_skips_guess_for_multipart_files() { let selected = - select_upload_content_type(None, Some("text/plain"), MULTIPART_THRESHOLD + 1); + select_upload_content_type(None, Some("text/plain"), multipart_threshold() + 1); assert_eq!(selected, None); } #[test] fn test_select_upload_content_type_uses_guess_at_multipart_boundary() { - let selected = select_upload_content_type(None, Some("text/plain"), MULTIPART_THRESHOLD); + let selected = select_upload_content_type(None, Some("text/plain"), multipart_threshold()); assert_eq!(selected, Some("text/plain")); } @@ -4056,7 +4055,7 @@ mod tests { let selected = select_upload_content_type( Some("application/octet-stream"), Some("text/plain"), - MULTIPART_THRESHOLD + 1, + multipart_threshold() + 1, ); assert_eq!(selected, Some("application/octet-stream")); @@ -4737,7 +4736,7 @@ mod tests { target: RemotePath::new("beta", "target", "medium.bin"), source_info: Box::new(ObjectInfo::file( "medium.bin", - (MULTIPART_THRESHOLD + 1) as i64, + (multipart_threshold() + 1) as i64, )), encryption: None, }, @@ -4745,7 +4744,7 @@ mod tests { target: "beta/target/medium.bin".to_string(), relative_path: "medium.bin".to_string(), modified: None, - size_bytes: Some(MULTIPART_THRESHOLD + 1), + size_bytes: Some(multipart_threshold() + 1), }], &TransferSelection::default(), ); diff --git a/crates/cli/tests/multipart_threshold.rs b/crates/cli/tests/multipart_threshold.rs new file mode 100644 index 0000000..bb8409d --- /dev/null +++ b/crates/cli/tests/multipart_threshold.rs @@ -0,0 +1,48 @@ +use std::process::Command; + +#[test] +fn storage_class_preflight_uses_multipart_threshold_from_environment() { + let config_dir = tempfile::tempdir().expect("create config directory"); + let source = tempfile::NamedTempFile::new().expect("create source file"); + + for (size, threshold, expected_code) in [ + (8, Some("7"), 7), + (8, Some("8"), 0), + (8, Some("9"), 0), + (8, None, 0), + (8, Some("invalid"), 0), + (64 * 1024 * 1024 + 1, None, 7), + (64 * 1024 * 1024 + 1, Some("134217728"), 0), + ] { + source.as_file().set_len(size).expect("set source size"); + for planned in [false, true] { + let mut command = Command::new(env!("CARGO_BIN_EXE_rc")); + command + .arg("cp") + .arg(source.path()) + .args([ + "threshold-test/bucket/file.bin", + "--storage-class", + "STANDARD", + "--dry-run", + ]) + .env("RC_CONFIG_DIR", config_dir.path()) + .env("RC_HOST_threshold-test", "http://key:secret@127.0.0.1:1") + .env_remove("RC_MULTIPART_THRESHOLD"); + if planned { + command.arg("--summary"); + } + if let Some(threshold) = threshold { + command.env("RC_MULTIPART_THRESHOLD", threshold); + } + let output = command.output().expect("run copy preflight"); + assert_eq!( + output.status.code(), + Some(expected_code), + "size={size}, threshold={threshold:?}, planned={planned}: {}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + } +} diff --git a/crates/s3/src/client.rs b/crates/s3/src/client.rs index 9ce099f..78b24f3 100644 --- a/crates/s3/src/client.rs +++ b/crates/s3/src/client.rs @@ -76,9 +76,6 @@ use crate::lifecycle_xml::{ validate_lifecycle_configuration_xml_response, }; -/// Keep single-part uploads small to avoid backend incompatibilities with -/// streaming aws-chunked payloads. -const SINGLE_PUT_OBJECT_MAX_SIZE: u64 = crate::multipart::DEFAULT_PART_SIZE; const S3_SERVICE_NAME: &str = "s3"; const S3_REPLICATION_XML_NAMESPACE: &str = "http://s3.amazonaws.com/doc/2006-03-01/"; const RUSTFS_FORCE_DELETE_HEADER: &str = "x-rustfs-force-delete"; @@ -3189,7 +3186,7 @@ impl S3Client { } fn should_use_multipart(file_size: u64) -> bool { - file_size > SINGLE_PUT_OBJECT_MAX_SIZE + file_size > crate::multipart::multipart_threshold() } fn sha256_hash(body: &[u8]) -> String { @@ -9872,21 +9869,70 @@ mod tests { } } + #[test] + fn multipart_threshold_uses_environment() { + const EXPECTED_THRESHOLD: &str = "RC_TEST_EXPECTED_MULTIPART_THRESHOLD"; + if let Ok(expected) = std::env::var(EXPECTED_THRESHOLD) { + let threshold = expected.parse::().expect("valid test threshold"); + assert!(!S3Client::should_use_multipart(threshold - 1)); + assert!(!S3Client::should_use_multipart(threshold)); + assert!(S3Client::should_use_multipart(threshold + 1)); + return; + } + + let default = crate::multipart::DEFAULT_PART_SIZE; + // Child environments avoid changing process-global state in parallel tests. + for (value, expected) in [ + (None, default), + (Some("1"), 1), + (Some("8388608"), 8 * 1024 * 1024), + (Some("134217728"), 128 * 1024 * 1024), + (Some("5368709120"), crate::multipart::MAX_PART_SIZE), + (Some(""), default), + (Some("invalid"), default), + (Some("0"), default), + (Some("-1"), default), + (Some("5368709121"), default), + (Some("18446744073709551616"), default), + ] { + let mut command = + std::process::Command::new(std::env::current_exe().expect("test executable path")); + command + .args([ + "--exact", + "client::tests::multipart_threshold_uses_environment", + ]) + .env(EXPECTED_THRESHOLD, expected.to_string()) + .env_remove("RC_MULTIPART_THRESHOLD"); + if let Some(value) = value { + command.env("RC_MULTIPART_THRESHOLD", value); + } + let output = command.output().expect("run threshold test in child"); + assert!( + output.status.success(), + "threshold {value:?}: {}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + } + #[test] fn should_use_multipart_for_large_files() { assert!(S3Client::should_use_multipart( - SINGLE_PUT_OBJECT_MAX_SIZE + 1 + crate::multipart::multipart_threshold() + 1 )); } #[test] fn should_use_single_part_for_small_files() { assert!(!S3Client::should_use_multipart(0)); - assert!(!S3Client::should_use_multipart(1024 * 1024)); assert!(!S3Client::should_use_multipart( - crate::multipart::DEFAULT_PART_SIZE + crate::multipart::multipart_threshold() - 1 + )); + assert!(!S3Client::should_use_multipart( + crate::multipart::multipart_threshold() )); - assert!(!S3Client::should_use_multipart(SINGLE_PUT_OBJECT_MAX_SIZE)); } #[tokio::test] diff --git a/crates/s3/src/multipart.rs b/crates/s3/src/multipart.rs index dd1aff0..3c41e60 100644 --- a/crates/s3/src/multipart.rs +++ b/crates/s3/src/multipart.rs @@ -18,6 +18,17 @@ pub const MAX_PART_SIZE: u64 = 5 * 1024 * 1024 * 1024; /// Maximum number of parts: 10,000 (S3 limit) pub const MAX_PARTS: usize = 10_000; +/// Upload threshold in bytes from `RC_MULTIPART_THRESHOLD`, defaulting to 64 MiB. +/// Missing or invalid values (outside 1 byte through 5 GiB) use the default. +pub fn multipart_threshold() -> u64 { + std::env::var("RC_MULTIPART_THRESHOLD") + .ok() + .and_then(|value| value.parse::().ok()) + // A larger threshold could select a single PUT beyond the S3 size limit. + .filter(|value| (1..=MAX_PART_SIZE).contains(value)) + .unwrap_or(DEFAULT_PART_SIZE) +} + /// Multipart upload configuration #[derive(Debug, Clone)] pub struct MultipartConfig {