diff --git a/.github/workflows/rust-volume-server-tests.yml b/.github/workflows/rust-volume-server-tests.yml index 93ff56a3c..ebbe6ce20 100644 --- a/.github/workflows/rust-volume-server-tests.yml +++ b/.github/workflows/rust-volume-server-tests.yml @@ -27,6 +27,29 @@ permissions: jobs: + changes: + name: Detect changed paths + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + outputs: + rust: ${{ steps.filter.outputs.rust }} + steps: + - name: Checkout code + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Filter changed paths + id: filter + uses: dorny/paths-filter@v3 + with: + filters: | + rust: + - 'seaweed-volume/**' + - '.github/workflows/rust-volume-server-tests.yml' + rust-unit-tests: name: Rust Unit Tests runs-on: ubuntu-22.04 @@ -76,6 +99,45 @@ jobs: - name: Run Rust unit tests (redb experimental cursor) run: cd seaweed-volume && cargo test --features redb-experimental-cursor --lib storage::needle_map + rust-unit-tests-windows: + name: Rust Unit Tests (Windows) + runs-on: windows-latest + timeout-minutes: 30 + needs: [changes] + if: needs.changes.outputs.rust == 'true' + defaults: + run: + shell: bash + + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + + # No glibc on Windows: key the cache on the toolchain and OS only. + - name: Fingerprint build toolchain + id: toolchain + run: echo "fingerprint=windows-rustc-$(rustc -V | awk '{print $2}')" >> "$GITHUB_OUTPUT" + + - name: Cache cargo registry and target + uses: actions/cache@v6 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + seaweed-volume/target + key: rust-windows-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock') }} + restore-keys: | + rust-windows-${{ steps.toolchain.outputs.fingerprint }}- + + - name: Run Rust unit tests + run: cd seaweed-volume && cargo test + + - name: Run Rust unit tests (redb experimental cursor) + run: cd seaweed-volume && cargo test --features redb-experimental-cursor --lib storage::needle_map + rust-integration-tests: name: Rust Integration Tests runs-on: ubuntu-22.04 diff --git a/seaweed-volume/Cargo.lock b/seaweed-volume/Cargo.lock index ce806a527..93ad58ac3 100644 --- a/seaweed-volume/Cargo.lock +++ b/seaweed-volume/Cargo.lock @@ -4604,6 +4604,7 @@ dependencies = [ "tracing", "tracing-subscriber", "uuid", + "windows-sys 0.61.2", "x509-parser", "xxhash-rust", ] diff --git a/seaweed-volume/Cargo.toml b/seaweed-volume/Cargo.toml index 6c823a8c9..19f280963 100644 --- a/seaweed-volume/Cargo.toml +++ b/seaweed-volume/Cargo.toml @@ -143,6 +143,11 @@ aws-types = "1" [target.'cfg(unix)'.dependencies] pprof = { version = "0.15", features = ["prost-codec"] } +# GetDiskFreeSpaceExW for per-path disk capacity on Windows (0.61.2 already +# in the tree via tempfile/mio, so this unifies rather than adding a version). +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem"] } + [dev-dependencies] tempfile = "3" diff --git a/seaweed-volume/src/storage/disk_location.rs b/seaweed-volume/src/storage/disk_location.rs index 5fc699ebf..142453ecd 100644 --- a/seaweed-volume/src/storage/disk_location.rs +++ b/seaweed-volume/src/storage/disk_location.rs @@ -1138,10 +1138,45 @@ pub fn get_disk_stats(path: &str) -> (u64, u64) { } (0, 0) } - #[cfg(not(unix))] + #[cfg(windows)] { - let _ = path; - (0, 0) + use std::os::windows::ffi::OsStrExt; + + // Canonicalize so symlinks, `.`/`..` segments, and relative paths + // resolve to the real location before querying. `\\?\`-prefixed + // extended-length paths and UNC (`\\?\UNC\...`) are passed through + // untouched: GetDiskFreeSpaceExW accepts them as-is. + let canonical = match std::fs::canonicalize(path) { + Ok(p) => p, + Err(_) => return (0, 0), + }; + // UTF-16 with trailing NUL for the Win32 wide-string call. + let mut wide: Vec = canonical.as_os_str().encode_wide().collect(); + // UNC directory names must end in a backslash for GetDiskFreeSpaceExW. + if !wide.ends_with(&[0x5C]) { + wide.push(0x5C); + } + wide.push(0); + // SAFETY: `wide` is NUL-terminated; the out-params are valid u64 + // writes; the call has no other preconditions. + unsafe { + let mut free_available: u64 = 0; + let mut total: u64 = 0; + let ok = windows_sys::Win32::Storage::FileSystem::GetDiskFreeSpaceExW( + wide.as_ptr(), + &mut free_available, + &mut total, + std::ptr::null_mut(), + ); + if ok == 0 { + return (0, 0); + } + return (total, free_available); + } + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("get_disk_stats is implemented for unix and windows only"); } } @@ -1339,6 +1374,17 @@ mod tests { use super::*; use tempfile::TempDir; + /// get_disk_stats must report real capacity for a real path on every + /// platform (Windows included) — consumers treat total==0 as "unknown" + /// and leave available_space at 0, which breaks volume assignment. + #[test] + fn test_get_disk_stats_reports_capacity_for_real_path() { + let tmp = TempDir::new().unwrap(); + let (total, free) = get_disk_stats(tmp.path().to_str().unwrap()); + assert!(total > 0, "expected total>0, got {total}"); + assert!(free > 0, "expected free>0, got {free}"); + } + /// When `-dir.idx` is configured the EC `.vif` may live in the idx /// directory; the sweep must look there too, not only the data dir. #[test] diff --git a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs index aaec531ba..e94837bb0 100644 --- a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs +++ b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs @@ -705,6 +705,17 @@ impl EcVolume { use std::os::unix::fs::FileExt; ecj_file.read_exact_at(&mut buf, off as u64)?; } + #[cfg(windows)] + { + // Positional read so concurrent readers of the shared .ecj + // handle can't interleave seek/read. Mirrors the + // read_exact_at helper at the bottom of this file. + read_exact_at(ecj_file, &mut buf, off as u64)?; + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("Platform not supported: only unix and windows are supported"); + } set.insert(NeedleId::from_bytes(&buf)); off += NEEDLE_ID_SIZE as i64; } @@ -1276,6 +1287,17 @@ impl EcVolume { use std::os::unix::fs::FileExt; ecx_file.read_exact_at(&mut entry_buf, file_offset)?; } + #[cfg(windows)] + { + // Positional read so concurrent readers of the shared .ecx + // handle can't interleave seek/read. Mirrors the + // read_exact_at helper at the bottom of this file. + read_exact_at(ecx_file, &mut entry_buf, file_offset)?; + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("Platform not supported: only unix and windows are supported"); + } let (_key, _offset, size) = idx_entry_from_bytes(&entry_buf); // Match Go's Size.Raw(): tombstone (-1) returns 0, other negatives return abs if !size.is_tombstone() { @@ -1379,6 +1401,17 @@ impl EcVolume { use std::os::unix::fs::FileExt; ecx_file.read_exact_at(&mut entry_buf, file_offset)?; } + #[cfg(windows)] + { + // Positional read so concurrent readers of the shared .ecx + // handle can't interleave seek/read. Mirrors the + // read_exact_at helper at the bottom of this file. + read_exact_at(ecx_file, &mut entry_buf, file_offset)?; + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("Platform not supported: only unix and windows are supported"); + } let (key, _offset, _old_size) = idx_entry_from_bytes(&entry_buf); if key == needle_id { let size_offset = file_offset + NEEDLE_ID_SIZE as u64 + OFFSET_SIZE as u64; @@ -1389,6 +1422,31 @@ impl EcVolume { use std::os::unix::fs::FileExt; ecx_file.write_all_at(&size_buf, size_offset)?; } + #[cfg(windows)] + { + // Positional write so concurrent readers of the shared + // .ecx handle can't observe a moved cursor. Mirrors the + // read_exact_at helper at the bottom of this file, with + // seek_write in place of seek_read. + use std::os::windows::fs::FileExt; + let mut written = 0; + let mut at = size_offset; + while written < size_buf.len() { + let n = ecx_file.seek_write(&size_buf[written..], at)?; + if n == 0 { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "seek_write wrote nothing", + )); + } + written += n; + at += n as u64; + } + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("Platform not supported: only unix and windows are supported"); + } return Ok(true); } else if key < needle_id { lo = mid + 1; @@ -1557,6 +1615,18 @@ impl EcVolume { use std::os::unix::fs::FileExt; ecx_file.read_exact_at(&mut entry_buf, file_offset)?; } + #[cfg(windows)] + { + // Positional read so concurrent find_needle_from_ecx_raw calls + // on the shared .ecx handle don't interleave seek/read and + // corrupt each other's binary search. Mirrors the + // read_exact_at helper at the bottom of this file. + read_exact_at(ecx_file, &mut entry_buf, file_offset)?; + } + #[cfg(not(any(unix, windows)))] + { + compile_error!("Platform not supported: only unix and windows are supported"); + } let (key, offset, size) = idx_entry_from_bytes(&entry_buf); if key == needle_id { return Ok(Some((offset, size))); diff --git a/seaweed-volume/src/storage/needle_map.rs b/seaweed-volume/src/storage/needle_map.rs index e279ccfdd..af25dd59a 100644 --- a/seaweed-volume/src/storage/needle_map.rs +++ b/seaweed-volume/src/storage/needle_map.rs @@ -607,6 +607,14 @@ impl RedbNeedleMap { } } + /// Test-only read of META `idx_size` through the live handle. See + /// [`test_support::live_meta_idx_size`] for why durability tests use + /// this instead of copying the open `.rdb`. + #[cfg(test)] + pub(crate) fn live_meta_idx_size(&self) -> Option { + self.read_idx_size_meta().unwrap() + } + /// Load from an .idx file, reusing an existing .rdb if it is consistent. /// /// Strategy: @@ -1476,22 +1484,30 @@ impl NeedleMap { pub(crate) mod test_support { use super::*; - /// The `.idx` size recorded in the durable state of the `.rdb` at - /// `rdb_path`, read from a copy taken while the map may still be open: - /// exactly what a crash would leave behind. `None` when nothing durable - /// has been recorded yet. - pub(crate) fn durable_idx_size(rdb_path: &Path) -> Option { - let copy = rdb_path.with_extension("crash-copy.rdb"); - std::fs::copy(rdb_path, ©).unwrap(); - let db = Database::open(©).unwrap(); - let txn = db.begin_read().unwrap(); - let meta = txn.open_table(META_TABLE).ok()?; - let size = meta.get(META_IDX_SIZE).unwrap().map(|g| g.value()); - drop(meta); - drop(txn); - drop(db); - let _ = std::fs::remove_file(©); - size + /// The `.idx` size in the map's META table, read through the live + /// handle. + /// + /// The load path records the `.idx` size with `Durability::None`, and + /// every `put`/`delete` also commits non-durably, so before the first + /// checkpoint this is the load-time value (`Some(0)` for a fresh map) — + /// NOT the crash-durable `None` a copy of the open `.rdb` would show. + /// A live read is the only portable observation: redb 4.2.0 takes an + /// exclusive whole-file lock, which is advisory on Unix but mandatory + /// on Windows, so copying the open `.rdb` fails there with OS error 33. + /// + /// It still pins the property under test: the only *durable* META + /// writer is `checkpoint`, so any value other than the load-time one + /// proves a checkpoint recorded progress — and the post-checkpoint + /// value equals the durable one, because checkpoints commit with + /// `Durability::Immediate`. What is lost vs the old copy: strict crash + /// fidelity — a hard crash pre-checkpoint would leave META absent + /// rather than `Some(0)` (loader-equivalent outcomes: full rebuild vs + /// replay-from-0, both correct). A clean close+reopen cannot recover + /// that distinction either: dropping the `Database` flushes pending + /// non-durable commits, so a reopened handle reads `Some(0)` just like + /// the live one. + pub(crate) fn live_meta_idx_size(nm: &RedbNeedleMap) -> Option { + nm.live_meta_idx_size() } } @@ -2169,7 +2185,7 @@ mod tests { #[test] fn test_redb_checkpoint_is_explicit_and_due_every_interval() { - use test_support::durable_idx_size; + use test_support::live_meta_idx_size; // Every non-durable redb commit leaves bookkeeping behind until a // durable one clears it, so a writable map asks for a checkpoint on @@ -2194,24 +2210,29 @@ mod tests { ) .unwrap(); assert!(nm.checkpoint_due()); + // No checkpoint taken yet: META still holds the load-time .idx size. + // put() only commits non-durably, so the live value is unchanged. assert_eq!( - durable_idx_size(&db_path), - None, - "put() must not commit durably" + live_meta_idx_size(&nm), + Some(0), + "put() must not record checkpoint progress" ); nm.checkpoint(true).unwrap(); assert!(!nm.checkpoint_due()); assert_eq!( - durable_idx_size(&db_path), + live_meta_idx_size(&nm), Some(EXPECTED_INTERVAL * NEEDLE_MAP_ENTRY_SIZE as u64), "checkpoint records how much of the .idx the table reflects" ); - // Snapshot the .rdb while the map is still open: what a crash leaves. + // Everything is durable after the checkpoint, so the map is closed + // first and the snapshot sees the same bytes on every platform. + // (Copying while open fails on Windows, where redb's file lock is + // mandatory: what a crash leaves.) + drop(nm); let crash_copy = dir.path().join("crash.rdb"); std::fs::copy(&db_path, &crash_copy).unwrap(); - drop(nm); let db = Database::open(&crash_copy).unwrap(); let txn = db.begin_read().unwrap(); let table = txn.open_table(NEEDLE_TABLE).unwrap(); diff --git a/seaweed-volume/src/storage/needle_map/file_pool.rs b/seaweed-volume/src/storage/needle_map/file_pool.rs index c5ba0fcc2..e2884de0c 100644 --- a/seaweed-volume/src/storage/needle_map/file_pool.rs +++ b/seaweed-volume/src/storage/needle_map/file_pool.rs @@ -205,6 +205,18 @@ mod tests { use std::os::unix::fs::FileExt; borrowed.read_exact_at(&mut buf, 0).unwrap(); } + #[cfg(windows)] + { + use std::os::windows::fs::FileExt; + let mut filled = 0; + let mut at = 0; + while filled < buf.len() { + let n = borrowed.seek_read(&mut buf[filled..], at).unwrap(); + assert!(n != 0, "unexpected EOF in seek_read"); + filled += n; + at += n as u64; + } + } assert_eq!(&buf, b"first"); } diff --git a/seaweed-volume/src/storage/volume.rs b/seaweed-volume/src/storage/volume.rs index e6f000c69..76c144f14 100644 --- a/seaweed-volume/src/storage/volume.rs +++ b/seaweed-volume/src/storage/volume.rs @@ -4418,6 +4418,18 @@ impl Volume { self.fail_fsync_for_test = fail; } + /// Test-only read of the redb META `.idx` size through the live handle. + /// See `needle_map::test_support::live_meta_idx_size` for why durability + /// tests read this live instead of copying the open `.rdb` (Windows + /// mandatory file locking rejects reads of the locked file). + #[cfg(test)] + pub(crate) fn live_meta_idx_size_for_test(&self) -> Option { + match self.nm.as_ref() { + Some(NeedleMap::Redb(nm)) => nm.live_meta_idx_size(), + _ => None, + } + } + #[cfg(test)] pub(crate) fn fail_next_idx_sync_for_test(&mut self, fail: bool) { self.fail_idx_sync_for_test = fail; @@ -6270,8 +6282,6 @@ mod tests { #[test] fn test_redb_volume_checkpoint_flushes_dat_before_index() { - use crate::storage::needle_map::test_support::durable_idx_size; - let tmp = TempDir::new().unwrap(); let dir = tmp.path().to_str().unwrap(); let mut v = Volume::new( @@ -6282,7 +6292,6 @@ mod tests { &VolumeSpec::default(), ) .unwrap(); - let rdb_path = std::path::PathBuf::from(v.file_name(".rdb")); let write = |v: &mut Volume, i: u64| { let mut n = Needle { id: NeedleId(i), @@ -6297,7 +6306,10 @@ mod tests { for i in 1..1000 { write(&mut v, i); } - assert_eq!(durable_idx_size(&rdb_path), None); + // No checkpoint due yet, so META still holds the load-time .idx + // size. Read live: copying the open .rdb fails on Windows, where + // redb's file lock is mandatory (see live_meta_idx_size). + assert_eq!(v.live_meta_idx_size_for_test(), Some(0)); // The 1000th write makes an index checkpoint due. A checkpoint makes // the index durable, so the volume has to flush the .dat first, and @@ -6305,12 +6317,16 @@ mod tests { // row pointing past the end of an unflushed .dat loads read-only. v.fail_next_fsync_for_test(true); write(&mut v, 1000); - assert_eq!(durable_idx_size(&rdb_path), None); + assert_eq!( + v.live_meta_idx_size_for_test(), + Some(0), + "skipped checkpoint must not record progress" + ); v.fail_next_fsync_for_test(false); write(&mut v, 1001); assert_eq!( - durable_idx_size(&rdb_path), + v.live_meta_idx_size_for_test(), Some(1001 * NEEDLE_MAP_ENTRY_SIZE as u64) ); } diff --git a/seaweed-volume/src/storage/volume_idx_repair.rs b/seaweed-volume/src/storage/volume_idx_repair.rs index 459987200..e67a0c881 100644 --- a/seaweed-volume/src/storage/volume_idx_repair.rs +++ b/seaweed-volume/src/storage/volume_idx_repair.rs @@ -216,7 +216,9 @@ mod tests { use crate::storage::needle::crc::CRC; use crate::storage::needle_map::NeedleMapKind; use crate::storage::volume::VolumeSpec; - use std::os::unix::fs::{FileExt, PermissionsExt}; + use std::io::{Seek, SeekFrom}; + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt; use tempfile::TempDir; fn open_volume(dir: &str) -> Volume { @@ -253,7 +255,7 @@ mod tests { /// writes (key, offset 0, tombstone) rows over the front of .idx instead of /// appending them. fn clobber_idx_head(idx_path: &str, keys: &[u64]) { - let file = OpenOptions::new().write(true).open(idx_path).unwrap(); + let mut file = OpenOptions::new().write(true).open(idx_path).unwrap(); for (i, key) in keys.iter().enumerate() { let mut row = Vec::new(); idx::write_index_entry( @@ -263,8 +265,13 @@ mod tests { TOMBSTONE_FILE_SIZE, ) .unwrap(); - file.write_at(&row, (i * NEEDLE_MAP_ENTRY_SIZE) as u64) + // Positional write without Unix-only `FileExt::write_at`, so this + // helper (and the tests using it) also builds on Windows. + // Single-threaded test helper: no concurrent reader can move the + // offset between seek and write. + file.seek(SeekFrom::Start((i * NEEDLE_MAP_ENTRY_SIZE) as u64)) .unwrap(); + file.write_all(&row).unwrap(); } } @@ -302,6 +309,7 @@ mod tests { let size_before = idx_size(&idx_path); // The rewrite replaces .idx wholesale, so it must not widen the mode. + #[cfg(unix)] fs::set_permissions(&idx_path, fs::Permissions::from_mode(0o600)).unwrap(); // Deletes against needles 9..12 land on the front of .idx and take the @@ -321,11 +329,14 @@ mod tests { let want = size_before + 4 * NEEDLE_MAP_ENTRY_SIZE as u64; assert_eq!(idx_size(&idx_path), want, "idx size after recovery"); - assert_eq!( - fs::metadata(&idx_path).unwrap().permissions().mode() & 0o777, - 0o600, - "idx mode after recovery" - ); + #[cfg(unix)] + { + assert_eq!( + fs::metadata(&idx_path).unwrap().permissions().mode() & 0o777, + 0o600, + "idx mode after recovery" + ); + } // The recovered rows go back in front, so .idx is in .dat append order // again: the fingerprint is gone and the last row is still the .dat tail.