diff --git a/.config/nextest.toml b/.config/nextest.toml index 63a141f3773..eb99cf61619 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -1,6 +1,14 @@ [profile.default] slow-timeout = { period = "30s", terminate-after = 5 } +# The multi-GiB regression tests are `#[ignore]`d, so only the "Rust tests (linux-musl)" job +# runs them. Each holds several GiB live, so they are held to one at a time and given far more +# than the default time budget. +[test-groups.slow-multi-gib] +max-threads = 1 + [[profile.default.overrides]] -filter = 'test(compress_large_int | fsst_compress_offsets_overflow_i32)' +filter = 'test(build_views_offsets_overflow_i32) + test(compress_large_int) + test(fsst_compress_offsets_overflow_i32)' +test-group = 'slow-multi-gib' +slow-timeout = { period = "60s", terminate-after = 20 } priority = 100 diff --git a/.github/workflows/musl.yml b/.github/workflows/musl.yml index c331dbc2859..43a181c9891 100644 --- a/.github/workflows/musl.yml +++ b/.github/workflows/musl.yml @@ -71,6 +71,26 @@ jobs: --exclude vortex-bench --exclude lance-bench --exclude datafusion-bench --exclude vortex-datafusion \ --exclude compress-bench --exclude random-access-bench --exclude vortex-bench-server + # The multi-GiB regression tests are `#[ignore]`d, so every test job skips them by + # default and this step is the only place they run. This job is the one that covers + # every event: `Rust tests (linux-arm64)` is gated to the upstream repository and is + # skipped on pushes to develop, so gating them there would leave forks and develop + # untested. They are named explicitly rather than run with a bare `--run-ignored`, + # which would also un-ignore the CUDA tests that have no GPU here, and the cargo + # arguments match the step above so that nothing is rebuilt. The `slow-multi-gib` + # nextest test group keeps them from running concurrently, since each holds several + # GiB live. + - name: Run multi-GiB regression tests + shell: bash + run: | + cargo nextest run --cargo-profile ci --locked --workspace --no-fail-fast --run-ignored only \ + --exclude vortex-cuda --exclude vortex-cub --exclude vortex-nvcomp \ + --exclude gpu-scan-cli --exclude vortex-test-e2e-cuda --exclude vortex-python-cuda \ + --exclude vortex-duckdb --exclude duckdb-bench --exclude vortex-sqllogictest \ + --exclude vortex-bench --exclude lance-bench --exclude datafusion-bench --exclude vortex-datafusion \ + --exclude compress-bench --exclude random-access-bench --exclude vortex-bench-server \ + -E 'test(build_views_offsets_overflow_i32) + test(compress_large_int) + test(fsst_compress_offsets_overflow_i32)' + - name: Alert incident.io if: failure() && github.event_name == 'push' && github.ref == 'refs/heads/develop' uses: ./.github/actions/alert-incident-io diff --git a/.github/workflows/rust-instrumented.yml b/.github/workflows/rust-instrumented.yml index de135445176..26055db907b 100644 --- a/.github/workflows/rust-instrumented.yml +++ b/.github/workflows/rust-instrumented.yml @@ -177,6 +177,7 @@ jobs: MSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer" TSAN_OPTIONS: "symbolize=1:suppressions=${{ github.workspace }}/vortex-ffi/tsan_suppressions.txt" TSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer" + # Compiles the multi-GiB tests out of the sanitizer build entirely. VORTEX_SKIP_SLOW_TESTS: "1" # -Cunsafe-allow-abi-mismatch=sanitizer: libraries like compiler_builtins # unset -Zsanitizer flag and we should allow that. @@ -243,6 +244,7 @@ jobs: MSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer" TSAN_OPTIONS: "symbolize=1:suppressions=${{ github.workspace }}/vortex-ffi/tsan_suppressions.txt" TSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer" + # Compiles the multi-GiB tests out of the sanitizer build entirely. VORTEX_SKIP_SLOW_TESTS: "1" # -Cunsafe-allow-abi-mismatch=sanitizer: libraries like compiler_builtins # unset -Zsanitizer flag and we should allow that. diff --git a/Cargo.toml b/Cargo.toml index 389d624c98f..59c529e2cc4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -441,3 +441,8 @@ incremental = false # This improved build times significantly for default common cases that we use locally [profile.dev.package.vortex-fastlanes] debug = false + +# FSST's compression kernel is hot enough that an unoptimized build dominates the runtime of the +# multi-GiB `fsst_compress_offsets_overflow_i32` regression (106s -> 64s locally when optimized). +[profile.ci.package.fsst-rs] +opt-level = 3 diff --git a/encodings/fsst/src/compress.rs b/encodings/fsst/src/compress.rs index 9df8f6989dd..a02e97dffb6 100644 --- a/encodings/fsst/src/compress.rs +++ b/encodings/fsst/src/compress.rs @@ -368,8 +368,22 @@ mod tests { assert!(!fsst_output_fits_in_i32_offsets(usize::MAX)); } + /// The i64 codes-offsets path must build a valid array. Driving `compress_views::` + /// directly covers the branch on a three-string input, so day-to-day coverage of it does not + /// depend on the multi-GiB `tests::fsst_compress_offsets_overflow_i32`. + #[test] + fn codes_offsets_i64_path_roundtrips() -> VortexResult<()> { + let array = VarBinViewArray::from_iter_str(["hello", "world", "fsst encoded"]); + let mut ctx = array_session().create_execution_ctx(); + let compressor = fsst_train_compressor(array.as_array(), &mut ctx)?; + let mask = array.validity()?.execute_mask(array.len(), &mut ctx)?; + let fsst = super::compress_views::(array.as_view(), &mask, &compressor, &mut ctx)?; + assert_eq!(fsst.codes().offsets().dtype().as_ptype(), PType::I64); + assert_eq!(fsst.len(), array.len()); + Ok(()) + } + /// Small inputs fit the i32 bound, so `fsst_compress` must pick i32 offsets. - /// The i64 branch is covered by `tests::fsst_compress_offsets_overflow_i32`. #[test] fn codes_offsets_dtype_small_input_is_i32() -> VortexResult<()> { let array = VarBinViewArray::from_iter_str(["hello", "world", "fsst encoded"]); diff --git a/encodings/fsst/src/tests.rs b/encodings/fsst/src/tests.rs index 2b908a8997f..00019927370 100644 --- a/encodings/fsst/src/tests.rs +++ b/encodings/fsst/src/tests.rs @@ -126,8 +126,6 @@ fn test_fsst_array_ops() { assert_arrays_eq!(fsst_array, canonical_array, &mut ctx); } -// TODO(someone): ideally CI would run this in release mode as well since debug builds make the -// allocation and compression loop substantially slower. /// Regression for #7833: [`fsst_compress`] must accept inputs whose cumulative compressed /// bytes exceed [`i32::MAX`]. Before the fix, the compress path hardcoded /// [`VarBinBuilder`] for the FSST output and panicked in @@ -141,17 +139,20 @@ fn test_fsst_array_ops() { /// is on the FSST output side. After the fix the test must succeed with the row count /// preserved. /// -/// Allocates ~1.1 GiB for the input and ~2.1 GiB for the FSST output (~3.2 GiB total), so -/// it is gated to CI runs and skipped when `VORTEX_SKIP_SLOW_TESTS` is set. To run it -/// locally: +/// Allocates ~1.1 GiB for the input and ~2.1 GiB for the FSST output (~3.2 GiB total), so it is +/// ignored by default and run only by the "Rust tests (linux-musl)" CI job. Setting +/// `VORTEX_SKIP_SLOW_TESTS` at build time drops it from the binary, which is how the sanitizer +/// jobs avoid compiling it at all. To run it locally (release mode, since debug builds make the +/// allocation and compression loop substantially slower): /// /// ```text -/// CI=1 cargo test --release -p vortex-fsst fsst_compress_offsets +/// cargo test --release -p vortex-fsst fsst_compress_offsets -- --ignored /// ``` /// /// [`fsst_compress`]: crate::compress::fsst_compress -#[test_with::env(CI)] #[test_with::no_env(VORTEX_SKIP_SLOW_TESTS)] +#[test] +#[ignore = "slow: allocates ~3.2 GiB, run by the \"Rust tests (linux-musl)\" CI job"] fn fsst_compress_offsets_overflow_i32() { const STRING_LEN: usize = 64 * 1024; // Escape coding doubles every byte, so ~1.06 GiB of input compresses to ~2.13 GiB, diff --git a/vortex-array/src/arrays/varbinview/build_views.rs b/vortex-array/src/arrays/varbinview/build_views.rs index 8a6a5a0530d..bf4a0179858 100644 --- a/vortex-array/src/arrays/varbinview/build_views.rs +++ b/vortex-array/src/arrays/varbinview/build_views.rs @@ -492,8 +492,6 @@ mod tests { build_views(0, MAX_BUFFER_LEN, bytes, &[31u32]); } - // TODO(someone): ideally CI would run this in release mode as well, since debug builds make the - // ~2.25 GiB allocation and fill loop substantially slower. /// Slow regression for the single-buffer fast-path guard. The fast path is only valid when the /// whole heap fits in one buffer (`bytes.len() <= max_buffer_len`); once the heap exceeds /// [`MAX_BUFFER_LEN`] (`i32::MAX`, ~2.0 GiB) `build_views` must roll the heap into multiple @@ -506,15 +504,19 @@ mod tests { /// the fast path swallowed the whole heap, it would emit a single >2 GiB buffer with offsets past /// `i32::MAX`, which the buffer-count and buffer-size assertions catch. /// - /// Allocates ~2.25 GiB, so it is gated to CI and skipped when `VORTEX_SKIP_SLOW_TESTS` is set: + /// Allocates ~2.25 GiB, so it is ignored by default and run only by the "Rust tests + /// (linux-musl)" CI job. Setting `VORTEX_SKIP_SLOW_TESTS` at build time drops it from the + /// binary, which is how the sanitizer jobs avoid compiling it at all. To run it locally + /// (release mode, since debug builds make the allocation and fill loop substantially slower): /// /// ```text - /// CI=1 cargo test --release -p vortex-array build_views_offsets_overflow + /// cargo test --release -p vortex-array build_views_offsets_overflow -- --ignored /// ``` /// /// [`MAX_BUFFER_LEN`]: super::MAX_BUFFER_LEN - #[test_with::env(CI)] #[test_with::no_env(VORTEX_SKIP_SLOW_TESTS)] + #[test] + #[ignore = "slow: allocates ~2.25 GiB, run by the \"Rust tests (linux-musl)\" CI job"] fn build_views_offsets_overflow_i32() { const STRING_LEN: usize = 64 * 1024; // Comfortably past MAX_BUFFER_LEN (`i32::MAX` ~= 2.0 GiB) so the heap must roll over. diff --git a/vortex-btrblocks/src/schemes/integer/tests.rs b/vortex-btrblocks/src/schemes/integer/tests.rs index a9ef24dc0e0..047a6582ca5 100644 --- a/vortex-btrblocks/src/schemes/integer/tests.rs +++ b/vortex-btrblocks/src/schemes/integer/tests.rs @@ -15,11 +15,13 @@ use vortex_array::arrays::Dict; use vortex_array::arrays::Masked; use vortex_array::arrays::PrimitiveArray; use vortex_array::assert_arrays_eq; +use vortex_array::dtype::PType; use vortex_array::validity::Validity; use vortex_buffer::Buffer; use vortex_buffer::BufferMut; use vortex_buffer::buffer; use vortex_compressor::CascadingCompressor; +use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_fastlanes::RLE; use vortex_sequence::Sequence; @@ -128,8 +130,16 @@ fn test_rle_compression() -> VortexResult<()> { Ok(()) } -#[test_with::env(CI)] +/// Compresses 50M values, so it is ignored by default and run only by the "Rust tests +/// (linux-musl)" CI job. Setting `VORTEX_SKIP_SLOW_TESTS` at build time drops it from the +/// binary, which is how the sanitizer jobs avoid compiling it at all. To run it locally: +/// +/// ```text +/// cargo test --release -p vortex-btrblocks compress_large_int -- --ignored +/// ``` #[test_with::no_env(VORTEX_SKIP_SLOW_TESTS)] +#[test] +#[ignore = "slow: compresses 50M values, run by the \"Rust tests (linux-musl)\" CI job"] fn compress_large_int() -> VortexResult<()> { const NUM_LISTS: usize = 10_000; const ELEMENTS_PER_LIST: usize = 5_000; @@ -146,3 +156,45 @@ fn compress_large_int() -> VortexResult<()> { Ok(()) } + +/// The compressor picks ALP exponents from a sample, so values the sample did not represent must +/// be stored as patches, indexed per chunk. This is the structure `compress_large_int` reaches +/// only by scale; here the misfit values are placed deliberately, which also pins the +/// `patch_chunk_offsets` width across the three magnitudes it is chosen from. +#[rstest::rstest] +#[case::sparse_patches(200_000, 1_000, PType::U8)] +#[case::dense_patches(200_000, 100, PType::U16)] +#[case::many_patches(1_000_000, 10, PType::U32)] +fn alp_patches_are_chunk_indexed( + #[case] len: usize, + #[case] patch_every: usize, + #[case] chunk_offsets_ptype: PType, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + + // Whole numbers dominate, so the sampled exponents encode them exactly; the sprinkled values + // need more decimal digits than those exponents can represent and must be patched. + let values = (0..len) + .map(|i| { + if i % patch_every == patch_every - 1 { + i as f64 + 0.123_456_789_012_345 + } else { + i as f64 + } + }) + .collect::() + .into_array(); + + let compressed = BtrBlocksCompressor::default().compress(&values, &mut ctx)?; + + let offsets = compressed + .children_names() + .iter() + .position(|name| name == "patch_chunk_offsets") + .map(|idx| compressed.children()[idx].clone()) + .vortex_expect("compressed array must carry chunk-indexed ALP patches"); + assert_eq!(offsets.dtype().as_ptype(), chunk_offsets_ptype); + + assert_arrays_eq!(compressed, values, &mut ctx); + Ok(()) +}