Skip to content

Commit

Permalink
Set max_background FUSE config to 64 by default. (#1137)
Browse files Browse the repository at this point in the history
This improves sequential read performance on instances with multiple
100Gbps network interfaces. It controls the number of requests that are
allowed in the pending queue that are classified as background, which
includes at least some read requests. It also indirectly controls the
"congestion threshold", which is set by default to 75% of the max
background value. When the congestion threshold is reached, FUSE will
stop sending the asynchronous part of readaheads from paged IO to the
filesystem.

Testing on 2 NIC instances shows up to approximately 29% speed-up on a
sequential read workload with 32 open files, from 76.74 to 99Gbps, for
paged IO. Although we don't have enough instrumentation to fully
understand the change in queueing behaviour in FUSE, we think it is
likely because we're able to serve sufficient readahead requests for the
object before hitting the congestion threshold when the limit is higher,
thus allowing mountpoint to start prefetching later parts of the object
sooner.

The value of 64 was picked by experimentation with values between 16
(the default) and 256, as well as specifically setting the congestion
threshold. Increasing the value generally led to better performance up
to 64, after which performance doesn't improve further (at least not
significantly). We wanted to choose the lowest value that seemed
reasonable for the desired performance improvement, to reduce the chance
of affecting a workload that wasn't being tested.

As well as the standard regression tests, the change was tested on trn1
instances with a 256KB sequential read workload reading 32 files in
parallel over 1, 2, and 4 network interfaces. It does not regress our
standard benchmarks nor performance on this test with 1 NIC in use.

This change also temporarily introduces two environment variables to
tune the behaviour, so we can isolate this change if a particular
workload is found to regress.

## Does this change impact existing behavior?

This improves performance on large instance types. There's a risk of
regression for workloads we don't test.

## Does this change need a changelog entry in any of the crates?

Yes, will submit a separate PR.

---

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: Andrew Peace <[email protected]>
Signed-off-by: Andy Peace <[email protected]>
Co-authored-by: Daniel Carl Jones <[email protected]>
  • Loading branch information
adpeace and dannycjones authored Nov 18, 2024
1 parent 3738860 commit 7198bc8
Showing 1 changed file with 50 additions and 0 deletions.
50 changes: 50 additions & 0 deletions mountpoint-s3/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,58 @@ where
self.next_handle.fetch_add(1, Ordering::SeqCst)
}

/// Helper to return the u16 value in an environment variable, or panic. Useful for unstable overrides.
fn parse_env_var_to_u16(var_name: &str, var_value: std::ffi::OsString) -> u16 {
var_value.to_string_lossy().parse::<u16>().unwrap_or_else(|_| {
panic!(
"Invalid value for environment variable {}. Must be positive integer.",
var_name
)
})
}

pub async fn init(&self, config: &mut KernelConfig) -> Result<(), libc::c_int> {
const ENV_VAR_KEY_MAX_BACKGROUND: &str = "UNSTABLE_MOUNTPOINT_MAX_BACKGROUND";
const ENV_VAR_KEY_CONGESTION_THRESHOLD: &str = "UNSTABLE_MOUNTPOINT_CONGESTION_THRESHOLD";
let _ = config.add_capabilities(fuser::consts::FUSE_DO_READDIRPLUS);
// Set max_background FUSE parameter to 64 by default, or override with environment variable.
// NOTE: Support for this environment variable may be removed in future without notice.
if let Some(user_max_background) = std::env::var_os(ENV_VAR_KEY_MAX_BACKGROUND) {
let max_background = Self::parse_env_var_to_u16(ENV_VAR_KEY_MAX_BACKGROUND, user_max_background);
let old = config
.set_max_background(max_background)
.unwrap_or_else(|_| panic!("Unable to set FUSE max_background configuration to {}", max_background));
tracing::warn!(
"Successfully overridden FUSE max_background configuration to {} (was {}) from unstable environment variable.",
max_background,
old
);
} else {
const DEFAULT_MAX_BACKGROUND: u16 = 64;
let max_background_result = config.set_max_background(DEFAULT_MAX_BACKGROUND);
if max_background_result.is_err() {
tracing::warn!(
"failed to set FUSE max_background to {}, using Kernel default",
DEFAULT_MAX_BACKGROUND
);
}
}

// Override FUSE congestion threshold if environment variable is present.
// NOTE: Support for this environment variable may be removed in future without notice.
if let Some(user_congestion_threshold) = std::env::var_os(ENV_VAR_KEY_CONGESTION_THRESHOLD) {
let congestion_threshold =
Self::parse_env_var_to_u16(ENV_VAR_KEY_CONGESTION_THRESHOLD, user_congestion_threshold);
let old = config
.set_congestion_threshold(congestion_threshold)
.unwrap_or_else(|_| panic!("unable to set FUSE congestion_threshold to {}", congestion_threshold));
tracing::warn!(
"Successfully overridden FUSE congestion_threshold configuration to {} (was {}) from unstable environment variable.",
congestion_threshold,
old
);
}

if self.config.allow_overwrite {
// Overwrites require FUSE_ATOMIC_O_TRUNC capability on the host, so we will panic if the
// host doesn't support it.
Expand Down

0 comments on commit 7198bc8

Please sign in to comment.