Skip to content

Commit 7b4d9d1

Browse files
vlademVlad Volodkin
andauthored
Add max_background setting to S3FilesystemConfig (#1746)
Add `max_background` setting to `S3FilesystemConfig`. ### Does this change impact existing behavior? No. ### Does this change need a changelog entry? Does it require a version change? Yes, new version of `mountpoint-s3-fs`. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the [Developer Certificate of Origin (DCO)](https://developercertificate.org/). --------- Signed-off-by: Vlad Volodkin <vlaad@amazon.co.uk> Co-authored-by: Vlad Volodkin <vlaad@amazon.co.uk>
1 parent 0e71446 commit 7b4d9d1

File tree

3 files changed

+44
-21
lines changed

3 files changed

+44
-21
lines changed

mountpoint-s3-fs/examples/mount_from_config.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ struct ConfigOptions {
8585

8686
/// Disk cache configuration
8787
disk_cache: Option<DiskCacheConfig>,
88+
89+
/// Limits the number of concurrent FUSE requests that the kernel may send, default: 64
90+
max_background_fuse_requests: Option<u16>,
8891
}
8992

9093
impl ConfigOptions {
@@ -103,6 +106,7 @@ impl ConfigOptions {
103106
fn build_filesystem_config(&self) -> Result<S3FilesystemConfig> {
104107
let mut fs_config = S3FilesystemConfig {
105108
cache_config: CacheConfig::new(mountpoint_s3_fs::fs::TimeToLive::Indefinite),
109+
max_background_fuse_requests: self.max_background_fuse_requests,
106110
..Default::default()
107111
};
108112

mountpoint-s3-fs/src/fs.rs

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -177,27 +177,15 @@ where
177177
self.next_handle.fetch_add(1, Ordering::SeqCst)
178178
}
179179

180-
/// Helper to return the u16 value in an environment variable, or panic. Useful for unstable overrides.
181-
fn parse_env_var_to_u16(var_name: &str, var_value: std::ffi::OsString) -> u16 {
182-
var_value
183-
.to_string_lossy()
184-
.parse::<u16>()
185-
.unwrap_or_else(|_| panic!("Invalid value for environment variable {var_name}. Must be positive integer."))
186-
}
187-
188180
pub async fn init(&self, config: &mut KernelConfig) -> Result<(), libc::c_int> {
189-
const ENV_VAR_KEY_MAX_BACKGROUND: &str = "UNSTABLE_MOUNTPOINT_MAX_BACKGROUND";
190-
const ENV_VAR_KEY_CONGESTION_THRESHOLD: &str = "UNSTABLE_MOUNTPOINT_CONGESTION_THRESHOLD";
191181
let _ = config.add_capabilities(fuser::consts::FUSE_DO_READDIRPLUS);
192-
// Set max_background FUSE parameter to 64 by default, or override with environment variable.
193-
// NOTE: Support for this environment variable may be removed in future without notice.
194-
if let Some(user_max_background) = std::env::var_os(ENV_VAR_KEY_MAX_BACKGROUND) {
195-
let max_background = Self::parse_env_var_to_u16(ENV_VAR_KEY_MAX_BACKGROUND, user_max_background);
182+
// Set max_background FUSE parameter to 64 by default, may be overriden with config setting or by an environment variable.
183+
if let Some(max_background) = self.config.max_background_fuse_requests() {
196184
let old = config
197185
.set_max_background(max_background)
198186
.unwrap_or_else(|_| panic!("Unable to set FUSE max_background configuration to {max_background}"));
199-
tracing::warn!(
200-
"Successfully overridden FUSE max_background configuration to {} (was {}) from unstable environment variable.",
187+
tracing::info!(
188+
"Successfully set FUSE max_background configuration to {} (was {}).",
201189
max_background,
202190
old
203191
);
@@ -213,14 +201,11 @@ where
213201
}
214202

215203
// Override FUSE congestion threshold if environment variable is present.
216-
// NOTE: Support for this environment variable may be removed in future without notice.
217-
if let Some(user_congestion_threshold) = std::env::var_os(ENV_VAR_KEY_CONGESTION_THRESHOLD) {
218-
let congestion_threshold =
219-
Self::parse_env_var_to_u16(ENV_VAR_KEY_CONGESTION_THRESHOLD, user_congestion_threshold);
204+
if let Some(congestion_threshold) = self.config.fuse_congestion_threshold() {
220205
let old = config
221206
.set_congestion_threshold(congestion_threshold)
222207
.unwrap_or_else(|_| panic!("unable to set FUSE congestion_threshold to {congestion_threshold}"));
223-
tracing::warn!(
208+
tracing::info!(
224209
"Successfully overridden FUSE congestion_threshold configuration to {} (was {}) from unstable environment variable.",
225210
congestion_threshold,
226211
old

mountpoint-s3-fs/src/fs/config.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ pub struct S3FilesystemConfig {
4343
pub mem_limit: u64,
4444
/// Prefetcher configuration
4545
pub prefetcher_config: PrefetcherConfig,
46+
/// Limits the number of concurrent FUSE requests that the kernel may send. Default is 64.
47+
/// This option may also be configured by `UNSTABLE_MOUNTPOINT_MAX_BACKGROUND` environment variable,
48+
/// but the value specified in the config takes priority.
49+
pub max_background_fuse_requests: Option<u16>,
4650
}
4751

4852
impl Default for S3FilesystemConfig {
@@ -67,6 +71,7 @@ impl Default for S3FilesystemConfig {
6771
use_upload_checksums: true,
6872
mem_limit: MINIMUM_MEM_LIMIT,
6973
prefetcher_config: Default::default(),
74+
max_background_fuse_requests: None,
7075
}
7176
}
7277
}
@@ -78,6 +83,35 @@ impl S3FilesystemConfig {
7883
incremental_upload: self.incremental_upload,
7984
}
8085
}
86+
87+
pub fn max_background_fuse_requests(&self) -> Option<u16> {
88+
// NOTE: Support for this environment variable may be removed in future without notice.
89+
const ENV_VAR_KEY_MAX_BACKGROUND: &str = "UNSTABLE_MOUNTPOINT_MAX_BACKGROUND";
90+
if self.max_background_fuse_requests.is_some() {
91+
self.max_background_fuse_requests
92+
} else if let Some(user_max_background) = std::env::var_os(ENV_VAR_KEY_MAX_BACKGROUND) {
93+
let max_background = Self::parse_env_var_to_u16(ENV_VAR_KEY_MAX_BACKGROUND, user_max_background);
94+
Some(max_background)
95+
} else {
96+
None
97+
}
98+
}
99+
100+
pub fn fuse_congestion_threshold(&self) -> Option<u16> {
101+
// NOTE: Support for this environment variable may be removed in future without notice.
102+
const ENV_VAR_KEY_CONGESTION_THRESHOLD: &str = "UNSTABLE_MOUNTPOINT_CONGESTION_THRESHOLD";
103+
std::env::var_os(ENV_VAR_KEY_CONGESTION_THRESHOLD).map(|user_congestion_threshold| {
104+
Self::parse_env_var_to_u16(ENV_VAR_KEY_CONGESTION_THRESHOLD, user_congestion_threshold)
105+
})
106+
}
107+
108+
/// Helper to return the u16 value in an environment variable, or panic. Useful for unstable overrides.
109+
fn parse_env_var_to_u16(var_name: &str, var_value: std::ffi::OsString) -> u16 {
110+
var_value
111+
.to_string_lossy()
112+
.parse::<u16>()
113+
.unwrap_or_else(|_| panic!("Invalid value for environment variable {var_name}. Must be positive integer."))
114+
}
81115
}
82116

83117
#[derive(Debug, Clone)]

0 commit comments

Comments
 (0)