Revert "io/writeback/linux: drop is_nfs skip — bounded cache works on every medium"
This reverts commit dffee56102.
This commit is contained in:
+18
-25
@@ -22,24 +22,21 @@
|
|||||||
//! fast storage (NVMe) sees smaller chunks to keep cache pressure
|
//! fast storage (NVMe) sees smaller chunks to keep cache pressure
|
||||||
//! tight. Bounds: [4 MiB, 256 MiB].
|
//! tight. Bounds: [4 MiB, 256 MiB].
|
||||||
//!
|
//!
|
||||||
//! ## NFS — same path, same safety net
|
//! ## NFS escape hatch
|
||||||
//!
|
//!
|
||||||
//! Earlier revisions of this code unconditionally skipped WAIT_AFTER +
|
//! `sync_file_range(WAIT_AFTER)` on an NFS-mounted file can block
|
||||||
//! `posix_fadvise(DONTNEED)` on NFS-mounted files, on the premise that
|
//! indefinitely waiting for the server's commit ack. If the server
|
||||||
//! "NFS clients have their own buffering and commit semantics that
|
//! never acks (network partition, server-side hang, slow commit), the
|
||||||
//! handle dirty-page bounds without us forcing the issue." That premise
|
//! syscall never returns and the consumer thread is stuck inside the
|
||||||
//! was empirically wrong: on Linux NFS clients dirty pages still
|
//! kernel — `/api/stop` can't reach it because halt is cooperative.
|
||||||
//! accumulate up to `vm.dirty_ratio` (default 20% of RAM, ~6.6 GB on a
|
|
||||||
//! 32 GB box) before the kernel forces writeback and throttles app
|
|
||||||
//! writes. Mux throughput on NFS therefore cycled between ~50 MB/s
|
|
||||||
//! (cache absorbing) and ~10 MB/s (cache draining under throttle) on
|
|
||||||
//! a ~100 s period — exactly the pathology this pipeline was built to
|
|
||||||
//! fix, but disabled on the medium where it actually mattered.
|
|
||||||
//!
|
//!
|
||||||
//! `is_nfs` is still detected (for logging and observability) but
|
//! When `fstatfs` reports the file lives on an NFS mount
|
||||||
//! `skip_wait` no longer keys off it. WAIT_AFTER + DONTNEED run on NFS
|
//! (`f_type == NFS_SUPER_MAGIC`), the pipeline skips the WAIT_AFTER +
|
||||||
//! exactly as on local storage. The safety net described below
|
//! `posix_fadvise(DONTNEED)` dance entirely. NFS clients have their
|
||||||
//! (`WAIT_AFTER_TIMEOUT`) catches the original NFS-hang concern.
|
//! own buffering and commit semantics that handle dirty-page bounds
|
||||||
|
//! without us forcing the issue. The async `SYNC_FILE_RANGE_WRITE`
|
||||||
|
//! kickoff still runs (non-blocking by spec) so writeback still gets
|
||||||
|
//! a nudge.
|
||||||
//!
|
//!
|
||||||
//! ## Defence in depth: WAIT_AFTER timeout
|
//! ## Defence in depth: WAIT_AFTER timeout
|
||||||
//!
|
//!
|
||||||
@@ -116,8 +113,8 @@ impl WritebackPipeline {
|
|||||||
let is_nfs = detect_nfs(fd);
|
let is_nfs = detect_nfs(fd);
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
target: "mux",
|
target: "mux",
|
||||||
"WritebackPipeline fd={fd} is_nfs={is_nfs} chunk_bytes={chunk_bytes} strategy=wait+dontneed (falls back to skip if WAIT_AFTER timeouts past {}s)",
|
"WritebackPipeline fd={fd} is_nfs={is_nfs} chunk_bytes={chunk_bytes} strategy={}",
|
||||||
WAIT_AFTER_TIMEOUT.as_secs(),
|
if is_nfs { "nfs-skip-wait" } else { "wait+dontneed" }
|
||||||
);
|
);
|
||||||
Self {
|
Self {
|
||||||
fd,
|
fd,
|
||||||
@@ -132,15 +129,11 @@ impl WritebackPipeline {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// True if we should bypass the WAIT_AFTER + DONTNEED finalisation
|
/// True if we should bypass the WAIT_AFTER + DONTNEED finalisation
|
||||||
/// step. The pipeline starts in the normal path on every medium
|
/// step. NFS always bypasses; local storage bypasses once the
|
||||||
/// (NFS, local, etc.) and flips here only if a real
|
/// pipeline has flipped to degraded after a WAIT_AFTER timeout.
|
||||||
/// `WAIT_AFTER` call exceeds [`WAIT_AFTER_TIMEOUT`] — at which
|
|
||||||
/// point we conclude this particular FS/server combination cannot
|
|
||||||
/// safely service WAIT_AFTER and fall back to skip mode for the
|
|
||||||
/// rest of the pipeline's life. See module-level comment.
|
|
||||||
#[inline]
|
#[inline]
|
||||||
fn skip_wait(&self) -> bool {
|
fn skip_wait(&self) -> bool {
|
||||||
self.degraded.load(Ordering::Relaxed)
|
self.is_nfs || self.degraded.load(Ordering::Relaxed)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Caller advanced the file position to `pos`. If a chunk boundary
|
/// Caller advanced the file position to `pos`. If a chunk boundary
|
||||||
|
|||||||
Reference in New Issue
Block a user