Skip to content

Commit 5de5ed8

Browse files
committed
fix(vfs): skip CAS round-trip for reads on reused-staging opens
The prior commit installed sparse_write on the can_reuse_staging path to enable range_upload at flush time. That regressed reads: a writable handle that hadn't written anything yet would still call fill_sparse_holes, which downloaded CAS bytes for the whole read region even though the staging file already held the full original content. Add a `staging_holds_full_original` flag to SparseWriteState, set via `new_with_full_staging` on the reused-staging path only. When true, fill_sparse_holes short-circuits — staging is the source of truth for non-dirty bytes, and track_write keeps that invariant on subsequent writes. Fresh sparse opens (where staging is an actual hole) leave the flag false and continue to fill from CAS.
1 parent 9a3099e commit 5de5ed8

2 files changed

Lines changed: 69 additions & 6 deletions

File tree

src/virtual_fs/inode.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,15 @@ pub struct SparseWriteState {
173173
pub effective_original_size: u64,
174174
/// Sorted, non-overlapping dirty byte ranges (start, end), in current-file coordinates.
175175
pub dirty_ranges: Vec<(u64, u64)>,
176+
/// When true, the on-disk staging file holds the full original content in
177+
/// [0, effective_original_size) — set for opens that reuse a current
178+
/// staging cache (no fresh sparse hole was created). Reads can serve
179+
/// directly from staging without consulting CAS; `fill_sparse_holes`
180+
/// short-circuits to a no-op. Stays valid for the lifetime of the open
181+
/// because dirty writes overlay onto the existing bytes (track_write
182+
/// updates both staging and the dirty range list) and the snapshot hash
183+
/// is pinned once dirty is set.
184+
pub staging_holds_full_original: bool,
176185
}
177186

178187
impl SparseWriteState {
@@ -182,6 +191,16 @@ impl SparseWriteState {
182191
original_size,
183192
effective_original_size: original_size,
184193
dirty_ranges: Vec::new(),
194+
staging_holds_full_original: false,
195+
}
196+
}
197+
198+
/// Variant of `new` for reused-staging opens: the staging file already
199+
/// holds the full original content, so reads bypass CAS.
200+
pub fn new_with_full_staging(original_hash: String, original_size: u64) -> Self {
201+
Self {
202+
staging_holds_full_original: true,
203+
..Self::new(original_hash, original_size)
185204
}
186205
}
187206

@@ -2697,6 +2716,32 @@ mod tests {
26972716
assert_eq!(table.lookup_child(ROOT_INODE, "a.txt").map(|e| e.inode), Some(a2));
26982717
}
26992718

2719+
// ── SparseWriteState constructors ───────────────────────────────
2720+
2721+
#[test]
2722+
fn sparse_new_defaults_to_empty_holes() {
2723+
let sw = SparseWriteState::new("h".into(), 100);
2724+
assert!(
2725+
!sw.staging_holds_full_original,
2726+
"fresh sparse open: staging is a hole, must consult CAS on reads"
2727+
);
2728+
assert_eq!(sw.original_size, 100);
2729+
assert_eq!(sw.effective_original_size, 100);
2730+
assert!(sw.dirty_ranges.is_empty());
2731+
}
2732+
2733+
#[test]
2734+
fn sparse_new_with_full_staging_marks_flag() {
2735+
let sw = SparseWriteState::new_with_full_staging("h".into(), 100);
2736+
assert!(
2737+
sw.staging_holds_full_original,
2738+
"reused-staging open: fill_sparse_holes must short-circuit"
2739+
);
2740+
assert_eq!(sw.original_size, 100);
2741+
assert_eq!(sw.effective_original_size, 100);
2742+
assert!(sw.dirty_ranges.is_empty());
2743+
}
2744+
27002745
// ── SparseWriteState::track_write ───────────────────────────────
27012746

27022747
// 0 10 20 30

src/virtual_fs/mod.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,14 +1787,23 @@ impl VirtualFs {
17871787
} else if will_install_sparse && entry.sparse_write.is_none() {
17881788
// Install sparse tracking against the snapshot hash. Covers two
17891789
// cases under the same drift guard:
1790-
// * created_sparse_staging: staging is a hole sized to snapshot;
1791-
// reads fill from CAS via `fill_sparse_holes`.
1792-
// * reused staging: on-disk file matches the snapshot hash
1793-
// fully (staging_is_current path); reads serve from staging
1794-
// and fill_sparse_holes is a no-op for non-dirty regions.
1790+
// * fresh sparse staging (can_reuse_staging=false): staging is
1791+
// a hole sized to snapshot; reads must fill from CAS via
1792+
// `fill_sparse_holes`.
1793+
// * reused staging (can_reuse_staging=true with staging_is_current
1794+
// snapshotted true): on-disk file already holds the full
1795+
// original content for the snapshot hash. Mark it via
1796+
// `new_with_full_staging` so `fill_sparse_holes` is a no-op
1797+
// and reads serve directly from staging without an extra
1798+
// CAS round-trip.
17951799
// Once set_dirty has fired, poll skips this inode so the snapshot
17961800
// remains valid for the lifetime of the open.
1797-
entry.sparse_write = Some(Arc::new(inode::SparseWriteState::new(xet_hash.to_string(), size)));
1801+
let sw = if can_reuse_staging {
1802+
inode::SparseWriteState::new_with_full_staging(xet_hash.to_string(), size)
1803+
} else {
1804+
inode::SparseWriteState::new(xet_hash.to_string(), size)
1805+
};
1806+
entry.sparse_write = Some(Arc::new(sw));
17981807
}
17991808
}
18001809

@@ -2132,6 +2141,15 @@ impl VirtualFs {
21322141
return Ok(());
21332142
}
21342143

2144+
// Reused-staging open: the on-disk file holds the full original content
2145+
// for the snapshot hash, so reads outside dirty ranges are already served
2146+
// correctly by the prior pread. Skip CAS to avoid an unnecessary round-
2147+
// trip — track_write keeps overlaying user bytes onto staging, so the
2148+
// invariant still holds for dirty regions too.
2149+
if sparse_write_state.staging_holds_full_original {
2150+
return Ok(());
2151+
}
2152+
21352153
// Skip the CAS download if the read region is fully covered by dirty ranges
21362154
// (the staging file already has the right data, no sparse holes to fill).
21372155
let ranges = &sparse_write_state.dirty_ranges;

0 commit comments

Comments
 (0)