From 2bc7ffa4dbbe5e1cf12fe59eba767ba705bafc97 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:13:01 +0700 Subject: [PATCH 1/8] feat!: mark growable enums non_exhaustive and write the version policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: `BackendKind`, `Compression`, `SessionMode`, `MemoryClass`, `MemoryTier`, `Tier`, `ModelSpec` and the TUI's `Panel`, `KeyInput` and `Transition` are `#[non_exhaustive]`. A consumer matching on any of them needs a `_` arm; in exchange, the next backend or codec does not break them. The interesting half is the enums that are *not* marked. `Predicate`, `Comparison`, `Literal`, `AggregateFunction`, `DistanceMetric` and the codec's `GpuNode` stay exhaustive on purpose: they are the plan vocabulary, every kernel must handle every variant, and the CPU reference is what the GPU paths are checked against. A `_` arm there is not forward compatibility, it is a wrong answer that compiles. Marking them was the first thing I tried and the compiler caught it — `evaluate_predicate` in the CPU kernels demanded a catch-all, which is exactly the arm that must not exist. Two cross-crate matches needed a decision rather than a `_ => {}`. The dashboard draws an unrecognised backend as `[?]` in yellow, because silently drawing it as `[CPU]` would misreport where the work ran. The detector returns a typed error naming the kind, never a quiet fallback to the reference path. RELEASING.md gains "What a version number means here": that a DataFusion, Arrow or Ballista major bump is a minor bump before 1.0 and a major after — breaking whether or not our own code changes, because ADR-0009 makes DataFusion our public vocabulary and cargo-semver-checks cannot see a break in a re-exported type. And the enum rule above, written down, so the next person knows which list a new enum joins. `datagen` moves behind a default-on feature so a direct consumer of oxidelake-storage can leave the demo generator out. It does not disappear from `oxidelake-api`'s tree: that arrives through oxidelake-runtime, whose `oxide gen-data` needs it, and features are per crate rather than per target. The issue's dependency-tree criterion turned out to rest on a premise that does not hold — datagen imports only arrow, oxidelake-core and two crate-internal modules, so there were never any generator dependencies to remove. `cargo semver-checks -p oxidelake-core` reports the enum change as a major break, which is the declaration this release exists to make. Closes #41 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- crates/oxidelake-compute/src/predict.rs | 1 + crates/oxidelake-core/src/hardware.rs | 1 + crates/oxidelake-core/src/telemetry.rs | 1 + crates/oxidelake-device/src/detector.rs | 11 +++++++ crates/oxidelake-memory/src/class.rs | 1 + crates/oxidelake-memory/src/spill.rs | 1 + crates/oxidelake-runtime/src/session.rs | 1 + crates/oxidelake-storage/Cargo.toml | 7 ++++- crates/oxidelake-storage/src/lib.rs | 2 ++ crates/oxidelake-storage/src/parquet_opts.rs | 1 + crates/oxidelake-tui/src/render.rs | 6 ++++ crates/oxidelake-tui/src/state.rs | 3 ++ docs/RELEASING.md | 33 ++++++++++++++++++++ 13 files changed, 68 insertions(+), 1 deletion(-) diff --git a/crates/oxidelake-compute/src/predict.rs b/crates/oxidelake-compute/src/predict.rs index 18cb033..e57ee4a 100644 --- a/crates/oxidelake-compute/src/predict.rs +++ b/crates/oxidelake-compute/src/predict.rs @@ -68,6 +68,7 @@ pub const PREDICT: &str = "predict"; /// shape is needed: the model path stays the SQL surface, and what changes is /// this description of it. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] pub enum ModelSpec { /// A stack of `Linear` layers named `0.weight`/`0.bias`, `1.weight`/…, /// ReLU between them and nothing after the last. diff --git a/crates/oxidelake-core/src/hardware.rs b/crates/oxidelake-core/src/hardware.rs index c2f6f39..9822393 100644 --- a/crates/oxidelake-core/src/hardware.rs +++ b/crates/oxidelake-core/src/hardware.rs @@ -13,6 +13,7 @@ use crate::EngineError; /// backends are opt-in cargo features and are selected at runtime by the /// hardware detector. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[non_exhaustive] pub enum BackendKind { /// Vectorized CPU execution through Arrow/DataFusion kernels and rayon. CpuSimd, diff --git a/crates/oxidelake-core/src/telemetry.rs b/crates/oxidelake-core/src/telemetry.rs index 05a10a3..8259406 100644 --- a/crates/oxidelake-core/src/telemetry.rs +++ b/crates/oxidelake-core/src/telemetry.rs @@ -13,6 +13,7 @@ use crate::BackendKind; /// The three memory tiers tracked by the spill manager. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[non_exhaustive] pub enum MemoryTier { /// Device (VRAM) memory. Device, diff --git a/crates/oxidelake-device/src/detector.rs b/crates/oxidelake-device/src/detector.rs index b610c84..4378e7b 100644 --- a/crates/oxidelake-device/src/detector.rs +++ b/crates/oxidelake-device/src/detector.rs @@ -89,6 +89,17 @@ impl HardwareDetector { } instantiate_metal() } + // `BackendKind` is `#[non_exhaustive]`, so a backend added to + // oxidelake-core compiles here rather than breaking this crate. + // It cannot be instantiated without a backend implementation, so + // it is a typed error naming the kind — never a silent fallback + // to the CPU, which would report work as accelerated that ran on + // the reference path. + other => Err(EngineError::plan(format!( + "backend `{other}` has no implementation in this build of \ + oxidelake-device; it was added to BackendKind without a \ + backend behind it" + ))), } } } diff --git a/crates/oxidelake-memory/src/class.rs b/crates/oxidelake-memory/src/class.rs index e0e4cb5..a032c61 100644 --- a/crates/oxidelake-memory/src/class.rs +++ b/crates/oxidelake-memory/src/class.rs @@ -4,6 +4,7 @@ use std::fmt; /// How a host buffer was allocated. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] pub enum MemoryClass { /// Page-locked memory registered with the device driver: eligible for DMA /// without staging copies. diff --git a/crates/oxidelake-memory/src/spill.rs b/crates/oxidelake-memory/src/spill.rs index a27844e..9e929a1 100644 --- a/crates/oxidelake-memory/src/spill.rs +++ b/crates/oxidelake-memory/src/spill.rs @@ -30,6 +30,7 @@ pub type BatchId = u64; /// Where a batch currently lives. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] pub enum Tier { /// Device memory. Device, diff --git a/crates/oxidelake-runtime/src/session.rs b/crates/oxidelake-runtime/src/session.rs index 267cf18..b1dd116 100644 --- a/crates/oxidelake-runtime/src/session.rs +++ b/crates/oxidelake-runtime/src/session.rs @@ -21,6 +21,7 @@ use crate::cluster; /// Where a session executes. #[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] pub enum SessionMode { /// In-process DataFusion with the placement rule targeting the local backend. Embedded { diff --git a/crates/oxidelake-storage/Cargo.toml b/crates/oxidelake-storage/Cargo.toml index 9173c4b..f337355 100644 --- a/crates/oxidelake-storage/Cargo.toml +++ b/crates/oxidelake-storage/Cargo.toml @@ -20,7 +20,12 @@ workspace = true features = ["io-uring"] [features] -default = [] +# `datagen` is the demo-table generator behind `oxide gen-data`. It is on by +# default so the binaries and the test suites get it without asking, and it is +# a feature so a library consumer can leave the demo data out of their build +# -- `oxidelake-api` does (#41). +default = ["datagen"] +datagen = [] io-uring = ["dep:io-uring", "dep:async-trait"] [dependencies] diff --git a/crates/oxidelake-storage/src/lib.rs b/crates/oxidelake-storage/src/lib.rs index c55bc45..45c9a18 100644 --- a/crates/oxidelake-storage/src/lib.rs +++ b/crates/oxidelake-storage/src/lib.rs @@ -10,6 +10,7 @@ //! //! Dependency direction: depends on `oxidelake-core` and `oxidelake-memory`. +#[cfg(feature = "datagen")] pub mod datagen; pub mod errors; pub mod ipc; @@ -21,6 +22,7 @@ pub mod store; #[cfg(all(feature = "io-uring", target_os = "linux"))] pub mod uring; +#[cfg(feature = "datagen")] pub use datagen::{ CHUNK_ROWS, DemoTable, demo_batches, demo_chunks, demo_schema, demo_write_options, write_demo_table, diff --git a/crates/oxidelake-storage/src/parquet_opts.rs b/crates/oxidelake-storage/src/parquet_opts.rs index 3937f71..eb7f0bf 100644 --- a/crates/oxidelake-storage/src/parquet_opts.rs +++ b/crates/oxidelake-storage/src/parquet_opts.rs @@ -9,6 +9,7 @@ use parquet::schema::types::ColumnPath; /// Compression codecs exposed on the command line (`--compression`). #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[non_exhaustive] pub enum Compression { /// ZSTD level 1: good ratio, fast decode (the default). #[default] diff --git a/crates/oxidelake-tui/src/render.rs b/crates/oxidelake-tui/src/render.rs index ebd5d44..c07ec0b 100644 --- a/crates/oxidelake-tui/src/render.rs +++ b/crates/oxidelake-tui/src/render.rs @@ -36,6 +36,12 @@ fn backend_tag(backend: BackendKind) -> Span<'static> { BackendKind::Cuda => ("[CUDA]", Color::Green), BackendKind::Metal => ("[Metal]", Color::Magenta), BackendKind::CpuSimd => ("[CPU]", Color::Blue), + // `BackendKind` is `#[non_exhaustive]`, so a backend added to + // oxidelake-core compiles here instead of breaking the dashboard. + // It renders as a question rather than as nothing: a tag the + // dashboard does not recognise is worth seeing, and silently + // drawing it as `[CPU]` would misreport where the work ran. + _ => ("[?]", Color::Yellow), }; Span::styled( text, diff --git a/crates/oxidelake-tui/src/state.rs b/crates/oxidelake-tui/src/state.rs index cbced61..2cd3aef 100644 --- a/crates/oxidelake-tui/src/state.rs +++ b/crates/oxidelake-tui/src/state.rs @@ -2,6 +2,7 @@ /// The four dashboard panels. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] pub enum Panel { /// Physical plan tree with a hardware tag per operator. PlanDag, @@ -46,6 +47,7 @@ impl Panel { /// Keyboard input, abstracted from the terminal backend so the state machine /// stays pure and testable. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] pub enum KeyInput { /// Move the selection up. Up, @@ -61,6 +63,7 @@ pub enum KeyInput { /// The outcome of handling one input. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] pub enum Transition { /// Keep running. Continue, diff --git a/docs/RELEASING.md b/docs/RELEASING.md index c4b1b16..ba5e948 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -89,6 +89,39 @@ Nine crates version together; a bump anywhere is a bump everywhere. a backend newly supported, a kernel that computes the same thing faster. - **MSRV bumps are minor**, never patch, and the lockfile is the floor. +### The DataFusion coupling + +ADR-0009 makes DataFusion the public vocabulary: `OxideFrame::new` and +`into_inner` take and return a DataFusion `DataFrame`, and +`oxidelake_api::prelude` re-exports DataFusion types. A consumer therefore +compiles against *our* DataFusion, not one of their choosing. + +**A DataFusion, Arrow or Ballista major bump is a minor bump of OxideLake +before 1.0, and a major bump after.** It is breaking whether or not a single +line of our own code changes, because a consumer holding DataFusion 54 types +cannot pass them to a build linked against 55. The chain moves as one unit +(`docs/dependencies.md`), so one bump, one version, one CHANGELOG entry that +names the upstream versions — never folded into a patch alongside other work. + +`cargo-semver-checks` will not see this one: our signatures are unchanged and +the break is in a type we re-export. It is the release manager's to declare. + +### Enums that may grow, and vocabulary that may not + +Public enums are `#[non_exhaustive]` where a new variant is a thing the world +gains — `BackendKind`, `Compression`, `SessionMode`, `MemoryClass`, +`MemoryTier`, `Tier`, `ModelSpec`, and the TUI's `Panel`/`KeyInput`/ +`Transition`. Adding to those is then a minor bump rather than a break, and a +consumer's `match` needs a `_` arm. + +The plan vocabulary is **deliberately exhaustive**: `Predicate`, `Comparison`, +`Literal`, `AggregateFunction`, `DistanceMetric` and the codec's `GpuNode`. +Every kernel must handle every variant, and the CPU reference is the thing +the GPU paths are checked against — so a `_` arm there is not forward +compatibility, it is a wrong answer that compiles. Adding a variant to those +is a break on purpose, and the compiler naming every kernel that has not +implemented it is the feature. + ## If something fails mid-release - **Before publish**: fix, delete the tag (`git push --delete origin vX.Y.Z`), From 7fd820257ba4132624ffbfe7759467025982a920 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:24:06 +0700 Subject: [PATCH 2/8] feat(planner): fingerprint the plan codec so a mixed fleet fails at decode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: the codec payload header grew four bytes and `VERSION` is 2. A 0.1.x executor and a 0.2.0 scheduler refuse each other's plans, which is the point. `postcard` is not self-describing. A field added to `GpuNode`, or two fields of the same width reordered, decodes without complaint into the wrong parameters — so a rolling upgrade did not fail, it computed a confident wrong answer from a plausible-looking plan. The version byte only helps if somebody remembers to bump it. The fingerprint is derived instead: FNV-1a over the crate version, the version byte, and `oxidelake_compute::FEATURE_MASK`. It therefore moves whether or not anybody remembered. The feature mask is the half that catches the quieter failure — a planner built with `predict` emits plans referencing a UDF an executor without it cannot resolve, which today fails at execution time and reads like a query bug rather than a deployment one. `check_fingerprint` is extracted rather than inlined so the refusal is testable without a `TaskContext`; a failure nobody can set up on purpose is a failure nobody tests. Its message names both fingerprints and both things that could differ, because the person reading it is looking at a cluster that half works. The insta snapshot is the review-time half: `tests/codec_wire.rs` pins the exact bytes of every `GpuNode` variant, so changing the wire form moves a committed file and has to be explained in the same pull request. I decoded all four encodings by hand before accepting it — variant tags, zigzag varints, little-endian f32s, string lengths — rather than accepting bytes because the test wrote them. Not done here: refusing at *registration* rather than at decode. That needs a Ballista registration hook, and decode-time refusal already stops the wrong answer; registration-time would only make the error arrive earlier and read better. Left on #42. Refs #42 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- Cargo.lock | 1 + crates/oxidelake-compute/src/lib.rs | 17 +++ crates/oxidelake-planner/Cargo.toml | 3 + crates/oxidelake-planner/src/codec.rs | 143 +++++++++++++++++- crates/oxidelake-planner/tests/codec_wire.rs | 98 ++++++++++++ .../codec_wire__gpu_node_encodings.snap | 8 + 6 files changed, 267 insertions(+), 3 deletions(-) create mode 100644 crates/oxidelake-planner/tests/codec_wire.rs create mode 100644 crates/oxidelake-planner/tests/snapshots/codec_wire__gpu_node_encodings.snap diff --git a/Cargo.lock b/Cargo.lock index da09e40..8814bda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4085,6 +4085,7 @@ dependencies = [ "arrow", "datafusion", "datafusion-proto", + "insta", "oxidelake-compute", "oxidelake-core", "postcard", diff --git a/crates/oxidelake-compute/src/lib.rs b/crates/oxidelake-compute/src/lib.rs index 1b00d23..9d20e5a 100644 --- a/crates/oxidelake-compute/src/lib.rs +++ b/crates/oxidelake-compute/src/lib.rs @@ -20,6 +20,23 @@ pub mod exec; pub mod operator; pub mod udf; +/// Which optional capabilities this build of the compute layer carries, as a +/// bitmask. +/// +/// A plan is only executable by an executor that can run every node in it, +/// and the features decide what a planner will put there — `predict` adds a +/// UDF the planner will happily reference and an executor without it cannot +/// resolve. The plan codec folds this into its fingerprint so the mismatch +/// is refused when the plan is decoded rather than discovered when it runs +/// (#42). +/// +/// Bit 0 `predict`, bit 1 `cuda`, bit 2 `metal`. Append, never renumber: an +/// old executor reading a new mask must still disagree with it, which it +/// does as long as the bits it knows keep their places. +pub const FEATURE_MASK: u32 = (cfg!(feature = "predict") as u32) + | ((cfg!(feature = "cuda") as u32) << 1) + | ((cfg!(feature = "metal") as u32) << 2); + pub use backend::local_backend; pub use exec::{ GpuAggregateExec, GpuFilterExec, GpuHashJoinExec, GpuVectorDistanceExec, diff --git a/crates/oxidelake-planner/Cargo.toml b/crates/oxidelake-planner/Cargo.toml index 10ac705..49be60c 100644 --- a/crates/oxidelake-planner/Cargo.toml +++ b/crates/oxidelake-planner/Cargo.toml @@ -27,4 +27,7 @@ tracing.workspace = true [dev-dependencies] arrow.workspace = true +# The codec snapshot: a change to `GpuNode`'s wire form has to move a +# committed file, so it cannot pass review unnoticed (#42). +insta.workspace = true tokio.workspace = true diff --git a/crates/oxidelake-planner/src/codec.rs b/crates/oxidelake-planner/src/codec.rs index 3a736bb..88f17d2 100644 --- a/crates/oxidelake-planner/src/codec.rs +++ b/crates/oxidelake-planner/src/codec.rs @@ -22,7 +22,68 @@ use oxidelake_core::BackendKind; use serde::{Deserialize, Serialize}; const MAGIC: &[u8; 4] = b"OXGP"; -const VERSION: u8 = 1; +/// Bumped to 2 by the fingerprint: the header grew four bytes (#42). +const VERSION: u8 = 2; + +/// What the two ends of a plan must agree on, beyond the version byte. +/// +/// `postcard` is not self-describing. A field added to or reordered within +/// `GpuNode` decodes into a *different* field of the same width without +/// complaint, so a mixed-build fleet does not fail — it computes the wrong +/// answer from plausible-looking parameters. The version byte only helps if +/// somebody remembers to bump it; this is derived, so it changes whether or +/// not anybody remembered. +/// +/// It folds in the crate version (every release is a new fingerprint, which +/// is blunt but never wrong) and the compute layer's enabled features (a +/// planner with `predict` emits plans referencing a UDF an executor without +/// it cannot resolve — today that fails at execution, which is late and +/// looks like a query bug rather than a deployment one). +const FINGERPRINT: u32 = fingerprint(); + +const fn fnv1a(mut hash: u32, bytes: &[u8]) -> u32 { + let mut i = 0; + while i < bytes.len() { + hash ^= bytes[i] as u32; + hash = hash.wrapping_mul(0x0100_0193); + i += 1; + } + hash +} + +const fn fingerprint() -> u32 { + let h = fnv1a(0x811c_9dc5, env!("CARGO_PKG_VERSION").as_bytes()); + let h = fnv1a(h, &[VERSION]); + fnv1a(h, &oxidelake_compute::FEATURE_MASK.to_le_bytes()) +} + +/// `MAGIC` + version byte + fingerprint. +const HEADER: usize = MAGIC.len() + 1 + 4; + +/// Refuses a plan this build cannot be trusted to read, before `postcard` +/// turns it into confident nonsense (#42). +/// +/// Different fingerprints mean a different crate version or a different +/// feature set at the other end, and either can change what the bytes after +/// the header mean. Extracted so the refusal is testable without a +/// `TaskContext`: the failure it guards against is the one nobody sets up by +/// accident. +fn check_fingerprint(tag: &[u8]) -> Result<()> { + let mut bytes = [0u8; 4]; + bytes.copy_from_slice(tag); + let theirs = u32::from_le_bytes(bytes); + if theirs == FINGERPRINT { + return Ok(()); + } + Err(DataFusionError::Internal(format!( + "OxidePhysicalCodec: plan was encoded by a build with fingerprint {theirs:#010x}; \ + this build is {FINGERPRINT:#010x}. The two ends of the cluster are not the same \ + version, or were built with different features (oxidelake-compute feature mask \ + {:#x} here). The plan is refused rather than decoded into parameters that would \ + only look right.", + oxidelake_compute::FEATURE_MASK + ))) +} /// Serialized form of one `Gpu*Exec` node (children excluded). #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -232,14 +293,15 @@ impl PhysicalExtensionCodec for OxidePhysicalCodec { inputs: &[Arc], ctx: &TaskContext, ) -> Result> { - if buf.len() > MAGIC.len() + 1 && &buf[..MAGIC.len()] == MAGIC { + if buf.len() > HEADER && &buf[..MAGIC.len()] == MAGIC { let version = buf[MAGIC.len()]; if version != VERSION { return Err(DataFusionError::Internal(format!( "OxidePhysicalCodec: unsupported payload version {version} (expected {VERSION})" ))); } - let node: GpuNode = postcard::from_bytes(&buf[MAGIC.len() + 1..]).map_err(|e| { + check_fingerprint(&buf[MAGIC.len() + 1..HEADER])?; + let node: GpuNode = postcard::from_bytes(&buf[HEADER..]).map_err(|e| { DataFusionError::Internal(format!("OxidePhysicalCodec: corrupt payload: {e}")) })?; return Self::build(node, inputs); @@ -252,6 +314,7 @@ impl PhysicalExtensionCodec for OxidePhysicalCodec { Some(gpu) => { buf.extend_from_slice(MAGIC); buf.push(VERSION); + buf.extend_from_slice(&FINGERPRINT.to_le_bytes()); let payload = postcard::to_allocvec(&gpu).map_err(|e| { DataFusionError::Internal(format!("OxidePhysicalCodec: encode failed: {e}")) })?; @@ -262,3 +325,77 @@ impl PhysicalExtensionCodec for OxidePhysicalCodec { } } } + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + /// The fingerprint is derived, not declared, so it cannot be forgotten + /// the way the version byte can (#42). + #[test] + fn the_fingerprint_depends_on_the_version_and_the_feature_set() { + // Recomputed here the way a different build would, rather than + // asserting a literal: a literal would have to be updated on every + // release and would then be testing that somebody updated it. + let expected = { + let h = fnv1a(0x811c_9dc5, env!("CARGO_PKG_VERSION").as_bytes()); + let h = fnv1a(h, &[VERSION]); + fnv1a(h, &oxidelake_compute::FEATURE_MASK.to_le_bytes()) + }; + assert_eq!(FINGERPRINT, expected); + + // A different crate version and a different feature set each move it. + let other_version = fnv1a( + fnv1a(fnv1a(0x811c_9dc5, b"0.0.0-not-this-build"), &[VERSION]), + &oxidelake_compute::FEATURE_MASK.to_le_bytes(), + ); + assert_ne!(FINGERPRINT, other_version, "the crate version is in it"); + + let other_features = fnv1a( + fnv1a( + fnv1a(0x811c_9dc5, env!("CARGO_PKG_VERSION").as_bytes()), + &[VERSION], + ), + &(oxidelake_compute::FEATURE_MASK ^ 0b1).to_le_bytes(), + ); + assert_ne!( + FINGERPRINT, other_features, + "flipping the `predict` bit must move it: that is the mismatch \ + this exists to catch" + ); + } + + /// The refusal names both fingerprints and says what it means, because + /// the person reading it is looking at a cluster that half works. + #[test] + fn a_foreign_fingerprint_is_refused_and_named() { + check_fingerprint(&FINGERPRINT.to_le_bytes()).expect("our own plan decodes"); + + let foreign = FINGERPRINT ^ 0xdead_beef; + let err = check_fingerprint(&foreign.to_le_bytes()) + .expect_err("a plan from another build is refused"); + let text = err.to_string(); + assert!(text.contains(&format!("{foreign:#010x}")), "{text}"); + assert!(text.contains(&format!("{FINGERPRINT:#010x}")), "{text}"); + assert!( + text.contains("not the same version") && text.contains("different features"), + "the message says which two things could differ: {text}" + ); + } + + /// The header is exactly what `try_encode` writes, so the offsets the + /// decoder slices at are not two independent guesses. + #[test] + fn the_header_is_magic_version_and_fingerprint() { + assert_eq!(HEADER, MAGIC.len() + 1 + 4); + let mut buf = Vec::new(); + buf.extend_from_slice(MAGIC); + buf.push(VERSION); + buf.extend_from_slice(&FINGERPRINT.to_le_bytes()); + assert_eq!(buf.len(), HEADER); + assert_eq!(&buf[..MAGIC.len()], MAGIC); + assert_eq!(buf[MAGIC.len()], VERSION); + check_fingerprint(&buf[MAGIC.len() + 1..HEADER]).expect("its own header verifies"); + } +} diff --git a/crates/oxidelake-planner/tests/codec_wire.rs b/crates/oxidelake-planner/tests/codec_wire.rs new file mode 100644 index 0000000..042e00f --- /dev/null +++ b/crates/oxidelake-planner/tests/codec_wire.rs @@ -0,0 +1,98 @@ +//! The plan codec's wire format, pinned (#42). +//! +//! `postcard` is not self-describing: a field added to `GpuNode`, or two +//! fields of the same width reordered, decodes without complaint into the +//! wrong parameters. A mixed-build fleet then does not fail — it computes a +//! confident wrong answer from a plausible-looking plan. +//! +//! Two things guard that. At run time the header carries a fingerprint +//! derived from the crate version and the compute layer's feature mask, so a +//! plan from a different build is refused at decode. At review time this +//! snapshot is the other half: changing `GpuNode`'s encoding moves a +//! committed file, so it cannot land unnoticed, and whoever moves it has to +//! say why in the same pull request. +//! +//! If this test fails and the change to `GpuNode` was deliberate: bump +//! `VERSION` in `codec.rs` and accept the snapshot with `cargo insta review` +//! — never blind-accept it. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use oxidelake_core::BackendKind; +use oxidelake_core::params::{ + AggregateFunction, AggregateSpec, Comparison, DistanceMetric, Literal, Predicate, +}; +use oxidelake_planner::codec::GpuNode; + +/// Every variant, with values chosen so a reordering of same-width fields +/// shows up: no two numeric fields share a value. +fn every_variant() -> Vec<(&'static str, GpuNode)> { + vec![ + ( + "filter", + GpuNode::Filter { + target: BackendKind::Cuda, + predicate: Predicate::Compare { + column: 3, + op: Comparison::Gt, + literal: Literal::Int64(17), + }, + projection: vec![5, 8], + }, + ), + ( + "hash_join", + GpuNode::HashJoin { + target: BackendKind::Metal, + left_key: 1, + right_key: 2, + projection: Some(vec![4, 6, 9]), + }, + ), + ( + "aggregate", + GpuNode::Aggregate { + target: BackendKind::CpuSimd, + spec: AggregateSpec { + group_by: 7, + aggregates: vec![(AggregateFunction::Sum, 11)], + }, + output: vec![("k".to_owned(), false), ("sv".to_owned(), true)], + }, + ), + ( + "vector_distance", + GpuNode::VectorDistance { + target: BackendKind::Cuda, + column: 13, + query: vec![0.5, 0.25], + metric: DistanceMetric::Cosine, + output_name: "dist".to_owned(), + }, + ), + ] +} + +/// The bytes each variant encodes to, as hex. A diff here is a wire-format +/// change; there is no other reason for these to move. +#[test] +fn every_gpu_node_variant_has_a_pinned_encoding() { + let mut rendered = String::new(); + for (name, node) in every_variant() { + let bytes = postcard::to_allocvec(&node).expect("a GpuNode encodes"); + let hex: Vec = bytes.iter().map(|b| format!("{b:02x}")).collect(); + rendered.push_str(&format!("{name}: {}\n", hex.join(" "))); + } + insta::assert_snapshot!("gpu_node_encodings", rendered); +} + +/// A round trip through the encoding every variant survives, so the snapshot +/// above pins something that is also correct rather than merely stable. +#[test] +fn every_variant_round_trips() { + for (name, node) in every_variant() { + let bytes = postcard::to_allocvec(&node).expect("encodes"); + let back: GpuNode = postcard::from_bytes(&bytes).expect("decodes"); + assert_eq!(back, node, "{name} did not survive a round trip"); + } +} diff --git a/crates/oxidelake-planner/tests/snapshots/codec_wire__gpu_node_encodings.snap b/crates/oxidelake-planner/tests/snapshots/codec_wire__gpu_node_encodings.snap new file mode 100644 index 0000000..95c0781 --- /dev/null +++ b/crates/oxidelake-planner/tests/snapshots/codec_wire__gpu_node_encodings.snap @@ -0,0 +1,8 @@ +--- +source: crates/oxidelake-planner/tests/codec_wire.rs +expression: rendered +--- +filter: 00 01 00 03 03 00 22 02 05 08 +hash_join: 01 02 01 02 01 03 04 06 09 +aggregate: 02 00 07 01 00 0b 02 01 6b 00 02 73 76 01 +vector_distance: 03 01 0d 02 00 00 00 3f 00 00 80 3e 01 04 64 69 73 74 From 7b3487e56e725550e65ec4c5032a6e56e52b424d Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:47:06 +0700 Subject: [PATCH 3/8] feat(cli): add --batch-size, --output and a worker --backend flag Closes #49 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- README.md | 49 +++++ crates/oxidelake-compute/src/backend.rs | 26 +++ crates/oxidelake-compute/src/lib.rs | 2 +- crates/oxidelake-device/src/detector.rs | 18 +- crates/oxidelake-runtime/src/bin/oxide.rs | 53 +++-- .../oxidelake-runtime/src/bin/oxide_worker.rs | 15 +- crates/oxidelake-runtime/src/lib.rs | 4 +- crates/oxidelake-runtime/src/output.rs | 185 +++++++++++++++++ crates/oxidelake-runtime/src/session.rs | 135 ++++++++++++- crates/oxidelake-runtime/tests/cli.rs | 189 ++++++++++++++++++ 10 files changed, 651 insertions(+), 25 deletions(-) create mode 100644 crates/oxidelake-runtime/src/output.rs diff --git a/README.md b/README.md index b3722fe..3ada8d4 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,55 @@ cargo test -p oxidelake-compute --features metal -- --ignored OXIDE_BACKEND=cuda cargo test -p oxidelake-compute --features cuda -- --ignored ``` +## Configuration + +Every knob OxideLake reads, in one table. Flags win over environment +variables; an explicit choice that the machine cannot honour is a startup +error, never a silent fallback. + +### `oxide` + +| Knob | Where | Default | What it does | +| --- | --- | --- | --- | +| `--query`, `-q` | `sql`, `explain`, `tui` | — | The SQL statement (optional for `tui`, which then shows the demo model). | +| `--table`, `-t` | `sql`, `explain`, `tui` | — | Registers `NAME=PATH` (a Parquet file or directory) before planning. Repeatable. | +| `--cluster` | `sql` | — | Runs on a Ballista scheduler (`df://host:port`) instead of in-process. | +| `--target` | `sql`, `explain`, `tui` | detected | Plans for `cpu`, `cuda` or `metal`. Placement only: execution uses the hardware that is present. Embedded mode only — on a cluster the scheduler decides. | +| `--batch-size` | `sql`, `explain`, `tui` | 8192, or 65536 on a GPU target | Rows per record batch. Batch boundaries never change results; larger batches amortise the host↔device round trip. `0` is refused. | +| `--output` | `sql` | `table` | `table`, `json` (an array of objects) or `csv` (RFC 4180 with a header). An empty result still prints the CSV header and `[]`, so a script can tell "no rows" from "the query failed". | +| `--rows` | `gen-data` | `1000000` | Rows in the demo table. | +| `--out` | `gen-data` | — | Output directory; writes `/t.parquet`. | +| `--seed` | `gen-data` | `42` | PRNG seed. The same seed always writes the same bytes. | +| `--row-group-rows` | `gen-data` | `65536` | Rows per Parquet row group. | +| `--compression` | `gen-data` | `zstd` | `none`, `lz4` or `zstd`. | + +### `oxide-scheduler` + +| Knob | Default | What it does | +| --- | --- | --- | +| `--bind-host` | `127.0.0.1` | Address to bind. | +| `--port` | `50050` | gRPC port. | +| `--cluster-backend` | `$OXIDE_CLUSTER_BACKEND`, then `cpu` | The capability the cluster declares, which is what placement rewrites against. | + +### `oxide-worker` + +| Knob | Default | What it does | +| --- | --- | --- | +| `--scheduler-host` / `--scheduler-port` | `localhost` / `50050` | The scheduler to join. | +| `--port` | `50051` | Arrow Flight port for shuffle data. | +| `--grpc-port` | `50052` | gRPC control port. | +| `--concurrent-tasks` | available parallelism | Tasks run at once. | +| `--work-dir` | a temporary directory | Where shuffle files go. | +| `--backend` | `$OXIDE_BACKEND`, then detected | The backend this worker executes on. Selected before the first task, so it reaches the operators; a backend the machine cannot provide is a startup error. | + +### Environment + +| Variable | Read by | What it does | +| --- | --- | --- | +| `OXIDE_BACKEND` | every process that executes operators | Forces `cpu`, `cuda` or `metal` instead of the detected backend. `oxide-worker --backend` overrides it. | +| `OXIDE_CLUSTER_BACKEND` | `oxide-scheduler` | The cluster's declared placement capability (default `cpu`: no GPU rewrites). `--cluster-backend` overrides it. | +| `RUST_LOG` | every binary | `tracing` filter; logs go to stderr, so they never mix into `--output json`. | + ## Repository map | Path | Purpose | diff --git a/crates/oxidelake-compute/src/backend.rs b/crates/oxidelake-compute/src/backend.rs index 949714d..e1cd30e 100644 --- a/crates/oxidelake-compute/src/backend.rs +++ b/crates/oxidelake-compute/src/backend.rs @@ -19,3 +19,29 @@ pub fn local_backend() -> Result, EngineError> { ))), } } + +/// Selects the process backend explicitly, ahead of anything that calls +/// [`local_backend`]. `requested` takes the same values as `OXIDE_BACKEND` +/// (`cpu`, `cuda`, `metal`); `None` means "best available" and ignores the +/// environment, so a caller that wants the environment honoured passes what +/// it read from there. +/// +/// This exists so a command-line flag can win over `OXIDE_BACKEND` and still +/// reach the operators: they read [`local_backend`], which memoises its first +/// answer, so setting the choice after the first query would silently do +/// nothing. An unavailable backend is an error here exactly as it is through +/// the environment — never a silent fallback to the CPU. +/// +/// Returns an error if a backend was already selected in this process: the +/// operators that have already run did so on the old one, and re-pointing +/// them mid-process would make a single query's backend a matter of timing. +pub fn init_local_backend(requested: Option<&str>) -> Result, EngineError> { + let selected = HardwareDetector::select_with(requested).map_err(|e| e.to_string()); + if LOCAL.set(selected).is_err() { + return Err(EngineError::execution( + "the process backend was already selected; init_local_backend must \ + run before the first local_backend() call", + )); + } + local_backend() +} diff --git a/crates/oxidelake-compute/src/lib.rs b/crates/oxidelake-compute/src/lib.rs index 9d20e5a..86a7cb2 100644 --- a/crates/oxidelake-compute/src/lib.rs +++ b/crates/oxidelake-compute/src/lib.rs @@ -37,7 +37,7 @@ pub const FEATURE_MASK: u32 = (cfg!(feature = "predict") as u32) | ((cfg!(feature = "cuda") as u32) << 1) | ((cfg!(feature = "metal") as u32) << 2); -pub use backend::local_backend; +pub use backend::{init_local_backend, local_backend}; pub use exec::{ GpuAggregateExec, GpuFilterExec, GpuHashJoinExec, GpuVectorDistanceExec, aggregate_output_schema, diff --git a/crates/oxidelake-device/src/detector.rs b/crates/oxidelake-device/src/detector.rs index 4378e7b..37d4d04 100644 --- a/crates/oxidelake-device/src/detector.rs +++ b/crates/oxidelake-device/src/detector.rs @@ -104,13 +104,14 @@ impl HardwareDetector { } } +/// The request can arrive through [`BACKEND_ENV`] or through a command-line +/// flag that mirrors it (`oxide-worker --backend`), so the message names the +/// backend and the reason rather than one of the two ways of asking. fn unavailable(kind: BackendKind, compiled_in: bool) -> EngineError { let detail = if compiled_in { - format!( - "requested via {BACKEND_ENV} but no {kind} device or driver is present on this machine" - ) + format!("explicitly requested but no {kind} device or driver is present on this machine") } else { - format!("requested via {BACKEND_ENV} but OxideLake was built without the `{kind}` feature") + format!("explicitly requested but OxideLake was built without the `{kind}` feature") }; EngineError::device(kind, detail) } @@ -197,7 +198,14 @@ mod tests { ), "{err}" ); - assert!(err.to_string().contains(BACKEND_ENV)); + // The message names the backend and why it is unavailable, not + // the way it was asked for: `oxide-worker --backend` reaches the + // same code and naming the environment variable there would send + // the reader to a knob they did not touch. + let message = err.to_string(); + assert!(message.contains("cuda"), "{message}"); + assert!(message.contains("explicitly requested"), "{message}"); + assert!(!message.contains(BACKEND_ENV), "{message}"); } if !availability.metal { let err = HardwareDetector::select_with(Some("metal")).unwrap_err(); diff --git a/crates/oxidelake-runtime/src/bin/oxide.rs b/crates/oxidelake-runtime/src/bin/oxide.rs index df04c88..9c0bb1f 100644 --- a/crates/oxidelake-runtime/src/bin/oxide.rs +++ b/crates/oxidelake-runtime/src/bin/oxide.rs @@ -4,7 +4,7 @@ use std::io::{self, Write}; use clap::{Parser, Subcommand}; use oxidelake_core::BackendKind; -use oxidelake_runtime::{OxideSession, dashboard}; +use oxidelake_runtime::{OutputFormat, OxideSession, SessionOptions, dashboard}; use oxidelake_storage::{Compression, demo_write_options, write_demo_table}; use tracing_subscriber::EnvFilter; @@ -75,6 +75,13 @@ enum Command { /// planned for an absent GPU take the CPU path. Embedded mode only. #[arg(long, value_name = "BACKEND")] target: Option, + /// Rows per record batch, overriding the CPU default (8192) and the + /// GPU-target default (65536). Batch boundaries never change results. + #[arg(long, value_name = "ROWS")] + batch_size: Option, + /// How to print the result: table, json or csv. + #[arg(long, value_name = "FORMAT", default_value = "table")] + output: OutputFormat, }, /// Print the physical plan for a SQL statement (with placement tags in embedded mode). Explain { @@ -87,6 +94,11 @@ enum Command { /// Plan for this backend (cpu, cuda or metal) instead of the detected one. #[arg(long, value_name = "BACKEND")] target: Option, + /// Rows per record batch (see `oxide sql --batch-size`), so the plan + /// is built under the same session configuration as the `oxide sql` + /// run it explains. + #[arg(long, value_name = "ROWS")] + batch_size: Option, }, /// Open the terminal dashboard: run a query and inspect its plan, /// telemetry and column profiles — or a synthetic demo without a query. @@ -100,6 +112,9 @@ enum Command { /// Plan for this backend (cpu, cuda or metal) instead of the detected one. #[arg(long, value_name = "BACKEND")] target: Option, + /// Rows per record batch (see `oxide sql --batch-size`). + #[arg(long, value_name = "ROWS")] + batch_size: Option, }, } @@ -116,16 +131,25 @@ fn parse_tables(tables: &[String]) -> anyhow::Result> { async fn session( cluster: Option<&str>, target: Option, + batch_size: Option, tables: &[String], ) -> anyhow::Result { + let mut options = SessionOptions::new(); + if let Some(rows) = batch_size { + options = options.with_batch_size(rows); + } let session = match (cluster, target) { (Some(_), Some(_)) => anyhow::bail!( "--target picks the embedded placement target; on a cluster the \ scheduler's OXIDE_CLUSTER_BACKEND decides placement" ), - (Some(url), None) => OxideSession::connect(url).await?, - (None, Some(target)) => OxideSession::local_with_target(target)?, - (None, None) => OxideSession::local()?, + (Some(url), None) => OxideSession::connect_with_options(url, &options).await?, + (None, target) => { + if let Some(target) = target { + options = options.with_target(target); + } + OxideSession::local_with_options(&options)? + } }; for (name, path) in parse_tables(tables)? { session.register_parquet(name, path).await?; @@ -162,28 +186,33 @@ async fn main() -> anyhow::Result<()> { tables, cluster, target, + batch_size, + output, } => { - let session = session(cluster.as_deref(), target, &tables).await?; + let session = session(cluster.as_deref(), target, batch_size, &tables).await?; // Not `DataFrame::show()`: that is `println!` inside DataFusion, // so a closed pipe panics there and the CLI never sees it (#34). - let batches = session.sql(&query).await?.collect().await?; - write_out( - &datafusion::arrow::util::pretty::pretty_format_batches(&batches)?.to_string(), - )?; - write_out("\n")?; + let frame = session.sql(&query).await?; + let schema = frame.schema().inner().clone(); + let batches = frame.collect().await?; + write_out(&oxidelake_runtime::output::render( + &schema, &batches, output, + )?)?; } Command::Explain { query, tables, target, + batch_size, } => { - let session = session(None, target, &tables).await?; + let session = session(None, target, batch_size, &tables).await?; write_out(&session.explain(&query).await?)?; } Command::Tui { query, tables, target, + batch_size, } => { if !oxidelake_tui::is_interactive_terminal() { eprintln!("{}", oxidelake_tui::NON_INTERACTIVE_TERMINAL_MESSAGE); @@ -191,7 +220,7 @@ async fn main() -> anyhow::Result<()> { } let model = match &query { Some(sql) => { - let session = session(None, target, &tables).await?; + let session = session(None, target, batch_size, &tables).await?; let names: Vec = parse_tables(&tables)? .into_iter() .map(|(name, _)| name.to_owned()) diff --git a/crates/oxidelake-runtime/src/bin/oxide_worker.rs b/crates/oxidelake-runtime/src/bin/oxide_worker.rs index 5396cca..00077aa 100644 --- a/crates/oxidelake-runtime/src/bin/oxide_worker.rs +++ b/crates/oxidelake-runtime/src/bin/oxide_worker.rs @@ -1,6 +1,7 @@ //! `oxide-worker` — a Ballista executor that can run OxideLake's `Gpu*Exec` operators. use clap::Parser; +use oxidelake_core::BackendKind; use oxidelake_runtime::cluster; use tracing_subscriber::EnvFilter; @@ -29,6 +30,12 @@ struct Cli { /// Directory for shuffle files (default: a temporary directory). #[arg(long)] work_dir: Option, + /// Backend this worker executes on (cpu, cuda or metal), overriding + /// `OXIDE_BACKEND`. A backend this machine cannot provide is a startup + /// error, never a silent fall back to the CPU — a worker that reports + /// GPU capacity it does not have poisons the whole cluster's placement. + #[arg(long, value_name = "BACKEND")] + backend: Option, } #[tokio::main] @@ -38,7 +45,13 @@ async fn main() -> anyhow::Result<()> { .with_writer(std::io::stderr) .init(); let cli = Cli::parse(); - let backend = oxidelake_compute::local_backend()?; + // Selected before anything runs: operators read the memoised + // `local_backend()`, so a choice made after the first task would be + // accepted and then ignored. + let backend = match cli.backend { + Some(kind) => oxidelake_compute::init_local_backend(Some(kind.as_str()))?, + None => oxidelake_compute::local_backend()?, + }; tracing::info!(backend = %backend.kind(), "worker backend"); cluster::run_executor(cluster::executor_config( &cli.scheduler_host, diff --git a/crates/oxidelake-runtime/src/lib.rs b/crates/oxidelake-runtime/src/lib.rs index 25fdd87..8b361a9 100644 --- a/crates/oxidelake-runtime/src/lib.rs +++ b/crates/oxidelake-runtime/src/lib.rs @@ -8,7 +8,9 @@ pub mod cluster; pub mod dashboard; +pub mod output; pub mod session; +pub use output::OutputFormat; pub use oxidelake_compute::udf; -pub use session::{OxideSession, SessionMode}; +pub use session::{OxideSession, SessionMode, SessionOptions}; diff --git a/crates/oxidelake-runtime/src/output.rs b/crates/oxidelake-runtime/src/output.rs new file mode 100644 index 0000000..af3b884 --- /dev/null +++ b/crates/oxidelake-runtime/src/output.rs @@ -0,0 +1,185 @@ +//! How `oxide sql` renders a result set (#49). +//! +//! The table form is for a person reading a terminal; `json` and `csv` exist +//! so the same query can feed a script without a second tool to parse the box +//! drawing. All three render the *same* batches — the format is a rendering +//! choice made after execution, never a planning one, so a result cannot +//! differ between them. + +use std::io::Write; +use std::str::FromStr; + +use datafusion::arrow::array::RecordBatch; +use datafusion::arrow::datatypes::SchemaRef; +use datafusion::arrow::util::pretty::pretty_format_batches; +use oxidelake_core::EngineError; + +/// Output formats accepted by `oxide sql --output`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[non_exhaustive] +pub enum OutputFormat { + /// An aligned ASCII table (the default). + #[default] + Table, + /// A JSON array of objects, one per row. + Json, + /// RFC 4180 CSV with a header row. + Csv, +} + +impl OutputFormat { + /// Every format, in the order `--help` lists them. + pub const ALL: [OutputFormat; 3] = [OutputFormat::Table, OutputFormat::Json, OutputFormat::Csv]; + + /// The canonical lowercase name used on the command line. + pub const fn as_str(self) -> &'static str { + match self { + OutputFormat::Table => "table", + OutputFormat::Json => "json", + OutputFormat::Csv => "csv", + } + } +} + +impl std::fmt::Display for OutputFormat { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +impl FromStr for OutputFormat { + type Err = EngineError; + + fn from_str(s: &str) -> Result { + match s.trim().to_ascii_lowercase().as_str() { + "table" => Ok(OutputFormat::Table), + "json" => Ok(OutputFormat::Json), + "csv" => Ok(OutputFormat::Csv), + other => Err(EngineError::plan(format!( + "unknown output format '{other}'; expected one of table, json, csv" + ))), + } + } +} + +/// Renders `batches` in `format`, terminated by a newline. +/// +/// `schema` is carried separately because an empty result has no batch to take +/// it from, and a CSV reader handed nothing at all cannot tell an empty result +/// from a failed one — so the header row is written even when no row is. +pub fn render( + schema: &SchemaRef, + batches: &[RecordBatch], + format: OutputFormat, +) -> Result { + match format { + OutputFormat::Table => Ok(format!("{}\n", pretty_format_batches(batches)?)), + OutputFormat::Json => { + let mut buffer = Vec::new(); + let mut writer = datafusion::arrow::json::ArrayWriter::new(&mut buffer); + writer.write_batches(&batches.iter().collect::>())?; + writer.finish()?; + finish(buffer) + } + OutputFormat::Csv => { + let mut buffer = Vec::new(); + let mut writer = datafusion::arrow::csv::Writer::new(&mut buffer); + let empty; + let batches = if batches.is_empty() { + empty = [RecordBatch::new_empty(SchemaRef::clone(schema))]; + &empty[..] + } else { + batches + }; + for batch in batches { + writer.write(batch)?; + } + drop(writer); + finish(buffer) + } + } +} + +/// Turns a writer's buffer into a newline-terminated `String`. +fn finish(mut buffer: Vec) -> Result { + if !buffer.ends_with(b"\n") { + buffer.write_all(b"\n").map_err(EngineError::from)?; + } + String::from_utf8(buffer) + .map_err(|e| EngineError::execution(format!("rendered output was not UTF-8: {e}"))) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use std::sync::Arc; + + use datafusion::arrow::array::{Int64Array, StringArray}; + use datafusion::arrow::datatypes::{DataType, Field, Schema}; + + use super::*; + + fn schema() -> SchemaRef { + Arc::new(Schema::new(vec![ + Field::new("k", DataType::Int64, true), + Field::new("name", DataType::Utf8, true), + ])) + } + + fn batch() -> RecordBatch { + RecordBatch::try_new( + schema(), + vec![ + Arc::new(Int64Array::from(vec![Some(1), None])), + Arc::new(StringArray::from(vec![Some("a,b"), None])), + ], + ) + .unwrap() + } + + #[test] + fn every_name_round_trips() { + for format in OutputFormat::ALL { + assert_eq!(format.as_str().parse::().unwrap(), format); + } + assert_eq!(" CSV ".parse::().unwrap(), OutputFormat::Csv); + } + + #[test] + fn an_unknown_name_names_the_alternatives() { + let message = "yaml".parse::().unwrap_err().to_string(); + assert!(message.contains("yaml"), "{message}"); + assert!(message.contains("table, json, csv"), "{message}"); + } + + #[test] + fn json_is_an_array_of_objects_and_drops_nulls() { + let text = render(&schema(), &[batch()], OutputFormat::Json).unwrap(); + assert_eq!(text, "[{\"k\":1,\"name\":\"a,b\"},{}]\n"); + } + + #[test] + fn csv_quotes_an_embedded_comma_and_keeps_the_header() { + let text = render(&schema(), &[batch()], OutputFormat::Csv).unwrap(); + assert_eq!(text, "k,name\n1,\"a,b\"\n,\n"); + } + + /// An empty result still says what the columns were: a script reading the + /// CSV can tell "no rows" from "the query failed". + #[test] + fn an_empty_result_still_has_a_csv_header() { + assert_eq!( + render(&schema(), &[], OutputFormat::Csv).unwrap(), + "k,name\n" + ); + assert_eq!(render(&schema(), &[], OutputFormat::Json).unwrap(), "[]\n"); + } + + #[test] + fn the_table_form_is_the_default() { + assert_eq!(OutputFormat::default(), OutputFormat::Table); + let text = render(&schema(), &[batch()], OutputFormat::Table).unwrap(); + assert!(text.contains("| k "), "{text}"); + assert!(text.ends_with("\n"), "{text}"); + } +} diff --git a/crates/oxidelake-runtime/src/session.rs b/crates/oxidelake-runtime/src/session.rs index b1dd116..176f8d9 100644 --- a/crates/oxidelake-runtime/src/session.rs +++ b/crates/oxidelake-runtime/src/session.rs @@ -35,6 +35,62 @@ pub enum SessionMode { }, } +/// Knobs a session is built with. +/// +/// Every field has a default that matches the no-argument constructors, so +/// `SessionOptions::default()` and [`OxideSession::local`] agree. The struct +/// is `#[non_exhaustive]`: a knob added later is a minor release, not a +/// broken build for anyone who used the builder methods. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +#[non_exhaustive] +pub struct SessionOptions { + /// Plan for this backend instead of the detected one (embedded only). + pub target: Option, + /// Rows per record batch. `None` leaves DataFusion's default (8192), or + /// [`oxidelake_storage::GPU_BATCH_SIZE`] when the placement target is a GPU. + pub batch_size: Option, +} + +impl SessionOptions { + /// Defaults: detected backend, default batch size. + pub fn new() -> Self { + Self::default() + } + + /// Plans for `target` rather than the detected backend. + pub fn with_target(mut self, target: BackendKind) -> Self { + self.target = Some(target); + self + } + + /// Sets the rows per record batch, overriding the CPU and GPU defaults. + pub fn with_batch_size(mut self, rows: usize) -> Self { + self.batch_size = Some(rows); + self + } + + /// Applies the batch size to `config`, or the GPU default when the + /// placement target is a device and no size was asked for. + /// + /// A batch size of zero is rejected here rather than passed on: DataFusion + /// would take it and then produce no rows at all, which reads as an empty + /// result rather than as a bad flag. + fn apply( + &self, + config: SessionConfig, + target: Option, + ) -> Result { + match self.batch_size { + Some(0) => Err(EngineError::plan( + "batch size must be at least 1 row (0 would make every query return nothing)", + )), + Some(rows) => Ok(config.with_batch_size(rows)), + None if target.is_some_and(BackendKind::is_gpu) => Ok(with_gpu_batch_size(config)), + None => Ok(config), + } + } +} + /// An OxideLake session. pub struct OxideSession { ctx: SessionContext, @@ -47,7 +103,7 @@ impl OxideSession { /// registered, the SQL UDFs, and the placement rule targeting the detected /// backend. pub fn local() -> Result { - Self::local_with_target(local_backend()?.kind()) + Self::local_with_options(&SessionOptions::new()) } /// An embedded session whose placement rule targets `target` instead of @@ -56,12 +112,18 @@ impl OxideSession { /// CPU reference, so planning for an absent GPU is safe — it is exactly /// what cluster executors do with the scheduler's plans. pub fn local_with_target(target: BackendKind) -> Result { + Self::local_with_options(&SessionOptions::new().with_target(target)) + } + + /// An embedded session built with `options`. + pub fn local_with_options(options: &SessionOptions) -> Result { + let target = match options.target { + Some(target) => target, + None => local_backend()?.kind(), + }; let telemetry = TelemetryHub::new(); let rule = HardwarePlacementRule::new(target).with_telemetry(Arc::clone(&telemetry)); - let mut config = with_pruning(SessionConfig::new()); - if target.is_gpu() { - config = with_gpu_batch_size(config); - } + let config = options.apply(with_pruning(SessionConfig::new()), Some(target))?; let state = SessionStateBuilder::new() .with_default_features() .with_config(config) @@ -84,8 +146,19 @@ impl OxideSession { /// and the SQL UDFs so queries plan client-side; placement itself happens /// on the scheduler. pub async fn connect(scheduler_url: &str) -> Result { + Self::connect_with_options(scheduler_url, &SessionOptions::new()).await + } + + /// Connects to a Ballista scheduler with `options`. `target` is ignored: + /// on a cluster the scheduler's `OXIDE_CLUSTER_BACKEND` decides placement, + /// so a client-side target would be a knob that quietly does nothing. + pub async fn connect_with_options( + scheduler_url: &str, + options: &SessionOptions, + ) -> Result { let config = with_pruning(SessionConfig::new_with_ballista()) .with_ballista_physical_extension_codec(cluster::oxide_codec()); + let config = options.apply(config, None)?; let state = SessionStateBuilder::new() .with_default_features() .with_config(config) @@ -167,4 +240,56 @@ mod tests { let text = session.explain("SELECT 1 + 1 AS two").await.unwrap(); assert!(text.contains("ProjectionExec"), "{text}"); } + + fn batch_size(session: &OxideSession) -> usize { + session.ctx().state().config().batch_size() + } + + /// `--batch-size` has to reach DataFusion's configuration: a flag that is + /// parsed and then dropped would pass every test that only compares query + /// results, because batch boundaries do not change them. + #[test] + fn the_batch_size_option_reaches_the_session_configuration() { + let session = + OxideSession::local_with_options(&SessionOptions::new().with_batch_size(7)).unwrap(); + assert_eq!(batch_size(&session), 7); + } + + /// Without an explicit size, a GPU-targeted session keeps the larger + /// default that amortises the host↔device round trip, and a CPU one keeps + /// DataFusion's. An explicit size overrides both. + #[test] + fn the_target_picks_the_default_batch_size_and_the_option_overrides_it() { + let gpu = OxideSession::local_with_target(BackendKind::Cuda).unwrap(); + assert_eq!(batch_size(&gpu), oxidelake_storage::GPU_BATCH_SIZE); + + let cpu = OxideSession::local_with_target(BackendKind::CpuSimd).unwrap(); + assert_eq!(batch_size(&cpu), SessionConfig::new().batch_size()); + + let forced = OxideSession::local_with_options( + &SessionOptions::new() + .with_target(BackendKind::Cuda) + .with_batch_size(1_024), + ) + .unwrap(); + assert_eq!(batch_size(&forced), 1_024); + } + + #[test] + fn a_zero_batch_size_is_refused() { + let err = OxideSession::local_with_options(&SessionOptions::new().with_batch_size(0)) + .unwrap_err() + .to_string(); + assert!(err.contains("at least 1 row"), "{err}"); + } + + /// The no-argument constructors and `SessionOptions::default()` are the + /// same session, so the builder cannot drift away from the plain one. + #[test] + fn the_default_options_are_the_plain_constructor() { + let plain = OxideSession::local().unwrap(); + let built = OxideSession::local_with_options(&SessionOptions::default()).unwrap(); + assert_eq!(plain.mode(), built.mode()); + assert_eq!(batch_size(&plain), batch_size(&built)); + } } diff --git a/crates/oxidelake-runtime/tests/cli.rs b/crates/oxidelake-runtime/tests/cli.rs index d464af1..890cbd7 100644 --- a/crates/oxidelake-runtime/tests/cli.rs +++ b/crates/oxidelake-runtime/tests/cli.rs @@ -376,3 +376,192 @@ fn explain_into_a_closed_pipe_exits_zero_without_panicking() { assert!(!stderr.contains("panicked"), "{stderr}"); assert_eq!(out.status.code(), Some(0), "stderr:\n{stderr}"); } + +// --- The operational flags (#49): --batch-size, --output, --backend --------- + +/// `k -> n` parsed out of the CSV form, with the header checked. +fn parse_csv(stdout: &str) -> BTreeMap, i64> { + let mut lines = stdout.lines(); + assert_eq!(lines.next(), Some("k,n"), "csv header\n{stdout}"); + lines + .map(|line| { + let (k, n) = line.split_once(',').unwrap_or_else(|| panic!("{line:?}")); + let key = (!k.is_empty()).then(|| k.parse::().unwrap()); + (key, n.parse::().unwrap()) + }) + .collect() +} + +/// `k -> n` parsed out of the JSON form. A null `k` is *absent* from the +/// object rather than `null`, which is arrow-json's encoding and is why this +/// reads the key rather than expecting it. +fn parse_json(stdout: &str) -> BTreeMap, i64> { + let trimmed = stdout.trim(); + let inner = trimmed + .strip_prefix('[') + .and_then(|s| s.strip_suffix(']')) + .unwrap_or_else(|| panic!("not a JSON array: {trimmed}")); + if inner.is_empty() { + return BTreeMap::new(); + } + inner + .split("},{") + .map(|object| { + let object = object.trim_matches(['{', '}']); + let mut row = (None, None); + for field in object.split(',') { + let (name, value) = field.split_once(':').unwrap(); + match name.trim_matches('"') { + "k" => row.0 = Some(value.parse::().unwrap()), + "n" => row.1 = Some(value.parse::().unwrap()), + other => panic!("unexpected field {other}"), + } + } + (row.0, row.1.expect("every row has a count")) + }) + .collect() +} + +const COUNT_QUERY: &str = "SELECT k, COUNT(v) AS n FROM t GROUP BY k ORDER BY k"; + +/// The format is a rendering choice made after execution, so all three must +/// carry the same rows — and `--batch-size` must not change them either. Both +/// claims are in the flags' own `--help` text; this is what makes them true. +#[test] +fn output_formats_and_batch_sizes_agree_on_the_same_rows() { + let dir = tempfile::tempdir().unwrap(); + gen_data(dir.path(), 20_000); + let table_arg = format!("t={}", dir.path().display()); + + let run = |args: &[&str]| { + let assert = oxide() + .args(["sql", "-q", COUNT_QUERY, "--table", &table_arg]) + .args(args) + .assert() + .success(); + String::from_utf8(assert.get_output().stdout.clone()).unwrap() + }; + + let csv = parse_csv(&run(&["--output", "csv"])); + assert!(!csv.is_empty()); + assert_eq!(parse_json(&run(&["--output", "json"])), csv); + + // The table form is the default, and naming it explicitly changes nothing. + let table = run(&[]); + assert_eq!(run(&["--output", "table"]), table); + assert!(table.starts_with('+'), "{table}"); + + // A batch size far below and far above the row count: same answer. + for rows in ["1", "7", "1000000"] { + assert_eq!( + parse_csv(&run(&["--output", "csv", "--batch-size", rows])), + csv, + "--batch-size {rows}" + ); + } +} + +/// An empty result still says what the columns were, so a script can tell +/// "no rows" from "the query failed". +#[test] +fn an_empty_result_keeps_its_csv_header_and_json_array() { + let dir = tempfile::tempdir().unwrap(); + gen_data(dir.path(), 4_096); + let table_arg = format!("t={}", dir.path().display()); + let run = |format: &str| { + let assert = oxide() + .args([ + "sql", + "-q", + "SELECT k, COUNT(v) AS n FROM t WHERE k = -1 GROUP BY k", + "--table", + &table_arg, + "--output", + format, + ]) + .assert() + .success(); + String::from_utf8(assert.get_output().stdout.clone()).unwrap() + }; + assert_eq!(run("csv"), "k,n\n"); + assert_eq!(run("json"), "[]\n"); +} + +/// Zero rows per batch would be accepted by DataFusion and then return +/// nothing, which reads as an empty result rather than as a bad flag. +#[test] +fn a_zero_batch_size_is_refused_rather_than_returning_nothing() { + let assert = oxide() + .args(["sql", "-q", "SELECT 1", "--batch-size", "0"]) + .assert() + .failure(); + let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); + assert!(stderr.contains("at least 1 row"), "{stderr}"); +} + +#[test] +fn an_unknown_output_format_names_the_ones_that_exist() { + let assert = oxide() + .args(["sql", "-q", "SELECT 1", "--output", "yaml"]) + .assert() + .failure(); + let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); + assert!(stderr.contains("table, json, csv"), "{stderr}"); +} + +/// The batch-size and output flags are documented knobs, so they have to be +/// discoverable from `--help` — that is where the README table points. +#[test] +fn the_flags_appear_in_help() { + let assert = oxide().args(["sql", "--help"]).assert().success(); + let help = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + for flag in ["--batch-size", "--output", "--target", "--cluster"] { + assert!(help.contains(flag), "oxide sql --help lacks {flag}\n{help}"); + } + + let assert = Command::cargo_bin("oxide-worker") + .unwrap() + .arg("--help") + .assert() + .success(); + let help = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + assert!(help.contains("--backend"), "{help}"); + assert!(help.contains("OXIDE_BACKEND"), "{help}"); +} + +/// A worker asked for a backend this machine cannot provide must fail at +/// startup rather than join the cluster on the CPU: the scheduler would then +/// place GPU operators on a worker that has no GPU. +/// +/// Guarded by the real availability, because on a machine that *does* have the +/// device the worker would start and then block waiting for a scheduler. +#[test] +fn a_worker_refuses_a_backend_this_machine_does_not_have() { + let availability = oxidelake_device::HardwareDetector::availability(); + let absent = if !availability.cuda { + "cuda" + } else if !availability.metal { + "metal" + } else { + return; + }; + let assert = Command::cargo_bin("oxide-worker") + .unwrap() + .args(["--backend", absent]) + .assert() + .failure(); + let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); + assert!(stderr.contains(absent), "{stderr}"); + assert!(stderr.contains("explicitly requested"), "{stderr}"); +} + +#[test] +fn a_worker_rejects_an_unknown_backend_name() { + let assert = Command::cargo_bin("oxide-worker") + .unwrap() + .args(["--backend", "tpu"]) + .assert() + .failure(); + let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); + assert!(stderr.contains("cpu, cuda, metal"), "{stderr}"); +} From c5a2aa406cd06be519dcf7864093928f777e97dd Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:59:08 +0700 Subject: [PATCH 4/8] fix(predict)!: read the activation from the model file instead of assuming it Closes #47 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- README.md | 18 +- crates/oxidelake-compute/src/lib.rs | 2 +- crates/oxidelake-compute/src/predict.rs | 318 +++++++++++++--- crates/oxidelake-compute/tests/predict.rs | 360 ++++++++++++++++-- docs/RELEASING.md | 2 +- ...-in-database-inference-at-the-udf-layer.md | 37 +- 6 files changed, 650 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index 3ada8d4..610e42d 100644 --- a/README.md +++ b/README.md @@ -70,9 +70,14 @@ pulls in a tensor library. ```bash cargo build --release -p oxidelake-runtime --features predict -# an oxmera Sequential/Linear model saved with safetensors +# an oxmera Sequential/Linear model saved with safetensors, whose header +# declares {"__metadata__": {"oxidelake.activation": "relu"}} ./target/release/oxide sql --table t=data/ \ -q "SELECT id, predict('scorer.safetensors', emb) AS logits FROM t LIMIT 5" + +# a checkpoint whose header says nothing: state the activation in the query +./target/release/oxide sql --table t=data/ \ + -q "SELECT id, predict('scorer.safetensors', emb, 'gelu') AS logits FROM t LIMIT 5" ``` ``` @@ -97,8 +102,15 @@ could not score them. The model file is read for its architecture, not just its weights: `predict` rebuilds an `oxmera::nn::Sequential` of `Linear` layers from the `0.weight`, -`1.weight`, … naming, with ReLU between them. That assumption, the CPU-only -execution, and why this is a UDF rather than an operator are recorded in +`1.weight`, … naming, and takes the activation between them from the +safetensors `__metadata__` key `oxidelake.activation` — `relu`, `gelu`, +`sigmoid`, `tanh` or `none`, applied between layers and never after the last. +A file that declares nothing is **refused**, because the alternative is +scoring it with an activation it was not trained with and returning a +confident wrong number; the third argument `predict(path, features, 'relu')` +is how you say it yourself, and it must agree with the file when the file has +an opinion. The CPU-only execution, and why this is a UDF rather than an +operator, are recorded in [ADR-0015](docs/decisions/ADR-0015-in-database-inference-at-the-udf-layer.md). `--target cpu|cuda|metal` picks the *placement* target; execution always uses the hardware that is present (operators planned for an absent GPU take the per-batch CPU path — exactly what cluster executors do with a scheduler's plan). diff --git a/crates/oxidelake-compute/src/lib.rs b/crates/oxidelake-compute/src/lib.rs index 86a7cb2..6502311 100644 --- a/crates/oxidelake-compute/src/lib.rs +++ b/crates/oxidelake-compute/src/lib.rs @@ -51,7 +51,7 @@ pub use oxidelake_core::params::{ #[cfg(feature = "predict")] pub mod predict; #[cfg(feature = "predict")] -pub use predict::{Model, ModelSpec, PREDICT, predict_udf}; +pub use predict::{ACTIVATION_KEY, Activation, Model, ModelSpec, PREDICT, predict_udf}; pub use udf::{ COSINE_DISTANCE, L2_DISTANCE, cosine_distance_udf, distance_metric_for, distance_udf_name, diff --git a/crates/oxidelake-compute/src/predict.rs b/crates/oxidelake-compute/src/predict.rs index e57ee4a..8d8be08 100644 --- a/crates/oxidelake-compute/src/predict.rs +++ b/crates/oxidelake-compute/src/predict.rs @@ -29,11 +29,25 @@ //! `0.bias`, `1.weight`, … — and rebuilds a stack of `Linear` layers from the //! shapes. `N.weight` of shape `[out, in]` is layer `N`. //! -//! **The activation is the prototype's one real assumption**: ReLU between -//! layers, nothing after the last. That covers an MLP scorer and nothing else, -//! and a model whose activations differ will silently produce wrong numbers — -//! which is why [`ModelSpec`] exists as the place to put an explicit -//! description when this graduates. See the module tests for what is pinned. +//! **The activation comes from the file, never from a guess** (#47). A +//! `safetensors` header can carry free-form metadata, so a model declares its +//! own activation there: +//! +//! ```json +//! {"__metadata__": {"oxidelake.activation": "relu"}} +//! ``` +//! +//! and a file without that key is refused rather than assumed. The activation +//! is applied *between* the `Linear` layers and never after the last, so the +//! output stays logits. A query may state the activation itself — +//! `predict(path, features, 'relu')` — which is how a file someone else wrote +//! is used; when the file also declares one, the two must agree. +//! +//! This is the difference between an honest error and a confident wrong +//! number: until 0.2.0 the loader built a stack of bare `Linear` layers and +//! documented ReLU between them, so every non-linear model it was pointed at +//! returned the answer of a linear one. See the module tests for what is +//! pinned. //! //! ## Cost model //! @@ -55,24 +69,118 @@ use datafusion::logical_expr::{ ColumnarValue, ScalarFunctionArgs, ScalarUDF, ScalarUDFImpl, Signature, Volatility, }; use oxmera::Tensor; -use oxmera::nn::{Linear, Module, Sequential}; +use oxmera::nn::{Linear, Sequential}; use safetensors::SafeTensors; /// Name of the inference UDF. pub const PREDICT: &str = "predict"; +/// The safetensors `__metadata__` key a model declares its activation under. +pub const ACTIVATION_KEY: &str = "oxidelake.activation"; + +/// The activation applied between a model's `Linear` layers. +/// +/// Never after the last one: the output of `predict` is logits, which is what +/// a caller composes with `[1]`, a threshold or a sort. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum Activation { + /// `max(0, x)`. + Relu, + /// The Gaussian error linear unit. + Gelu, + /// The logistic function. + Sigmoid, + /// Hyperbolic tangent. + Tanh, + /// No activation: the stack is one affine map. Spelled out rather than + /// left to a missing key, so a linear model is a statement and not an + /// omission. + None, +} + +impl Activation { + /// Every activation, in the order the error messages list them. + pub const ALL: [Activation; 5] = [ + Activation::Relu, + Activation::Gelu, + Activation::Sigmoid, + Activation::Tanh, + Activation::None, + ]; + + /// The lowercase name used in `__metadata__` and in SQL. + pub const fn as_str(self) -> &'static str { + match self { + Activation::Relu => "relu", + Activation::Gelu => "gelu", + Activation::Sigmoid => "sigmoid", + Activation::Tanh => "tanh", + Activation::None => "none", + } + } + + /// The names this accepts, for an error that tells the reader what to write. + fn names() -> String { + Self::ALL + .iter() + .map(|a| a.as_str()) + .collect::>() + .join(", ") + } + + fn parse(name: &str) -> Result { + Self::ALL + .into_iter() + .find(|a| a.as_str() == name.trim().to_ascii_lowercase()) + .ok_or_else(|| { + exec(format!( + "{PREDICT}: unknown activation '{name}'; expected one of {}", + Self::names() + )) + }) + } + + fn apply(self, x: &Tensor) -> oxmera::Result { + match self { + Activation::Relu => x.relu(), + Activation::Gelu => x.gelu(), + Activation::Sigmoid => x.sigmoid(), + Activation::Tanh => x.tanh(), + Activation::None => Ok(x.clone()), + } + } +} + +impl std::fmt::Display for Activation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + /// How a model file is interpreted. /// -/// One variant today. It is an enum rather than a bare `load_mlp` so that the -/// activation assumption documented above has somewhere to go when a second -/// shape is needed: the model path stays the SQL surface, and what changes is -/// this description of it. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +/// One shape today. It is an enum rather than a bare `load_mlp` so that a +/// second architecture has somewhere to go: the model path stays the SQL +/// surface, and what changes is this description of it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[non_exhaustive] pub enum ModelSpec { /// A stack of `Linear` layers named `0.weight`/`0.bias`, `1.weight`/…, - /// ReLU between them and nothing after the last. - SequentialMlpRelu, + /// with an activation between them and nothing after the last. + SequentialMlp { + /// The activation the caller asserts. `None` means "take it from the + /// file's `__metadata__`", which is the only way that does not guess. + /// When both are present they must agree — a query that states the + /// wrong activation for a file is a mistake worth an error, not a + /// silent override of what the model was trained with. + activation: Option, + }, +} + +impl ModelSpec { + /// The spec that reads everything, including the activation, from the file. + pub const FROM_FILE: ModelSpec = ModelSpec::SequentialMlp { activation: None }; } /// A loaded model and the shapes it accepts. @@ -86,12 +194,29 @@ pub struct Model { pub in_features: usize, /// Rows the last layer's weight produces. pub out_features: usize, + /// The activation between layers, as the file (or the query) declared it. + pub activation: Activation, } impl Model { /// Run `rows x in_features` through the stack. + /// + /// The layers are stepped by hand rather than through + /// `Sequential::forward` so the activation lands *between* them and not + /// after the last. The activation is not a child of the `Sequential` + /// either: children are numbered `0.`, `1.`, … in `named_parameters`, so + /// inserting a parameterless module between the layers would renumber + /// every weight and stop the file loading at all. fn forward(&self, input: &Tensor) -> oxmera::Result { - self.module.forward(input) + let last = self.module.len().saturating_sub(1); + let mut x = input.clone(); + for (i, layer) in self.module.iter().enumerate() { + x = layer.forward(&x)?; + if i != last { + x = self.activation.apply(&x)?; + } + } + Ok(x) } } @@ -104,9 +229,16 @@ impl Model { /// process keeps serving the old weights, which is the right default for a /// query engine (a plan should not change answers halfway) and the wrong one /// for a notebook, so it is stated rather than assumed. -static MODELS: OnceLock>>> = OnceLock::new(); +/// +/// Keyed by the path *and* the spec: two queries over the same file, one +/// asserting an activation and one not, must both be checked against the +/// header rather than the second silently inheriting the first's answer. +static MODELS: OnceLock> = OnceLock::new(); + +/// The cache's contents: a loaded model per path and asserted activation. +type ModelCache = HashMap<(String, ModelSpec), Arc>; -fn cache() -> &'static Mutex>> { +fn cache() -> &'static Mutex { MODELS.get_or_init(|| Mutex::new(HashMap::new())) } @@ -120,18 +252,21 @@ pub fn model(path: &str, spec: ModelSpec) -> Result> { let mut guard = cache() .lock() .map_err(|_| DataFusionError::Execution(format!("{PREDICT}: model cache poisoned")))?; - if let Some(found) = guard.get(path) { + let key = (path.to_string(), spec); + if let Some(found) = guard.get(&key) { return Ok(Arc::clone(found)); } let loaded = Arc::new(load(path, spec)?); - guard.insert(path.to_string(), Arc::clone(&loaded)); + guard.insert(key, Arc::clone(&loaded)); Ok(loaded) } /// Rebuild the architecture from the tensor names and shapes, then fill it. fn load(path: &str, spec: ModelSpec) -> Result { - let ModelSpec::SequentialMlpRelu = spec; - let shapes = layer_shapes(path)?; + let ModelSpec::SequentialMlp { activation } = spec; + let bytes = std::fs::read(path).map_err(|e| exec(format!("{PREDICT}: reading {path}: {e}")))?; + let activation = resolve_activation(path, &bytes, activation)?; + let shapes = layer_shapes(path, &bytes)?; // Destructured together so the "not empty" check happens once and the // compiler carries it, rather than a first()? followed by a last() that // is unreachable-but-fallible. @@ -158,9 +293,50 @@ fn load(path: &str, spec: ModelSpec) -> Result { module: stack, in_features: first_in, out_features: last_out, + activation, }) } +/// The activation to use, from the file's `__metadata__`, the query, or both. +/// +/// A file that declares nothing and a query that asserts nothing is the one +/// case with no answer, and it is an error. Guessing there is what made every +/// non-linear model load and return a linear model's numbers before 0.2.0, so +/// the message names the key and both ways of supplying it. +fn resolve_activation( + path: &str, + bytes: &[u8], + asserted: Option, +) -> Result { + let declared = declared_activation(path, bytes)?; + match (declared, asserted) { + (Some(declared), Some(asserted)) if declared != asserted => Err(exec(format!( + "{PREDICT}: {path} declares `{ACTIVATION_KEY}: {declared}` but the query asks for '{asserted}'; the model was trained with one of them, so change the query or the file rather than running the other" + ))), + (Some(declared), _) => Ok(declared), + (None, Some(asserted)) => Ok(asserted), + (None, None) => Err(exec(format!( + "{PREDICT}: {path} does not declare `{ACTIVATION_KEY}` in its safetensors `__metadata__`, so the activation between its Linear layers is unknown and assuming one would return confident wrong numbers. Either save the file with that key (`safetensors::serialize` takes a metadata map; `oxmera::nn::serialize::save` does not write one as of oxmera 0.5), or state it in the query: predict('{path}', features, 'relu'). Accepted values: {}", + Activation::names() + ))), + } +} + +/// The `oxidelake.activation` value in the file's header, if it has one. +fn declared_activation(path: &str, bytes: &[u8]) -> Result> { + let (_, metadata) = SafeTensors::read_metadata(bytes) + .map_err(|e| exec(format!("{PREDICT}: {path} is not a safetensors file: {e}")))?; + metadata + .metadata() + .as_ref() + .and_then(|entries| entries.get(ACTIVATION_KEY)) + .map(|name| { + Activation::parse(name) + .map_err(|e| exec(format!("{PREDICT}: {path}: `{ACTIVATION_KEY}`: {e}"))) + }) + .transpose() +} + /// `(in_features, out_features)` per layer, in layer order. /// /// Read from the file rather than configured: `N.weight` has shape @@ -168,9 +344,8 @@ fn load(path: &str, spec: ModelSpec) -> Result { /// Layers must be numbered `0..n` with no gaps — a gap means this is not the /// `Sequential` layout, and guessing at that point would build a model that /// loads and computes something else. -fn layer_shapes(path: &str) -> Result> { - let bytes = std::fs::read(path).map_err(|e| exec(format!("{PREDICT}: reading {path}: {e}")))?; - let file = SafeTensors::deserialize(&bytes) +fn layer_shapes(path: &str, bytes: &[u8]) -> Result> { + let file = SafeTensors::deserialize(bytes) .map_err(|e| exec(format!("{PREDICT}: {path} is not a safetensors file: {e}")))?; let mut by_index: HashMap = HashMap::new(); @@ -244,22 +419,33 @@ impl ScalarUDFImpl for PredictUdf { &self.signature } - /// `predict(model_path, features)`: a Utf8 literal and a list of numbers. + /// `predict(model_path, features)` or `predict(model_path, features, + /// activation)`: a Utf8 literal, a list of numbers, and optionally the + /// activation as a string. /// /// The features are coerced to `FixedSizeList` exactly as the /// distance UDFs do, so a column written as `List` still works /// and the dimension is known before execution. fn coerce_types(&self, arg_types: &[DataType]) -> Result> { - let [path, features] = arg_types else { + let (path, features, activation) = match arg_types { + [path, features] => (path, features, None), + [path, features, activation] => (path, features, Some(activation)), + other => { + return Err(DataFusionError::Plan(format!( + "{PREDICT} takes 2 or 3 arguments (model path, features [, activation]), got {}", + other.len() + ))); + } + }; + if let Some(activation) = activation + && !is_string(activation) + { return Err(DataFusionError::Plan(format!( - "{PREDICT} takes 2 arguments (model path, features), got {}", - arg_types.len() + "{PREDICT}: the third argument is the activation as a string ({}), got {activation:?}", + Activation::names() ))); - }; - if !matches!( - path, - DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View - ) { + } + if !is_string(path) { return Err(DataFusionError::Plan(format!( "{PREDICT}: the first argument is the model path as a string, got {path:?}" ))); @@ -273,10 +459,14 @@ impl ScalarUDFImpl for PredictUdf { ))); } }; - Ok(vec![ + let mut coerced = vec![ DataType::Utf8, DataType::FixedSizeList(Arc::new(Field::new("item", DataType::Float32, true)), dim), - ]) + ]; + if activation.is_some() { + coerced.push(DataType::Utf8); + } + Ok(coerced) } /// A vector out, one element per output feature — the model's width is @@ -291,14 +481,20 @@ impl ScalarUDFImpl for PredictUdf { } fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result { - let [path, features] = args.args.as_slice() else { - return Err(exec(format!( - "{PREDICT} takes 2 arguments, got {}", - args.args.len() - ))); + let (path, features, activation) = match args.args.as_slice() { + [path, features] => (path, features, None), + [path, features, activation] => (path, features, Some(activation)), + other => { + return Err(exec(format!( + "{PREDICT} takes 2 or 3 arguments, got {}", + other.len() + ))); + } }; let path = model_path(path)?; - let model = model(&path, ModelSpec::SequentialMlpRelu)?; + let activation = activation.map(activation_argument).transpose()?; + let activation = activation.as_deref().map(Activation::parse).transpose()?; + let model = model(&path, ModelSpec::SequentialMlp { activation })?; let list = feature_list(features)?; let (values, dim) = feature_values(&list)?; if dim != model.in_features { @@ -348,21 +544,45 @@ impl ScalarUDFImpl for PredictUdf { } } -fn model_path(value: &ColumnarValue) -> Result { +fn is_string(data_type: &DataType) -> bool { + matches!( + data_type, + DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View + ) +} + +/// A literal string argument, or `None` when it is not one. +fn constant_string(value: &ColumnarValue) -> Option { match value { ColumnarValue::Scalar(ScalarValue::Utf8(Some(s))) | ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(s))) - | ColumnarValue::Scalar(ScalarValue::Utf8View(Some(s))) => Ok(s.clone()), - // Deliberately refused rather than supported. A per-row model path - // means a per-row model load, and the cache would grow without bound - // on a column of distinct paths — a query that quietly consumes the - // machine is worse than one that will not plan. - _ => Err(DataFusionError::Plan(format!( - "{PREDICT}: the model path must be a constant string, not a column" - ))), + | ColumnarValue::Scalar(ScalarValue::Utf8View(Some(s))) => Some(s.clone()), + _ => None, } } +fn model_path(value: &ColumnarValue) -> Result { + // Deliberately refused rather than supported. A per-row model path means a + // per-row model load, and the cache would grow without bound on a column + // of distinct paths — a query that quietly consumes the machine is worse + // than one that will not plan. + constant_string(value).ok_or_else(|| { + DataFusionError::Plan(format!( + "{PREDICT}: the model path must be a constant string, not a column" + )) + }) +} + +/// The activation argument, which must be a literal for the same reason the +/// path must: it selects which model is built and cached. +fn activation_argument(value: &ColumnarValue) -> Result { + constant_string(value).ok_or_else(|| { + DataFusionError::Plan(format!( + "{PREDICT}: the activation must be a constant string, not a column" + )) + }) +} + fn feature_list(value: &ColumnarValue) -> Result { match value { ColumnarValue::Array(array) => array diff --git a/crates/oxidelake-compute/tests/predict.rs b/crates/oxidelake-compute/tests/predict.rs index d95b2de..3b273d7 100644 --- a/crates/oxidelake-compute/tests/predict.rs +++ b/crates/oxidelake-compute/tests/predict.rs @@ -5,35 +5,124 @@ //! The reference is the point. A UDF that computes *something* plausible is //! the failure mode here — the architecture is inferred from tensor names, so //! a wrong inference produces confident wrong numbers rather than an error. +//! That is not hypothetical: before 0.2.0 the loader built bare `Linear` +//! layers while every document said "ReLU between them", and the only test of +//! the numbers compared them against the same bare stack. So the activation +//! is now read from the file's `__metadata__` (#47), and these tests check it +//! against a reference that does *not* share the loader's loop: the `none` +//! case is compared with oxmera's own `Sequential::forward`, and `relu` and +//! `gelu` must disagree with it and with each other. #![cfg(feature = "predict")] // The house convention for test files (see conformance.rs and siblings): a // failed setup should be a loud panic with a reason, not a `?` that turns // into an opaque test-harness error. #![allow(clippy::unwrap_used, clippy::expect_used)] +use std::collections::HashMap; +use std::path::Path; use std::sync::Arc; use arrow::array::{Array, FixedSizeListArray, Float32Array, Int32Array, ListArray, RecordBatch}; use arrow::datatypes::{DataType, Field, Schema}; use datafusion::prelude::SessionContext; -use oxidelake_compute::predict_udf; +use oxidelake_compute::{ACTIVATION_KEY, predict_udf}; use oxmera::Tensor; use oxmera::nn::{Linear, Module, Sequential}; +use safetensors::SafeTensors; const IN: usize = 4; const HIDDEN: usize = 3; const OUT: usize = 2; -/// A two-layer MLP with fixed seeds, saved where the test can point SQL at it. -fn model_on_disk(dir: &std::path::Path) -> (Sequential, String) { - let model = Sequential::new() +fn two_layer_mlp() -> Sequential { + Sequential::new() .push(Linear::new(IN, HIDDEN, 11)) - .push(Linear::new(HIDDEN, OUT, 22)); - let path = dir.join("scorer.safetensors"); - oxmera::nn::serialize::save(&model, &path).expect("save"); + .push(Linear::new(HIDDEN, OUT, 22)) +} + +/// Saves `model` at `path`, adding `oxidelake.activation` to the safetensors +/// `__metadata__` when `activation` is given. +/// +/// Written here rather than taken from a helper because `oxmera`'s +/// `nn::serialize::save` writes no metadata (0.5): the file is saved, read +/// back, and re-serialized with the header this engine asks for. That is +/// exactly what a user of a third-party checkpoint has to do, so the test +/// doing it is the documentation of it working. +fn save_with_activation(model: &Sequential, path: &Path, activation: Option<&str>) { + oxmera::nn::serialize::save(model, path).expect("save"); + let Some(activation) = activation else { return }; + let bytes = std::fs::read(path).expect("read back"); + let file = SafeTensors::deserialize(&bytes).expect("deserialize"); + let tensors: Vec<_> = file.tensors(); + let metadata = HashMap::from([(ACTIVATION_KEY.to_owned(), activation.to_owned())]); + let out = safetensors::serialize( + tensors.iter().map(|(name, view)| (name.clone(), view)), + Some(metadata), + ) + .expect("serialize with metadata"); + std::fs::write(path, out).expect("write"); +} + +/// A two-layer MLP with fixed seeds, declaring `activation`, saved where the +/// test can point SQL at it. +fn model_on_disk_with(dir: &Path, name: &str, activation: Option<&str>) -> (Sequential, String) { + let model = two_layer_mlp(); + let path = dir.join(name); + save_with_activation(&model, &path, activation); (model, path.to_string_lossy().into_owned()) } +/// The default fixture: a ReLU model. +fn model_on_disk(dir: &Path) -> (Sequential, String) { + model_on_disk_with(dir, "scorer.safetensors", Some("relu")) +} + +/// The expected output for one row, computed by stepping the layers and +/// applying `activation` between them — never after the last. +fn reference(model: &Sequential, input: &[f32; IN], activation: &str) -> Vec { + oxmera::no_grad(|| -> oxmera::Result { + let mut x = Tensor::from_vec_f32(input.to_vec(), [1, IN])?; + let last = model.len() - 1; + for (i, layer) in model.iter().enumerate() { + x = layer.forward(&x)?; + if i != last { + x = match activation { + "relu" => x.relu()?, + "gelu" => x.gelu()?, + "sigmoid" => x.sigmoid()?, + "tanh" => x.tanh()?, + "none" => x, + other => panic!("the test does not model {other}"), + }; + } + } + Ok(x) + }) + .expect("reference forward") + .to_vec_f32() + .expect("reference values") +} + +/// The `y` column of the first batch, as a list array. +fn outputs(out: &[RecordBatch], column: usize) -> ListArray { + out[0] + .column(column) + .as_any() + .downcast_ref::() + .expect("predict returns a list") + .clone() +} + +fn row_values(list: &ListArray, row: usize) -> Vec { + let value = list.value(row); + value + .as_any() + .downcast_ref::() + .expect("float output") + .values() + .to_vec() +} + fn features() -> Vec> { vec![ Some([1.0, 0.0, -2.0, 0.5]), @@ -102,12 +191,7 @@ async fn predict_agrees_with_the_model_it_loaded() { &path, ) .await; - let batch = &out[0]; - let y = batch - .column(1) - .as_any() - .downcast_ref::() - .expect("predict returns a list"); + let y = outputs(&out, 1); for (row, expected_in) in features().iter().enumerate() { let Some(input) = expected_in else { @@ -122,27 +206,85 @@ async fn predict_agrees_with_the_model_it_loaded() { "row {row}: a real feature vector must produce a value" ); - // The reference: the same model, called directly. + let want = reference(&model, input, "relu"); + let got = row_values(&y, row); + assert_eq!(got.len(), OUT, "row {row}: output width"); + for (i, expected) in want.iter().enumerate().take(OUT) { + assert!( + (got[i] - expected).abs() < 1e-5, + "row {row}, output {i}: SQL gave {} and the model gives {expected}", + got[i], + ); + } + } +} + +/// `none` is the plain affine stack, so oxmera's own `Sequential::forward` — +/// which knows nothing about this crate's loop — is the reference. +#[tokio::test] +async fn an_activation_of_none_is_oxmeras_own_sequential() { + let dir = tempfile::tempdir().expect("tempdir"); + let (model, path) = model_on_disk_with(dir.path(), "linear.safetensors", Some("none")); + + let out = run( + "SELECT predict('{model}', features) AS y FROM t ORDER BY id", + &path, + ) + .await; + let y = outputs(&out, 0); + + for (row, input) in features().iter().enumerate() { + let Some(input) = input else { continue }; let want = oxmera::no_grad(|| { model.forward(&Tensor::from_vec_f32(input.to_vec(), [1, IN]).unwrap()) }) .expect("reference forward") .to_vec_f32() .expect("reference values"); + for (i, expected) in want.iter().enumerate().take(OUT) { + assert!( + (row_values(&y, row)[i] - expected).abs() < 1e-5, + "row {row}, output {i}" + ); + } + } +} - let got = y.value(row); - let got = got - .as_any() - .downcast_ref::() - .expect("float output"); - assert_eq!(got.len(), OUT, "row {row}: output width"); +/// The same weights under three activations must give three answers. This is +/// what a stubbed or ignored activation fails: it would make all three equal. +#[tokio::test] +async fn the_declared_activation_changes_the_numbers() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut answers = Vec::new(); + for activation in ["relu", "gelu", "none"] { + let (model, path) = model_on_disk_with( + dir.path(), + &format!("{activation}.safetensors"), + Some(activation), + ); + let out = run( + "SELECT predict('{model}', features) AS y FROM t ORDER BY id", + &path, + ) + .await; + let got = row_values(&outputs(&out, 0), 3); + let want = reference( + &model, + &features()[3].expect("row 3 is not null"), + activation, + ); for (i, expected) in want.iter().enumerate().take(OUT) { assert!( - (got.value(i) - expected).abs() < 1e-5, - "row {row}, output {i}: SQL gave {} and the model gives {expected}", - got.value(i), + (got[i] - expected).abs() < 1e-5, + "{activation}, output {i}: SQL gave {} and the model gives {expected}", + got[i] ); } + answers.push((activation, got)); + } + for pair in answers.windows(2) { + let [(a, x), (b, y)] = pair else { continue }; + assert_ne!(x, y, "{a} and {b} produced identical output"); } } @@ -210,7 +352,7 @@ async fn a_feature_width_the_model_does_not_accept_is_an_error() { // A model that wants 7 features, against the 4-wide column. let wrong = Sequential::new().push(Linear::new(7, OUT, 1)); let path = dir.path().join("wrong.safetensors"); - oxmera::nn::serialize::save(&wrong, &path).unwrap(); + save_with_activation(&wrong, &path, Some("relu")); let ctx = SessionContext::new(); ctx.register_udf(predict_udf()); @@ -249,3 +391,173 @@ async fn a_column_of_model_paths_is_refused() { }; assert!(err.contains("constant string"), "{err}"); } + +// --- the activation contract (#47) ----------------------------------------- + +/// A model that declares nothing is refused, and the message says both ways +/// of fixing it. The old behaviour — load it and apply no activation at all — +/// is the one outcome that must not happen. +#[tokio::test] +async fn a_model_without_the_activation_key_is_refused() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_, path) = model_on_disk_with(dir.path(), "bare.safetensors", None); + + let ctx = SessionContext::new(); + ctx.register_udf(predict_udf()); + ctx.register_batch("t", batch()).unwrap(); + let err = ctx + .sql(&format!("SELECT predict('{path}', features) FROM t")) + .await + .expect("plans: the file is only read at execution") + .collect() + .await + .expect_err("a header-less model must not run"); + let msg = err.to_string(); + assert!(msg.contains(ACTIVATION_KEY), "{msg}"); + assert!(msg.contains("__metadata__"), "{msg}"); + assert!(msg.contains("features, 'relu'"), "{msg}"); + assert!(msg.contains("relu, gelu, sigmoid, tanh, none"), "{msg}"); +} + +/// The third argument is how a checkpoint someone else wrote is used. +#[tokio::test] +async fn the_query_may_state_the_activation_the_file_does_not() { + let dir = tempfile::tempdir().expect("tempdir"); + let (model, path) = model_on_disk_with(dir.path(), "stated.safetensors", None); + + let out = run( + "SELECT predict('{model}', features, 'gelu') AS y FROM t ORDER BY id", + &path, + ) + .await; + let want = reference(&model, &features()[0].expect("row 0"), "gelu"); + let got = row_values(&outputs(&out, 0), 0); + for (i, expected) in want.iter().enumerate().take(OUT) { + assert!((got[i] - expected).abs() < 1e-5, "output {i}"); + } +} + +/// When both say something, they must say the same thing: silently letting +/// the query override the file would be the original bug with extra steps. +#[tokio::test] +async fn a_query_that_contradicts_the_file_is_refused() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_, path) = model_on_disk_with(dir.path(), "relu-model.safetensors", Some("relu")); + + let ctx = SessionContext::new(); + ctx.register_udf(predict_udf()); + ctx.register_batch("t", batch()).unwrap(); + let err = ctx + .sql(&format!( + "SELECT predict('{path}', features, 'gelu') FROM t" + )) + .await + .unwrap() + .collect() + .await + .expect_err("a contradiction must not run"); + let msg = err.to_string(); + assert!(msg.contains("relu"), "{msg}"); + assert!(msg.contains("gelu"), "{msg}"); +} + +/// Agreeing with the file is fine, and gives the file's answer. +#[tokio::test] +async fn a_query_that_repeats_the_file_is_accepted() { + let dir = tempfile::tempdir().expect("tempdir"); + let (model, path) = model_on_disk_with(dir.path(), "agree.safetensors", Some("relu")); + let out = run( + "SELECT predict('{model}', features, 'relu') AS y FROM t ORDER BY id", + &path, + ) + .await; + let want = reference(&model, &features()[0].expect("row 0"), "relu"); + let got = row_values(&outputs(&out, 0), 0); + for (i, expected) in want.iter().enumerate().take(OUT) { + assert!((got[i] - expected).abs() < 1e-5, "output {i}"); + } +} + +#[tokio::test] +async fn an_unknown_activation_names_the_ones_that_exist() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_, path) = model_on_disk_with(dir.path(), "unknown.safetensors", None); + + let ctx = SessionContext::new(); + ctx.register_udf(predict_udf()); + ctx.register_batch("t", batch()).unwrap(); + let err = ctx + .sql(&format!( + "SELECT predict('{path}', features, 'swish') FROM t" + )) + .await + .unwrap() + .collect() + .await + .expect_err("an unknown activation must not run"); + let msg = err.to_string(); + assert!(msg.contains("swish"), "{msg}"); + assert!(msg.contains("relu, gelu, sigmoid, tanh, none"), "{msg}"); +} + +/// A file whose header says something this engine does not know is refused +/// with the file named — the alternative is ignoring the key and computing +/// the wrong model, which is what the key exists to prevent. +#[tokio::test] +async fn an_unknown_activation_in_the_file_is_refused() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_, path) = model_on_disk_with(dir.path(), "swish.safetensors", Some("swish")); + + let ctx = SessionContext::new(); + ctx.register_udf(predict_udf()); + ctx.register_batch("t", batch()).unwrap(); + let err = ctx + .sql(&format!("SELECT predict('{path}', features) FROM t")) + .await + .unwrap() + .collect() + .await + .expect_err("an unknown declared activation must not run"); + let msg = err.to_string(); + assert!(msg.contains("swish.safetensors"), "{msg}"); + assert!(msg.contains(ACTIVATION_KEY), "{msg}"); +} + +#[tokio::test] +async fn a_column_of_activations_is_refused() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_, path) = model_on_disk_with(dir.path(), "col.safetensors", Some("relu")); + + let ctx = SessionContext::new(); + ctx.register_udf(predict_udf()); + ctx.register_batch("t", batch()).unwrap(); + let result = ctx + .sql(&format!( + "SELECT predict('{path}', features, CAST(id AS VARCHAR)) FROM t" + )) + .await; + let err = match result { + Err(e) => e.to_string(), + Ok(df) => df + .collect() + .await + .expect_err("a column of activations must not run") + .to_string(), + }; + assert!(err.contains("constant string"), "{err}"); +} + +#[tokio::test] +async fn four_arguments_are_refused_naming_the_shape() { + let ctx = SessionContext::new(); + ctx.register_udf(predict_udf()); + ctx.register_batch("t", batch()).unwrap(); + let result = ctx + .sql("SELECT predict('m', features, 'relu', 'extra') FROM t") + .await; + let err = match result { + Err(e) => e.to_string(), + Ok(df) => df.collect().await.expect_err("four arguments").to_string(), + }; + assert!(err.contains("2 or 3 arguments"), "{err}"); +} diff --git a/docs/RELEASING.md b/docs/RELEASING.md index ba5e948..d431392 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -110,7 +110,7 @@ the break is in a type we re-export. It is the release manager's to declare. Public enums are `#[non_exhaustive]` where a new variant is a thing the world gains — `BackendKind`, `Compression`, `SessionMode`, `MemoryClass`, -`MemoryTier`, `Tier`, `ModelSpec`, and the TUI's `Panel`/`KeyInput`/ +`MemoryTier`, `Tier`, `ModelSpec`, `Activation`, and the TUI's `Panel`/`KeyInput`/ `Transition`. Adding to those is then a minor bump rather than a break, and a consumer's `match` needs a `_` arm. diff --git a/docs/decisions/ADR-0015-in-database-inference-at-the-udf-layer.md b/docs/decisions/ADR-0015-in-database-inference-at-the-udf-layer.md index 90450c4..ff0b1d4 100644 --- a/docs/decisions/ADR-0015-in-database-inference-at-the-udf-layer.md +++ b/docs/decisions/ADR-0015-in-database-inference-at-the-udf-layer.md @@ -1,6 +1,6 @@ # ADR-0015: In-database inference belongs at the UDF layer, not the operator layer -- Status: accepted (prototype) +- Status: accepted (prototype); the activation section revised 2026-09-21 for 0.2.0 (#47) - Date: 2026-09-06 ## Context @@ -63,12 +63,26 @@ convention `oxmera::nn::Sequential` already writes — `0.weight`, `0.bias`, shape `[out, in]` is layer `N`. Layers must be numbered `0..n` with no gaps and consecutive widths must compose, or the file is refused. -**The activation is the one real assumption**: ReLU between layers, nothing -after the last. That covers an MLP scorer and nothing else, and a model whose -activations differ will produce confidently wrong numbers rather than an -error. `ModelSpec` exists as the enum where an explicit description goes when -a second shape is needed; today it has one variant and the assumption is -documented at the call site. +**The activation is read too, and never assumed** (revised for 0.2.0, #47). +A safetensors header carries free-form metadata, so a model declares its own +activation there under `oxidelake.activation` (`relu`, `gelu`, `sigmoid`, +`tanh` or `none`), applied between the `Linear` layers and never after the +last. A query may state it instead — `predict(path, features, 'relu')` — +which is how a checkpoint someone else wrote is used; when the file also +declares one, the two must agree or the query is refused. + +The original decision here was to assume ReLU and document the assumption. +That was wrong twice over. The loader in fact pushed bare `Linear` layers and +applied no activation at all, so *every* non-linear model returned a linear +model's numbers; and the only test of the numbers compared them against the +same bare stack, so nothing could see it. An assumption that produces silent +wrong output is not made safe by an ADR paragraph admitting it — reading the +value, or refusing, is the only version of this that a user can trust. The +`none` case is now compared against oxmera's own `Sequential::forward`, which +does not share the loader's loop. + +`ModelSpec` remains the enum where a second *architecture* goes; the +activation is a field on its one variant rather than a new one. ## Consequences @@ -86,7 +100,12 @@ documented at the call site. the test runs the same model through `oxmera` directly and asserts the SQL answer matches within `1e-5`. A wrong architecture inference is the failure mode this guards. +- A model file that predates 0.2.0 no longer runs until it declares an + activation or the query states one. That is deliberate: those files were + being scored as linear models whatever they were trained as, so continuing + to accept them silently is the bug, not the compatibility. - Not addressed: GPU inference (the model runs on the CPU), model cache invalidation (a file that changes under a running process keeps serving the - old weights — right for a query engine, wrong for a notebook), and any model - that is not an MLP. + old weights — right for a query engine, wrong for a notebook), per-layer + activations (one value covers the whole stack), and any model that is not + an MLP. From fe402b7c637a59001b34b675653b41111527b9a2 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:18:17 +0700 Subject: [PATCH 5/8] feat(telemetry): count CPU fallbacks and print why a node stayed on the CPU Closes #32 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- STATUS.md | 2 +- crates/oxidelake-compute/src/operator.rs | 400 +++++++++++++++--- crates/oxidelake-core/src/telemetry.rs | 80 ++++ crates/oxidelake-planner/src/rule.rs | 187 ++++++-- crates/oxidelake-planner/tests/placement.rs | 75 ++++ crates/oxidelake-runtime/src/dashboard.rs | 3 + crates/oxidelake-runtime/src/session.rs | 31 +- crates/oxidelake-runtime/tests/cli.rs | 46 ++ crates/oxidelake-tui/src/model.rs | 6 + crates/oxidelake-tui/src/render.rs | 29 ++ ...renders_navigates_resizes_and_quits-3.snap | 2 +- ...tui_render_test__initial_frame_120x40.snap | 2 +- docs/architecture.md | 4 +- 13 files changed, 765 insertions(+), 102 deletions(-) diff --git a/STATUS.md b/STATUS.md index ac472d2..72ca7af 100644 --- a/STATUS.md +++ b/STATUS.md @@ -33,7 +33,7 @@ Filled in as phases complete. Every component goes under exactly one column — | CUDA kernel sources (`crates/oxidelake-device/kernels/cuda/*.cu`) + launch orchestration (`cuda/ops.rs`) | embedded-source and scan-helper unit tests; **(T4)** every kernel NVRTC-compiled and launched: conformance suite `OXIDE_BACKEND=cuda cargo test -p oxidelake-compute --features cuda -- --ignored` equal to stock DataFusion (3 seeds × 5 sizes × filter/join/aggregate/vector L2+cosine) **with the transfer-counter witness (`bytes_h2d`/`bytes_d2h` > 0)**; the aggregation kernel's group-table publication bug (below) fixed and re-verified | | | | Metal kernel sources (`crates/oxidelake-device/kernels/metal/*.metal`) | **(macOS)** MSL runtime compile + dispatch, exercised by the conformance suite above | | | | `Gpu*Exec` operators (filter+project, hash join, aggregate, vector distance) on `CpuBackend` | conformance suite vs stock DataFusion; EXPLAIN tags; per-batch CPU fallback path; **(macOS)** same suite executed on `MetalBackend`; **(T4)** same suite executed on `CudaBackend` | | | -| `GpuOperator` fallback + telemetry, `local_backend()` selection | unit + conformance tests (CPU); **(T4)** CUDA streams and transfer accounting through the conformance witness; a 1M-row `oxide sql … --target cuda` with `RUST_LOG=oxidelake_compute=debug` logged **no** per-batch CPU fallback | | | +| `GpuOperator` fallback + telemetry, `local_backend()` selection | unit + conformance tests (CPU); **(T4)** CUDA streams and transfer accounting through the conformance witness; a 1M-row `oxide sql … --target cuda` with `RUST_LOG=oxidelake_compute=debug` logged **no** per-batch CPU fallback (since 0.2.0 the same claim is a counter, `fallback_batches`, in the Inspector panel and in every `OperatorSnapshot`, rather than something only a raised log level can see — #32) | | | | Parquet writer options + `write_parquet` (row groups, dictionaries, Bloom filters on keys, page statistics) | unit test inspects row-group/Bloom metadata | | | | DataFusion pruning settings + `scan_pruning_metrics` proof (statistics, Bloom filter, page index) | `tests/pruning.rs` on a 200k-row / 100-row-group dataset | | | | Arrow IPC cache/spill files (64-byte-aligned buffers) | round-trip + alignment + truncation tests | | | diff --git a/crates/oxidelake-compute/src/operator.rs b/crates/oxidelake-compute/src/operator.rs index 50eec50..f678347 100644 --- a/crates/oxidelake-compute/src/operator.rs +++ b/crates/oxidelake-compute/src/operator.rs @@ -17,7 +17,7 @@ //! * a hash join's build side is hashed once ([`JoinBuild`]) and uploaded once //! per operator, then probed by every batch. -use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, OnceLock}; use std::time::Instant; @@ -172,6 +172,10 @@ pub struct GpuOperator { next_stream: AtomicUsize, stats: Option>, device_build: OnceLock>, + /// One `warn!` per operator instance, not one per batch: a fallback that + /// happens at all usually happens for every batch, and a log line per + /// batch would bury the fact it is reporting. + warned_fallback: AtomicBool, } impl std::fmt::Debug for GpuOperator { @@ -183,9 +187,19 @@ impl std::fmt::Debug for GpuOperator { } } -/// What a device attempt produced: the downloaded batch and the bytes moved -/// host→device / device→host, or nothing when the backend declined. -type DeviceOutcome = Option<(RecordBatch, (usize, usize))>; +/// What a device attempt produced. +/// +/// The declining case carries *why* rather than being a bare `None` (#32): a +/// fallback is counted and warned about at the point the decision is made, +/// and a counter that cannot say what it counted is not much better than the +/// `debug!` it replaces. +enum DeviceOutcome { + /// It ran on the device: the downloaded batch and the bytes moved + /// host→device / device→host. + Ran(RecordBatch, (usize, usize)), + /// The device path was declined; the CPU reference runs instead. + Declined(EngineError), +} impl GpuOperator { /// Creates an operator on `backend`, optionally reporting into `stats`. @@ -204,6 +218,7 @@ impl GpuOperator { next_stream: AtomicUsize::new(0), stats, device_build: OnceLock::new(), + warned_fallback: AtomicBool::new(false), }) } @@ -232,8 +247,9 @@ impl GpuOperator { } } - /// Runs `gpu` on a GPU backend; `Ok(None)` when the backend answered - /// `Unsupported` (the caller then takes the CPU reference path). + /// Runs `gpu` on a GPU backend; [`DeviceOutcome::Declined`] when the + /// backend answered `Unsupported` (the caller then takes the CPU + /// reference path). fn try_device(&self, gpu: G) -> Result where G: FnOnce(&DeviceStream) -> Result<(RecordBatch, usize), EngineError>, @@ -242,16 +258,54 @@ impl GpuOperator { match gpu(stream) { Ok((batch, h2d)) => { let d2h = batch.get_array_memory_size(); - Ok(Some((batch, (h2d, d2h)))) - } - Err(err) if err.is_unsupported() => { - tracing::debug!(backend = %self.backend.kind(), error = %err, "GPU path unsupported for this batch; using the CPU reference"); - Ok(None) + Ok(DeviceOutcome::Ran(batch, (h2d, d2h))) } + Err(err) if err.is_unsupported() => Ok(DeviceOutcome::Declined(err)), Err(err) => Err(err), } } + /// Why this operator has no device path for an operation, or `None` when + /// it has one. + /// + /// `supported` is the backend's own answer for the specific shape (this + /// predicate, a hash join, …), asked before anything is uploaded. + fn no_device_path(&self, supported: bool, what: &str) -> Option { + let kind = self.backend.kind(); + if !kind.is_gpu() { + Some(EngineError::unsupported( + "backend.device", + format!( + "the backend selected on this machine is `{kind}`, so an \ + operator placed on a device runs the CPU reference" + ), + )) + } else if !supported { + Some(EngineError::unsupported( + "backend.operation", + format!("backend `{kind}` does not support {what}"), + )) + } else { + None + } + } + + /// Counts a batch that took the CPU reference path, and says so once. + fn fell_back(&self, why: &EngineError) { + if let Some(stats) = &self.stats { + stats.record_fallback(); + } + if !self.warned_fallback.swap(true, Ordering::Relaxed) { + tracing::warn!( + backend = %self.backend.kind(), + reason = %why, + "operator placed on a device is running the CPU reference; \ + results are correct and unaccelerated" + ); + } + tracing::debug!(backend = %self.backend.kind(), reason = %why, "batch on the CPU reference"); + } + /// Uploads `sub`, runs `kernel` on it and downloads the result. fn round_trip( &self, @@ -274,15 +328,18 @@ impl GpuOperator { projection: &[usize], ) -> Result { let start = Instant::now(); - let device_ok = self.backend.kind().is_gpu() && self.backend.supports_predicate(predicate); - let outcome = if device_ok { - self.filter_project_on_device(batch, predicate, projection)? - } else { - None + let outcome = match self + .no_device_path(self.backend.supports_predicate(predicate), "this predicate") + { + Some(why) => DeviceOutcome::Declined(why), + None => self.filter_project_on_device(batch, predicate, projection)?, }; let (out, moved) = match outcome { - Some((out, moved)) => (out, Some(moved)), - None => (kernels::filter_project(batch, predicate, projection)?, None), + DeviceOutcome::Ran(out, moved) => (out, Some(moved)), + DeviceOutcome::Declined(why) => { + self.fell_back(&why); + (kernels::filter_project(batch, predicate, projection)?, None) + } }; self.record(batch.num_rows(), &out, start, moved); Ok(out) @@ -329,7 +386,7 @@ impl GpuOperator { if let Some(pos) = plan.row_id_position() { dev_projection.push(pos); } - let Some((dev_out, moved)) = self.try_device(|stream| { + let (dev_out, moved) = match self.try_device(|stream| { self.round_trip(stream, &sub, |stream, dev| { self.backend.filter_project( stream, @@ -340,9 +397,9 @@ impl GpuOperator { }, ) }) - })? - else { - return Ok(None); + })? { + DeviceOutcome::Ran(batch, moved) => (batch, moved), + declined => return Ok(declined), }; // Reassemble in projection order: device outputs for eligible columns, // host gathers (at the compacted row ids) for the rest. @@ -365,7 +422,7 @@ impl GpuOperator { } } let out = RecordBatch::try_new(Arc::new(Schema::new(fields)), columns)?; - Ok(Some((out, moved))) + Ok(DeviceOutcome::Ran(out, moved)) } /// Inner hash join of one probe batch against a prepared build side. The @@ -379,18 +436,25 @@ impl GpuOperator { projection: Option<&[usize]>, ) -> Result { let start = Instant::now(); - let device_ok = self.backend.kind().is_gpu() && self.backend.supports_hash_join(); - let outcome = if device_ok { - self.hash_join_on_device(left, build, left_key, projection)? - } else { - None + let outcome = match self.no_device_path(self.backend.supports_hash_join(), "hash joins") { + Some(why) => DeviceOutcome::Declined(why), + None => self.hash_join_on_device(left, build, left_key, projection)?, }; let (out, moved) = match outcome { - Some((out, moved)) => (out, Some(moved)), - None => ( - kernels::hash_join_probe(left, left_key, build.batch(), build.table(), projection)?, - None, - ), + DeviceOutcome::Ran(out, moved) => (out, Some(moved)), + DeviceOutcome::Declined(why) => { + self.fell_back(&why); + ( + kernels::hash_join_probe( + left, + left_key, + build.batch(), + build.table(), + projection, + )?, + None, + ) + } }; self.record(left.num_rows(), &out, start, moved); Ok(out) @@ -479,7 +543,7 @@ impl GpuOperator { let left_sub = left_plan.sub_batch(left)?; let left_key_pos = left_plan.position(left_key).unwrap_or(0); let right_key_pos = right_plan.position(build.key()).unwrap_or(0); - let Some((dev_out, moved)) = self.try_device(|stream| { + let (dev_out, moved) = match self.try_device(|stream| { let dev_build = self.device_build(stream, build, &right_plan)?; self.round_trip(stream, &left_sub, |stream, dev_left| { self.backend.hash_join( @@ -492,9 +556,9 @@ impl GpuOperator { }, ) }) - })? - else { - return Ok(None); + })? { + DeviceOutcome::Ran(batch, moved) => (batch, moved), + declined => return Ok(declined), }; // Device output = left sub-batch columns ++ right sub-batch columns // (each with its row-id column last when present). @@ -532,7 +596,7 @@ impl GpuOperator { } } let out = RecordBatch::try_new(Arc::new(Schema::new(fields)), columns)?; - Ok(Some((out, moved))) + Ok(DeviceOutcome::Ran(out, moved)) } /// Grouped aggregation of one (complete) input batch. @@ -542,15 +606,16 @@ impl GpuOperator { spec: &AggregateSpec, ) -> Result { let start = Instant::now(); - let device_ok = self.backend.kind().is_gpu() && self.backend.supports_aggregate(); - let outcome = if device_ok { - self.aggregate_on_device(batch, spec)? - } else { - None + let outcome = match self.no_device_path(self.backend.supports_aggregate(), "aggregation") { + Some(why) => DeviceOutcome::Declined(why), + None => self.aggregate_on_device(batch, spec)?, }; let (out, moved) = match outcome { - Some((out, moved)) => (out, Some(moved)), - None => (kernels::aggregate(batch, spec)?, None), + DeviceOutcome::Ran(out, moved) => (out, Some(moved)), + DeviceOutcome::Declined(why) => { + self.fell_back(&why); + (kernels::aggregate(batch, spec)?, None) + } }; self.record(batch.num_rows(), &out, start, moved); Ok(out) @@ -603,17 +668,19 @@ impl GpuOperator { output_name: &str, ) -> Result { let start = Instant::now(); - let outcome = if self.backend.kind().is_gpu() { - self.vector_distance_on_device(batch, column, query, metric, output_name)? - } else { - None + let outcome = match self.no_device_path(true, "vector distance") { + Some(why) => DeviceOutcome::Declined(why), + None => self.vector_distance_on_device(batch, column, query, metric, output_name)?, }; let (out, moved) = match outcome { - Some((out, moved)) => (out, Some(moved)), - None => ( - kernels::vector_distance(batch, column, query, metric, output_name)?, - None, - ), + DeviceOutcome::Ran(out, moved) => (out, Some(moved)), + DeviceOutcome::Declined(why) => { + self.fell_back(&why); + ( + kernels::vector_distance(batch, column, query, metric, output_name)?, + None, + ) + } }; self.record(batch.num_rows(), &out, start, moved); Ok(out) @@ -634,7 +701,7 @@ impl GpuOperator { row_ids: false, }; let sub = plan.sub_batch(batch)?; - let Some((dev_out, moved)) = self.try_device(|stream| { + let (dev_out, moved) = match self.try_device(|stream| { self.round_trip(stream, &sub, |stream, dev| { self.backend.vector_distance( stream, @@ -647,9 +714,9 @@ impl GpuOperator { }, ) }) - })? - else { - return Ok(None); + })? { + DeviceOutcome::Ran(batch, moved) => (batch, moved), + declined => return Ok(declined), }; let distance = Arc::clone(dev_out.column(dev_out.num_columns() - 1)); let mut fields: Vec = batch.schema().fields().iter().cloned().collect(); @@ -657,7 +724,7 @@ impl GpuOperator { let mut columns = batch.columns().to_vec(); columns.push(distance); let out = RecordBatch::try_new(Arc::new(Schema::new(fields)), columns)?; - Ok(Some((out, moved))) + Ok(DeviceOutcome::Ran(out, moved)) } } @@ -665,9 +732,132 @@ impl GpuOperator { #[allow(clippy::unwrap_used, clippy::expect_used)] mod tests { use arrow::array::StringArray; + use oxidelake_core::telemetry::TelemetryHub; + use oxidelake_core::{BackendKind, DeviceId}; + use oxidelake_device::{CpuBackend, DeviceBuffer, HostBuffer, MemoryInfo}; use super::*; + /// A backend that *claims* to be a GPU and runs the CPU kernels. + /// + /// This is the only way to test the fallback counter (#32) on a machine + /// with no device: the counter's whole purpose is to distinguish a plan + /// placed on a device from the work that actually ran there, and both + /// halves of that need a backend that reports `cuda` while declining some + /// shapes. `float_predicates` is the knob the Metal backend really has — + /// it has no f64, so a `Float64` comparison is declined before anything + /// is uploaded. + #[derive(Debug)] + struct FakeGpu { + inner: CpuBackend, + float_predicates: bool, + } + + impl FakeGpu { + fn shared(float_predicates: bool) -> Arc { + Arc::new(Self { + inner: CpuBackend::new(), + float_predicates, + }) + } + } + + /// Exhaustive on purpose: `Predicate` is plan vocabulary, so a variant + /// added to it must be classified here rather than falling into a `_` + /// arm that would quietly report "no float" for a float predicate. + fn mentions_float(predicate: &Predicate) -> bool { + use oxidelake_core::params::Literal; + match predicate { + Predicate::Compare { literal, .. } => matches!(literal, Literal::Float64(_)), + Predicate::And(a, b) => mentions_float(a) || mentions_float(b), + } + } + + impl GpuBackend for FakeGpu { + fn kind(&self) -> BackendKind { + BackendKind::Cuda + } + fn device_id(&self) -> DeviceId { + DeviceId::new(BackendKind::Cuda, 0) + } + fn memory_info(&self) -> Result { + self.inner.memory_info() + } + fn alloc_device(&self, bytes: usize) -> Result { + self.inner.alloc_device(bytes) + } + fn alloc_pinned_host(&self, bytes: usize) -> Result { + self.inner.alloc_pinned_host(bytes) + } + fn create_stream(&self) -> Result { + self.inner.create_stream() + } + fn copy_h2d( + &self, + stream: &DeviceStream, + src: &HostBuffer, + dst: &mut DeviceBuffer, + ) -> Result<(), EngineError> { + self.inner.copy_h2d(stream, src, dst) + } + fn copy_d2h( + &self, + stream: &DeviceStream, + src: &DeviceBuffer, + dst: &mut HostBuffer, + ) -> Result<(), EngineError> { + self.inner.copy_d2h(stream, src, dst) + } + fn synchronize(&self, stream: &DeviceStream) -> Result<(), EngineError> { + self.inner.synchronize(stream) + } + fn upload( + &self, + stream: &DeviceStream, + batch: &RecordBatch, + ) -> Result { + self.inner.upload(stream, batch) + } + fn download( + &self, + stream: &DeviceStream, + batch: &DeviceBatch, + ) -> Result { + self.inner.download(stream, batch) + } + fn filter_project( + &self, + stream: &DeviceStream, + args: FilterProjectArgs<'_>, + ) -> Result { + self.inner.filter_project(stream, args) + } + fn hash_join( + &self, + stream: &DeviceStream, + args: HashJoinArgs<'_>, + ) -> Result { + self.inner.hash_join(stream, args) + } + fn aggregate( + &self, + stream: &DeviceStream, + args: AggregateArgs<'_>, + ) -> Result { + self.inner.aggregate(stream, args) + } + fn vector_distance( + &self, + stream: &DeviceStream, + args: VectorDistanceArgs<'_>, + ) -> Result { + self.inner.vector_distance(stream, args) + } + fn supports_predicate(&self, predicate: &Predicate) -> bool { + self.float_predicates || !mentions_float(predicate) + } + } + fn batch() -> RecordBatch { let schema = Arc::new(Schema::new(vec![ Field::new("k", DataType::Int64, true), @@ -752,4 +942,96 @@ mod tests { assert_eq!(build.key(), 0); assert_eq!(build.batch().num_rows(), 4); } + + // --- the CPU-fallback counter (#32) ------------------------------------ + + fn int_predicate() -> Predicate { + Predicate::compare( + 0, + oxidelake_core::params::Comparison::Gt, + oxidelake_core::params::Literal::Int64(1), + ) + } + + fn float_predicate() -> Predicate { + Predicate::compare( + 2, + oxidelake_core::params::Comparison::Lt, + oxidelake_core::params::Literal::Float64(3.0), + ) + } + + fn run_filter(backend: Arc, predicate: &Predicate) -> (u64, u64, RecordBatch) { + let hub = TelemetryHub::new(); + let stats = hub.register_operator("GpuFilterExec", backend.kind()); + let op = GpuOperator::new(backend, Some(Arc::clone(&stats))).unwrap(); + let b = batch(); + let out = op.filter_project(&b, predicate, &[0, 2]).unwrap(); + let snapshot = stats.snapshot(); + (snapshot.batches, snapshot.fallback_batches, out) + } + + /// A predicate the backend accepts runs on the device: no fallback. + #[test] + fn a_supported_predicate_counts_no_fallback() { + let (batches, fallbacks, _) = run_filter(FakeGpu::shared(true), &float_predicate()); + assert_eq!(batches, 1); + assert_eq!(fallbacks, 0); + } + + /// A predicate the backend declines is counted, and the answer is still + /// right — which is exactly why the counter has to exist: nothing else + /// about the query changes. + #[test] + fn a_declined_predicate_counts_a_fallback_and_still_answers() { + let strict = FakeGpu::shared(false); + let lenient = FakeGpu::shared(true); + let (batches, fallbacks, declined) = run_filter(strict, &float_predicate()); + assert_eq!(batches, 1); + assert_eq!(fallbacks, 1, "a declined predicate is a counted fallback"); + let (_, accepted_fallbacks, accepted) = run_filter(lenient, &float_predicate()); + assert_eq!(accepted_fallbacks, 0); + assert_eq!(declined, accepted, "the two paths must agree on the rows"); + + // The same backend with an Int64 predicate has nothing to decline. + let (_, int_fallbacks, _) = run_filter(FakeGpu::shared(false), &int_predicate()); + assert_eq!(int_fallbacks, 0); + } + + /// The shape #32 is about: an operator placed on a device on a machine + /// that has none. Every batch is a fallback, and the snapshot says so in + /// one call. + #[test] + fn a_cpu_machine_running_a_gpu_plan_falls_back_entirely() { + let hub = TelemetryHub::new(); + let stats = hub.register_operator("GpuFilterExec", BackendKind::Cuda); + let op = GpuOperator::new(Arc::new(CpuBackend::new()), Some(Arc::clone(&stats))).unwrap(); + let b = batch(); + for _ in 0..3 { + op.filter_project(&b, &int_predicate(), &[0, 2]).unwrap(); + } + let snapshot = stats.snapshot(); + assert_eq!(snapshot.batches, 3); + assert_eq!(snapshot.fallback_batches, 3); + assert!(snapshot.fell_back_entirely()); + } + + /// Aggregation and vector distance count too, not just the filter. + #[test] + fn every_operator_counts_its_fallbacks() { + let hub = TelemetryHub::new(); + let b = batch(); + + let stats = hub.register_operator("GpuAggregateExec", BackendKind::Cuda); + let op = GpuOperator::new(Arc::new(CpuBackend::new()), Some(Arc::clone(&stats))).unwrap(); + op.aggregate( + &b, + &oxidelake_core::params::AggregateSpec { + group_by: 0, + aggregates: vec![(oxidelake_core::params::AggregateFunction::Count, 2)], + }, + ) + .unwrap(); + assert_eq!(stats.snapshot().fallback_batches, 1); + } } diff --git a/crates/oxidelake-core/src/telemetry.rs b/crates/oxidelake-core/src/telemetry.rs index 8259406..dbae6a3 100644 --- a/crates/oxidelake-core/src/telemetry.rs +++ b/crates/oxidelake-core/src/telemetry.rs @@ -46,6 +46,7 @@ pub struct OperatorStats { bytes_h2d: AtomicU64, bytes_d2h: AtomicU64, memory_bytes: AtomicU64, + fallback_batches: AtomicU64, } impl OperatorStats { @@ -61,6 +62,7 @@ impl OperatorStats { bytes_h2d: AtomicU64::new(0), bytes_d2h: AtomicU64::new(0), memory_bytes: AtomicU64::new(0), + fallback_batches: AtomicU64::new(0), } } @@ -99,6 +101,22 @@ impl OperatorStats { self.memory_bytes.store(bytes, Ordering::Relaxed); } + /// Records one batch that took the CPU reference path although the + /// operator was placed on a device (#32). + /// + /// Identical results and identical `EXPLAIN` tags are exactly what a + /// silently-falling-back cluster produces, so this is the number that + /// distinguishes "the GPU ran it" from "something ran it". A batch is + /// counted once, where the decision is made. + pub fn record_fallback(&self) { + self.fallback_batches.fetch_add(1, Ordering::Relaxed); + } + + /// Batches that took the CPU reference path. + pub fn fallback_batches(&self) -> u64 { + self.fallback_batches.load(Ordering::Relaxed) + } + /// A point-in-time copy of the counters. pub fn snapshot(&self) -> OperatorSnapshot { OperatorSnapshot { @@ -112,6 +130,7 @@ impl OperatorStats { bytes_h2d: self.bytes_h2d.load(Ordering::Relaxed), bytes_d2h: self.bytes_d2h.load(Ordering::Relaxed), memory_bytes: self.memory_bytes.load(Ordering::Relaxed), + fallback_batches: self.fallback_batches.load(Ordering::Relaxed), } } } @@ -139,9 +158,20 @@ pub struct OperatorSnapshot { pub bytes_d2h: u64, /// Currently allocated bytes. pub memory_bytes: u64, + /// Batches that ran on the CPU reference although the operator was + /// placed on a device. Zero is the claim "this operator is accelerated"; + /// anything else is the size of the gap between the plan and what ran. + pub fallback_batches: u64, } impl OperatorSnapshot { + /// `true` when every batch this operator processed took the CPU + /// reference path — the shape of a GPU deployment that is not using its + /// GPU at all. + pub fn fell_back_entirely(&self) -> bool { + self.batches > 0 && self.fallback_batches >= self.batches + } + /// Mean per-batch latency in milliseconds (zero when nothing ran). pub fn mean_batch_latency_ms(&self) -> f64 { if self.batches == 0 { @@ -261,6 +291,26 @@ pub struct PlanNodeSummary { pub detail: String, } +/// Why the placement rule left a node on the CPU (#32). +/// +/// A skipped node is invisible in `EXPLAIN`: it is an ordinary DataFusion +/// operator, indistinguishable from one that was never eligible. The reason +/// was written to `debug!` and nowhere else, so the only way to find out was +/// to re-run the query with `RUST_LOG` turned up — which is not available to +/// someone reading a plan. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PlacementSkip { + /// The DataFusion operator that stayed on the CPU, e.g. `HashJoinExec`. + pub node: String, + /// Why, in one line. + pub reason: String, +} + +/// Most placement notes a hub keeps for one plan. A pathological plan should +/// not grow the hub without bound, and a reader stops reading long before +/// this. +pub const MAX_PLACEMENT_SKIPS: usize = 256; + /// Most operator registrations a hub keeps. A hub lives as long as its session /// and every `Gpu*Exec` instance registers once, so without a bound a /// long-lived session would accumulate one entry per operator of every query @@ -276,6 +326,7 @@ pub struct TelemetryHub { tiers: TierGauges, spill: SpillCounters, plan: RwLock>, + skips: RwLock>, } impl TelemetryHub { @@ -314,6 +365,35 @@ impl TelemetryHub { .clear(); } + /// Records why the placement rule left a node on the CPU. + pub fn record_skip(&self, node: impl Into, reason: impl Into) { + let mut skips = self.skips.write().unwrap_or_else(PoisonError::into_inner); + if skips.len() >= MAX_PLACEMENT_SKIPS { + return; + } + skips.push(PlacementSkip { + node: node.into(), + reason: reason.into(), + }); + } + + /// The placement notes recorded since the last [`Self::clear_skips`]. + pub fn skips(&self) -> Vec { + self.skips + .read() + .unwrap_or_else(PoisonError::into_inner) + .clone() + } + + /// Forgets the placement notes (called before planning a fresh query, so + /// the notes belong to the plan being looked at). + pub fn clear_skips(&self) { + self.skips + .write() + .unwrap_or_else(PoisonError::into_inner) + .clear(); + } + /// Replaces the displayed plan. pub fn set_plan(&self, nodes: Vec) { *self.plan.write().unwrap_or_else(PoisonError::into_inner) = nodes; diff --git a/crates/oxidelake-planner/src/rule.rs b/crates/oxidelake-planner/src/rule.rs index 5022f89..cc2ef80 100644 --- a/crates/oxidelake-planner/src/rule.rs +++ b/crates/oxidelake-planner/src/rule.rs @@ -69,6 +69,21 @@ impl HardwarePlacementRule { self.target } + /// Records why a node stayed on the CPU, in the log and — when a hub is + /// attached — where `oxide explain` can print it (#32). + /// + /// A skipped node looks exactly like one that was never eligible, so + /// without this the reason existed only in a `debug!` line nobody was + /// going to turn on. + fn skip(&self, node: &str, reason: impl std::fmt::Display) -> Option<()> { + let reason = reason.to_string(); + tracing::debug!(node, reason, "leaving node on the CPU"); + if let Some(hub) = &self.telemetry { + hub.record_skip(node, reason); + } + None + } + /// Attaches the rule's telemetry hub to a freshly built exec, when set. fn instrument(&self, exec: T, with_telemetry: impl FnOnce(T, Arc) -> T) -> T { match &self.telemetry { @@ -112,8 +127,12 @@ impl HardwarePlacementRule { fn lower_filter(&self, filter: &FilterExec) -> Result> { let input = filter.input(); let schema = input.schema(); - let Some(predicate) = lower_predicate(filter.predicate(), &schema) else { - return Ok(None); + let predicate = match try_lower_predicate(filter.predicate(), &schema) { + Ok(predicate) => predicate, + Err(why) => { + self.skip("FilterExec", why); + return Ok(None); + } }; let projection: Vec = match filter.projection() { Some(p) => p.iter().copied().collect(), @@ -121,7 +140,7 @@ impl HardwarePlacementRule { }; let exec = GpuFilterExec::try_new(Arc::clone(input), predicate, projection, self.target)?; if exec.schema() != filter.schema() { - tracing::debug!("skipping FilterExec: fused schema differs from the original"); + self.skip("FilterExec", "the fused schema differs from the original"); return Ok(None); } Ok(Some(self.instrument(exec, GpuFilterExec::with_telemetry))) @@ -200,7 +219,10 @@ impl HardwarePlacementRule { ) { Ok(exec) => exec, Err(err) => { - tracing::debug!(error = %err, "skipping ProjectionExec: distance call not representable"); + self.skip( + "ProjectionExec", + format_args!("the distance call is not representable: {err}"), + ); return None; } }; @@ -253,7 +275,10 @@ impl HardwarePlacementRule { match matches.as_slice() { [only] => indices.push(*only), _ => { - tracing::debug!(field = %field.name(), "skipping HashJoinExec: projected field is ambiguous"); + self.skip( + "HashJoinExec", + format_args!("the projected field `{}` is ambiguous", field.name()), + ); return Ok(None); } } @@ -276,7 +301,10 @@ impl HardwarePlacementRule { )? .with_projection(Some(ours))?; if exec.schema() != join.schema() { - tracing::debug!("skipping HashJoinExec: rewritten schema differs from the original"); + self.skip( + "HashJoinExec", + "the rewritten schema differs from the original", + ); return Ok(None); } Ok(Some(self.instrument(exec, GpuHashJoinExec::with_telemetry))) @@ -310,8 +338,9 @@ impl HardwarePlacementRule { } let exec = GpuHashJoinExec::try_new(left, right, lk, rk, self.target)?; if exec.schema() != join.schema() { - tracing::debug!( - "skipping SortMergeJoinExec: rewritten schema differs from the original" + self.skip( + "SortMergeJoinExec", + "the rewritten schema differs from the original", ); return Ok(None); } @@ -321,8 +350,12 @@ impl HardwarePlacementRule { fn lower_aggregate(&self, agg: &AggregateExec) -> Result> { let (source, spec) = match agg.mode() { AggregateMode::Single | AggregateMode::SinglePartitioned => { - let Some(spec) = lower_aggregate_spec(agg) else { - return Ok(None); + let spec = match try_lower_aggregate_spec(agg) { + Ok(spec) => spec, + Err(why) => { + self.skip("AggregateExec", why); + return Ok(None); + } }; (Arc::clone(agg.input()), spec) } @@ -348,8 +381,12 @@ impl HardwarePlacementRule { { return Ok(None); } - let Some(spec) = lower_aggregate_spec(partial) else { - return Ok(None); + let spec = match try_lower_aggregate_spec(partial) { + Ok(spec) => spec, + Err(why) => { + self.skip("AggregateExec", why); + return Ok(None); + } }; (Arc::clone(partial.input()), spec) } @@ -374,7 +411,10 @@ impl HardwarePlacementRule { self.instrument(exec, GpuAggregateExec::with_telemetry), )), Err(err) => { - tracing::debug!(error = %err, "skipping AggregateExec: output schema not representable"); + self.skip( + "AggregateExec", + format_args!("the output schema is not representable: {err}"), + ); Ok(None) } } @@ -429,40 +469,92 @@ fn literal(expr: &Arc) -> Option { /// Lowers a DataFusion predicate into the bounded v1 grammar, or `None` when /// any part of it is outside that grammar. pub fn lower_predicate(expr: &Arc, schema: &Schema) -> Option { - let binary = as_any(expr).downcast_ref::()?; + try_lower_predicate(expr, schema).ok() +} + +/// [`lower_predicate`], keeping the reason so it can be shown (#32). +/// +/// The public function stays `Option`-shaped; this is the same walk with the +/// refusal named, so the two cannot disagree about what is representable. +fn try_lower_predicate( + expr: &Arc, + schema: &Schema, +) -> std::result::Result { + let Some(binary) = as_any(expr).downcast_ref::() else { + return Err(format!( + "`{expr}` is not a binary comparison; the v1 grammar is \ + column literal, conjoined with AND" + )); + }; if *binary.op() == Operator::And { - let l = lower_predicate(binary.left(), schema)?; - let r = lower_predicate(binary.right(), schema)?; - return Some(Predicate::and(l, r)); + let l = try_lower_predicate(binary.left(), schema)?; + let r = try_lower_predicate(binary.right(), schema)?; + return Ok(Predicate::and(l, r)); } - let op = comparison(binary.op())?; + let Some(op) = comparison(binary.op()) else { + return Err(format!( + "`{}` is not one of the comparisons the device kernels implement \ + (=, !=, <, <=, >, >=)", + binary.op() + )); + }; let (column, lit, op) = match (column_index(binary.left()), literal(binary.right())) { (Some(c), Some(l)) => (c, l, op), _ => match (literal(binary.left()), column_index(binary.right())) { (Some(l), Some(c)) => (c, l, flip(op)), - _ => return None, + _ => { + return Err(format!( + "`{binary}` does not compare a column against an Int64 or \ + Float64 literal" + )); + } }, }; - let dt = schema.fields().get(column)?.data_type(); - if *dt != lit.data_type() { - return None; + let Some(field) = schema.fields().get(column) else { + return Err(format!( + "column {column} is out of range for the input schema" + )); + }; + if *field.data_type() != lit.data_type() { + return Err(format!( + "`{}` is {} and the literal is {}; the kernels compare a column \ + against a literal of its own type", + field.name(), + field.data_type(), + lit.data_type() + )); } - Some(Predicate::compare(column, op, lit)) + Ok(Predicate::compare(column, op, lit)) } /// Lowers an `AggregateExec`'s grouping and aggregates into an [`AggregateSpec`]. pub fn lower_aggregate_spec(agg: &AggregateExec) -> Option { + try_lower_aggregate_spec(agg).ok() +} + +/// [`lower_aggregate_spec`], keeping the reason so it can be shown (#32). +fn try_lower_aggregate_spec(agg: &AggregateExec) -> std::result::Result { let group = agg.group_expr(); if !group.is_single() || group.expr().len() != 1 || !group.null_expr().is_empty() { - return None; + return Err(format!( + "the device kernel groups by exactly one column; this groups by {}", + group.expr().len() + )); } let input_schema = agg.input().schema(); - let group_by = column_index(&group.expr()[0].0)?; - if input_schema.field(group_by).data_type() != &DataType::Int64 { - return None; + let Some(group_by) = column_index(&group.expr()[0].0) else { + return Err("the grouping key is an expression, not a plain column".to_owned()); + }; + let group_field = input_schema.field(group_by); + if group_field.data_type() != &DataType::Int64 { + return Err(format!( + "the grouping key `{}` is {}; the device kernel groups on Int64", + group_field.name(), + group_field.data_type() + )); } if agg.filter_expr().iter().any(Option::is_some) { - return None; + return Err("a FILTER clause on an aggregate has no device kernel".to_owned()); } let mut aggregates = Vec::with_capacity(agg.aggr_expr().len()); for expr in agg.aggr_expr() { @@ -471,25 +563,44 @@ pub fn lower_aggregate_spec(agg: &AggregateExec) -> Option { "count" => AggregateFunction::Count, "min" => AggregateFunction::Min, "max" => AggregateFunction::Max, - _ => return None, + other => { + return Err(format!( + "`{other}` is not one of the aggregates the device kernels \ + implement (sum, count, min, max)" + )); + } }; if expr.is_distinct() { - return None; + return Err(format!( + "DISTINCT is not implemented for `{}`", + expr.fun().name() + )); } let args = expr.expressions(); if args.len() != 1 { - return None; + return Err(format!( + "`{}` takes {} arguments; the device kernels take one", + expr.fun().name(), + args.len() + )); } - let column = column_index(&args[0])?; - if !matches!( - input_schema.field(column).data_type(), - DataType::Int64 | DataType::Float64 - ) { - return None; + let Some(column) = column_index(&args[0]) else { + return Err(format!( + "`{}` aggregates an expression, not a plain column", + expr.fun().name() + )); + }; + let field = input_schema.field(column); + if !matches!(field.data_type(), DataType::Int64 | DataType::Float64) { + return Err(format!( + "`{}` is {}; the device kernels aggregate Int64 and Float64", + field.name(), + field.data_type() + )); } aggregates.push((func, column)); } - Some(AggregateSpec { + Ok(AggregateSpec { group_by, aggregates, }) diff --git a/crates/oxidelake-planner/tests/placement.rs b/crates/oxidelake-planner/tests/placement.rs index 0c6a612..39b0076 100644 --- a/crates/oxidelake-planner/tests/placement.rs +++ b/crates/oxidelake-planner/tests/placement.rs @@ -361,3 +361,78 @@ async fn ineligible_distance_projections_stay_on_cpu() { assert_same_results(sql, target, None).await; } } + +// --- placement notes (#32) -------------------------------------------------- + +/// Plans `sql` on a Cuda target with a telemetry hub attached and returns the +/// `node: reason` notes the rule recorded. +async fn placement_notes(sql: &str) -> Vec { + let hub = oxidelake_core::telemetry::TelemetryHub::new(); + let rule = HardwarePlacementRule::new(BackendKind::Cuda).with_telemetry(Arc::clone(&hub)); + let state = SessionStateBuilder::new() + .with_default_features() + .with_physical_optimizer_rules(physical_optimizer_rules(rule)) + .build(); + let ctx = SessionContext::new_with_state(state); + register(&ctx); + hub.clear_skips(); + ctx.sql(sql) + .await + .unwrap() + .create_physical_plan() + .await + .unwrap(); + hub.skips() + .into_iter() + .map(|s| format!("{}: {}", s.node, s.reason)) + .collect() +} + +/// A node that stayed on the CPU says why, and the reason names the thing the +/// reader has to change. Without this the plan shows an ordinary +/// `AggregateExec` and the reason lives only in a `debug!` line. +#[tokio::test] +async fn an_ineligible_shape_records_why_it_stayed_on_the_cpu() { + let notes = placement_notes("SELECT k, avg(v) FROM t GROUP BY k").await; + assert!( + notes.iter().any(|n| n.starts_with("AggregateExec:") + && n.contains("avg") + && n.contains("sum, count, min, max")), + "{notes:?}" + ); + + let notes = placement_notes("SELECT s, sum(v) FROM t GROUP BY s").await; + assert!( + notes + .iter() + .any(|n| n.contains("grouping key `s`") && n.contains("Int64")), + "{notes:?}" + ); + + let notes = placement_notes("SELECT k FROM t WHERE k >= 2 OR v < 1.0").await; + assert!( + notes.iter().any(|n| n.starts_with("FilterExec:")), + "{notes:?}" + ); + + // A string comparison: the literal is not one the kernels hold, and the + // note says so rather than repeating "ineligible". + let notes = placement_notes("SELECT k FROM t WHERE s = 's1'").await; + assert!( + notes + .iter() + .any(|n| n.starts_with("FilterExec:") && n.contains("Int64 or Float64 literal")), + "{notes:?}" + ); +} + +/// A plan the rule lowered completely has nothing to explain. A note that +/// appeared for an accelerated query would teach readers to ignore the +/// section. +#[tokio::test] +async fn a_fully_lowered_plan_records_no_notes() { + assert_eq!( + placement_notes("SELECT k, sum(v) FROM t WHERE k >= 2 GROUP BY k").await, + Vec::::new() + ); +} diff --git a/crates/oxidelake-runtime/src/dashboard.rs b/crates/oxidelake-runtime/src/dashboard.rs index ad4c43b..ae79f37 100644 --- a/crates/oxidelake-runtime/src/dashboard.rs +++ b/crates/oxidelake-runtime/src/dashboard.rs @@ -80,6 +80,7 @@ fn operators_for_plan( bytes_h2d: 0, bytes_d2h: 0, memory_bytes: 0, + fallback_batches: 0, }); entry.rows_in += op.rows_in; entry.rows_out += op.rows_out; @@ -88,6 +89,7 @@ fn operators_for_plan( entry.bytes_h2d += op.bytes_h2d; entry.bytes_d2h += op.bytes_d2h; entry.memory_bytes = entry.memory_bytes.max(op.memory_bytes); + entry.fallback_batches += op.fallback_batches; } plan.iter() .enumerate() @@ -107,6 +109,7 @@ fn operators_for_plan( bytes_h2d: 0, bytes_d2h: 0, memory_bytes: 0, + fallback_batches: 0, }); op.id = id; op.backend = node.backend; diff --git a/crates/oxidelake-runtime/src/session.rs b/crates/oxidelake-runtime/src/session.rs index 176f8d9..bc9b018 100644 --- a/crates/oxidelake-runtime/src/session.rs +++ b/crates/oxidelake-runtime/src/session.rs @@ -205,8 +205,37 @@ impl OxideSession { /// mode. In cluster mode this is the client-side plan (`DistributedQueryExec`); /// the scheduler's plan is what carries the tags there. pub async fn explain(&self, query: &str) -> Result { + self.telemetry.clear_skips(); let plan = self.ctx.sql(query).await?.create_physical_plan().await?; - Ok(displayable(plan.as_ref()).indent(true).to_string()) + let mut text = displayable(plan.as_ref()).indent(true).to_string(); + text.push_str(&self.placement_notes()); + Ok(text) + } + + /// The placement rule's reasons for leaving nodes on the CPU (#32). + /// + /// A node the rule skipped is an ordinary DataFusion operator in the + /// plan above, indistinguishable from one that was never eligible. The + /// reason used to live only in a `debug!` line, which is no use to + /// someone reading a plan, so it is printed under it. + /// + /// Empty in cluster mode: the rule runs on the scheduler there, and this + /// process only planned the `DistributedQueryExec` wrapper. + fn placement_notes(&self) -> String { + let skips = self.telemetry.skips(); + if skips.is_empty() { + return String::new(); + } + let mut out = String::from("\nplacement notes (target "); + match &self.mode { + SessionMode::Embedded { target } => out.push_str(target.as_str()), + SessionMode::Cluster { .. } => out.push_str("on the scheduler"), + } + out.push_str("):\n"); + for skip in skips { + out.push_str(&format!(" {}: {}\n", skip.node, skip.reason)); + } + out } } diff --git a/crates/oxidelake-runtime/tests/cli.rs b/crates/oxidelake-runtime/tests/cli.rs index 890cbd7..773f55c 100644 --- a/crates/oxidelake-runtime/tests/cli.rs +++ b/crates/oxidelake-runtime/tests/cli.rs @@ -565,3 +565,49 @@ fn a_worker_rejects_an_unknown_backend_name() { let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); assert!(stderr.contains("cpu, cuda, metal"), "{stderr}"); } + +/// `oxide explain` names why a node stayed on the CPU (#32). +/// +/// The plan above the notes shows an ordinary `AggregateExec`, which is +/// exactly what a node that was never eligible looks like; the difference is +/// only visible because the rule writes its reason down. +#[test] +fn explain_names_why_a_node_stayed_on_the_cpu() { + let dir = tempfile::tempdir().unwrap(); + gen_data(dir.path(), 4_096); + let table_arg = format!("t={}", dir.path().display()); + + let assert = oxide() + .args([ + "explain", + "-q", + "SELECT k, AVG(v) FROM t GROUP BY k", + "--table", + &table_arg, + "--target", + "cuda", + ]) + .assert() + .success(); + let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + assert!(stdout.contains("placement notes (target cuda)"), "{stdout}"); + assert!(stdout.contains("AggregateExec:"), "{stdout}"); + assert!(stdout.contains("sum, count, min, max"), "{stdout}"); + + // A plan that lowers completely has nothing to explain. + let assert = oxide() + .args([ + "explain", + "-q", + "SELECT k, SUM(v) FROM t WHERE k >= 2 GROUP BY k", + "--table", + &table_arg, + "--target", + "cuda", + ]) + .assert() + .success(); + let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + assert!(stdout.contains("GpuAggregateExec[cuda]"), "{stdout}"); + assert!(!stdout.contains("placement notes"), "{stdout}"); +} diff --git a/crates/oxidelake-tui/src/model.rs b/crates/oxidelake-tui/src/model.rs index 2ce6361..84ae177 100644 --- a/crates/oxidelake-tui/src/model.rs +++ b/crates/oxidelake-tui/src/model.rs @@ -86,6 +86,7 @@ pub fn demo_model() -> DashboardModel { bytes_h2d: 13_000_000, bytes_d2h: 512, memory_bytes: 16_777_216, + fallback_batches: 0, }, OperatorSnapshot { id: 1, @@ -98,6 +99,10 @@ pub fn demo_model() -> DashboardModel { bytes_h2d: 24_000_000, bytes_d2h: 19_500_000, memory_bytes: 33_554_432, + // The demo shows what a partial fallback looks like: 7 of 100 + // batches had a predicate this backend declined, so the panel has + // a non-zero value to render and a reader can see what it means. + fallback_batches: 7, }, OperatorSnapshot { id: 2, @@ -110,6 +115,7 @@ pub fn demo_model() -> DashboardModel { bytes_h2d: 0, bytes_d2h: 0, memory_bytes: 8_388_608, + fallback_batches: 0, }, ]; DashboardModel { diff --git a/crates/oxidelake-tui/src/render.rs b/crates/oxidelake-tui/src/render.rs index c07ec0b..e20419a 100644 --- a/crates/oxidelake-tui/src/render.rs +++ b/crates/oxidelake-tui/src/render.rs @@ -11,6 +11,7 @@ use ratatui::widgets::{ }; use oxidelake_core::BackendKind; +use oxidelake_core::telemetry::OperatorSnapshot; use crate::model::DashboardModel; use crate::state::{AppState, Panel}; @@ -172,6 +173,7 @@ fn render_inspector(state: &AppState, model: &DashboardModel, frame: &mut Frame< human_bytes(op.bytes_h2d), human_bytes(op.bytes_d2h) )), + fallback_line(op), ], (Some(node), None) => vec![Line::from(node.name.clone()), Line::from("no counters yet")], _ => vec![Line::from("no operator selected")], @@ -179,6 +181,33 @@ fn render_inspector(state: &AppState, model: &DashboardModel, frame: &mut Frame< frame.render_widget(Paragraph::new(lines).block(block), area); } +/// The CPU-fallback line (#32). +/// +/// A GPU deployment that silently runs everything on the CPU produces the +/// same rows and the same `EXPLAIN` tags as one that does not, so this is the +/// line that tells them apart. It is coloured rather than merely printed: +/// zero is the claim being made by the backend tag above it, and any other +/// number contradicts it. +fn fallback_line(op: &OperatorSnapshot) -> Line<'static> { + let style = if op.fell_back_entirely() { + Style::default().fg(Color::Red).add_modifier(Modifier::BOLD) + } else if op.fallback_batches > 0 { + Style::default().fg(Color::Yellow) + } else { + Style::default().fg(Color::Green) + }; + Line::from(vec![ + Span::raw("cpu fallback "), + Span::styled( + format!( + "{:>14}", + format!("{} / {}", op.fallback_batches, op.batches) + ), + style, + ), + ]) +} + fn ratio(used: u64, total: u64) -> f64 { if total == 0 { 0.0 diff --git a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap index cd53039..ab2440c 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap @@ -36,7 +36,7 @@ OxideLake data/t.parquet — SELECT k, sum(v), count(v) FROM t WHERE k >= 2 AND │latency/batch 4.100 ms ││ │ │memory 8.0 MiB ││ │ │H2D / D2H 0 B / 0 B ││ │ -│ ││ │ +│cpu fallback 0 / 100 ││ │ │ ││ │ │ ││ │ │ ││ │ diff --git a/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap b/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap index 1785287..8845a4c 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap @@ -34,7 +34,7 @@ expression: text "│latency/batch 1.840 ms ││ │" "│memory 16.0 MiB ││ │" "│H2D / D2H 12.4 MiB / 512 B ││ │" -"│ ││ │" +"│cpu fallback 0 / 100 ││ │" "│ ││ │" "│ ││ │" "│ ││ │" diff --git a/docs/architecture.md b/docs/architecture.md index aa6a638..ce303c7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -91,7 +91,9 @@ flowchart LR ## Telemetry and TUI -`TelemetryHub` (in `oxidelake-core`) is the only coupling between engine and dashboard; in v1 it observes the local process only. The TUI is a state machine with a pure `render(state, frame)`; repaints are bracketed in DEC 2026 synchronized updates. Four panels: Plan DAG with hardware tags, Inspector, Telemetry gauges, Describe (P25/P50/P99). Tested two ways: in-process with `ratatui::backend::TestBackend` + `insta`, and end-to-end with `termlens` driving the deterministic `oxidelake-tui-demo` binary in a real PTY ([ADR-0011](decisions/ADR-0011-tui-testing-layers.md)). +`TelemetryHub` (in `oxidelake-core`) is the only coupling between engine and dashboard; in v1 it observes the local process only. The TUI is a state machine with a pure `render(state, frame)`; repaints are bracketed in DEC 2026 synchronized updates. Four panels: Plan DAG with hardware tags, Inspector, Telemetry gauges, Describe (P25/P50/P99). + +The Inspector's `cpu fallback` line is the one counter that cannot be inferred from the others (#32). A GPU deployment that runs every batch on the CPU reference produces identical rows and identical `EXPLAIN` tags, so `fallback_batches / batches` is what tells the two apart; it is green at zero, yellow when some batches fell back and red when all of them did. The matching planner-side answer is the `placement notes` section `oxide explain` prints under the plan, naming each node the rule left on the CPU and why — a skipped node is an ordinary DataFusion operator in the plan above it, indistinguishable from one that was never eligible. Tested two ways: in-process with `ratatui::backend::TestBackend` + `insta`, and end-to-end with `termlens` driving the deterministic `oxidelake-tui-demo` binary in a real PTY ([ADR-0011](decisions/ADR-0011-tui-testing-layers.md)). ## Out of scope for v1 From d9b76a0b1ee4a4fecc2dde327974412a41752e6d Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:40:46 +0700 Subject: [PATCH 6/8] feat(observability): add query spans, a per-query log line and a metrics endpoint Closes #33 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- README.md | 4 +- .../oxidelake-compute/src/exec/aggregate.rs | 5 + crates/oxidelake-compute/src/exec/filter.rs | 7 + crates/oxidelake-compute/src/exec/join.rs | 4 + crates/oxidelake-compute/src/exec/mod.rs | 23 +++ crates/oxidelake-compute/src/exec/vector.rs | 6 + crates/oxidelake-core/src/telemetry.rs | 169 +++++++++++++++- crates/oxidelake-planner/src/codec.rs | 50 +++-- crates/oxidelake-runtime/Cargo.toml | 4 + crates/oxidelake-runtime/src/bin/oxide.rs | 11 +- .../src/bin/oxide_scheduler.rs | 47 +++++ .../oxidelake-runtime/src/bin/oxide_worker.rs | 45 +++++ crates/oxidelake-runtime/src/lib.rs | 2 + crates/oxidelake-runtime/src/metrics.rs | 184 ++++++++++++++++++ crates/oxidelake-runtime/src/session.rs | 59 +++++- crates/oxidelake-runtime/tests/cli.rs | 54 +++++ docs/architecture.md | 10 + 17 files changed, 662 insertions(+), 22 deletions(-) create mode 100644 crates/oxidelake-runtime/src/metrics.rs diff --git a/README.md b/README.md index 610e42d..20c4cdc 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,7 @@ error, never a silent fallback. | `--bind-host` | `127.0.0.1` | Address to bind. | | `--port` | `50050` | gRPC port. | | `--cluster-backend` | `$OXIDE_CLUSTER_BACKEND`, then `cpu` | The capability the cluster declares, which is what placement rewrites against. | +| `--metrics-port` / `--metrics-host` | off / `127.0.0.1` | As for the worker. A scheduler plans but does not execute, so its counters are empty; the endpoint exists so every process is scraped the same way. | ### `oxide-worker` @@ -197,6 +198,7 @@ error, never a silent fallback. | `--grpc-port` | `50052` | gRPC control port. | | `--concurrent-tasks` | available parallelism | Tasks run at once. | | `--work-dir` | a temporary directory | Where shuffle files go. | +| `--metrics-port` / `--metrics-host` | off / `127.0.0.1` | Serve Prometheus metrics at `/metrics` (needs `--features metrics`). Unauthenticated: bind it on a private interface. Without the feature the flag is refused, never ignored. | | `--backend` | `$OXIDE_BACKEND`, then detected | The backend this worker executes on. Selected before the first task, so it reaches the operators; a backend the machine cannot provide is a startup error. | ### Environment @@ -205,7 +207,7 @@ error, never a silent fallback. | --- | --- | --- | | `OXIDE_BACKEND` | every process that executes operators | Forces `cpu`, `cuda` or `metal` instead of the detected backend. `oxide-worker --backend` overrides it. | | `OXIDE_CLUSTER_BACKEND` | `oxide-scheduler` | The cluster's declared placement capability (default `cpu`: no GPU rewrites). `--cluster-backend` overrides it. | -| `RUST_LOG` | every binary | `tracing` filter; logs go to stderr, so they never mix into `--output json`. | +| `RUST_LOG` | every binary | `tracing` filter; logs go to stderr, so they never mix into `--output json`, and are uncoloured when stderr is not a terminal. `RUST_LOG=oxidelake_runtime=info` gives one line per query with the mode, rows, elapsed time and CPU fallbacks. | ## Repository map diff --git a/crates/oxidelake-compute/src/exec/aggregate.rs b/crates/oxidelake-compute/src/exec/aggregate.rs index d0f6561..4c576c2 100644 --- a/crates/oxidelake-compute/src/exec/aggregate.rs +++ b/crates/oxidelake-compute/src/exec/aggregate.rs @@ -203,12 +203,17 @@ impl ExecutionPlan for GpuAggregateExec { let input = Arc::clone(&self.input); let input_schema = input.schema(); let operator = self.config.operator("GpuAggregateExec")?; + let span = self + .config + .span("GpuAggregateExec", partition, operator.backend().kind()); let baseline = BaselineMetrics::new(&self.metrics, partition); let spec = self.spec.clone(); let schema = Arc::clone(&self.schema); let stream = futures::stream::once(async move { let batches = collect(input, context).await?; let all = concat_batches(&input_schema, &batches)?; + // Entered after the input is collected: the await is above it. + let _entered = span.enter(); let _timer = baseline.elapsed_compute().timer(); let out = operator.aggregate(&all, &spec)?; let out = RecordBatch::try_new(Arc::clone(&schema), out.columns().to_vec())?; diff --git a/crates/oxidelake-compute/src/exec/filter.rs b/crates/oxidelake-compute/src/exec/filter.rs index 4b86864..cb34943 100644 --- a/crates/oxidelake-compute/src/exec/filter.rs +++ b/crates/oxidelake-compute/src/exec/filter.rs @@ -162,10 +162,17 @@ impl ExecutionPlan for GpuFilterExec { ) -> Result { let input = self.input.execute(partition, context)?; let operator = self.config.operator("GpuFilterExec")?; + let span = self + .config + .span("GpuFilterExec", partition, operator.backend().kind()); let baseline = BaselineMetrics::new(&self.metrics, partition); let predicate = self.predicate.clone(); let projection = self.projection.clone(); let stream = input.map(move |batch| { + // The closure body is synchronous, so the guard is created and + // dropped inside one poll: entering a span across an await point + // is what `tracing` warns about, and there is none here. + let _entered = span.enter(); let batch = batch?; let _timer = baseline.elapsed_compute().timer(); let out = operator.filter_project(&batch, &predicate, &projection)?; diff --git a/crates/oxidelake-compute/src/exec/join.rs b/crates/oxidelake-compute/src/exec/join.rs index 0710703..b8c9146 100644 --- a/crates/oxidelake-compute/src/exec/join.rs +++ b/crates/oxidelake-compute/src/exec/join.rs @@ -265,6 +265,9 @@ impl ExecutionPlan for GpuHashJoinExec { ) -> Result { let left = self.left.execute(partition, Arc::clone(&context))?; let operator = self.config.operator("GpuHashJoinExec")?; + let span = self + .config + .span("GpuHashJoinExec", partition, operator.backend().kind()); let baseline = BaselineMetrics::new(&self.metrics, partition); let (left_key, right_key) = (self.left_key, self.right_key); let projection = self.projection.clone(); @@ -285,6 +288,7 @@ impl ExecutionPlan for GpuHashJoinExec { let stream = futures::stream::once(async move { let build = build_fut.await.map_err(DataFusionError::Shared)?; Ok::<_, DataFusionError>(left.map(move |batch| { + let _entered = span.enter(); let batch = batch?; let _timer = baseline.elapsed_compute().timer(); let out = operator.hash_join(&batch, &build, left_key, projection.as_deref())?; diff --git a/crates/oxidelake-compute/src/exec/mod.rs b/crates/oxidelake-compute/src/exec/mod.rs index ae6dd01..26e709c 100644 --- a/crates/oxidelake-compute/src/exec/mod.rs +++ b/crates/oxidelake-compute/src/exec/mod.rs @@ -68,6 +68,29 @@ impl ExecConfig { }); Ok(GpuOperator::new(backend, stats)?) } + + /// The span every batch of one partition is processed inside (#33). + /// + /// One span per `(operator, partition)`, entered around each batch rather + /// than one span per batch: a scan has thousands of batches and a span + /// each would drown the interesting fields. `target` is what the plan + /// says and `backend` is what is actually executing — the pair is the + /// whole question a reader of these logs is asking, and they differ + /// exactly when the CPU-fallback counter is moving. + pub(crate) fn span( + &self, + operator: &'static str, + partition: usize, + backend: BackendKind, + ) -> tracing::Span { + tracing::info_span!( + "oxide.operator", + operator, + partition, + target = %self.target, + backend = %backend, + ) + } } pub(crate) fn plan_properties( diff --git a/crates/oxidelake-compute/src/exec/vector.rs b/crates/oxidelake-compute/src/exec/vector.rs index 0aef0d2..09cdf91 100644 --- a/crates/oxidelake-compute/src/exec/vector.rs +++ b/crates/oxidelake-compute/src/exec/vector.rs @@ -195,12 +195,18 @@ impl ExecutionPlan for GpuVectorDistanceExec { ) -> Result { let input = self.input.execute(partition, context)?; let operator = self.config.operator("GpuVectorDistanceExec")?; + let span = self.config.span( + "GpuVectorDistanceExec", + partition, + operator.backend().kind(), + ); let baseline = BaselineMetrics::new(&self.metrics, partition); let column = self.column; let query = Arc::clone(&self.query); let metric = self.metric; let output_name = self.output_name.clone(); let stream = input.map(move |batch| { + let _entered = span.enter(); let batch = batch?; let _timer = baseline.elapsed_compute().timer(); let out = operator.vector_distance(&batch, column, &query, metric, &output_name)?; diff --git a/crates/oxidelake-core/src/telemetry.rs b/crates/oxidelake-core/src/telemetry.rs index dbae6a3..7f20dba 100644 --- a/crates/oxidelake-core/src/telemetry.rs +++ b/crates/oxidelake-core/src/telemetry.rs @@ -4,7 +4,7 @@ use std::collections::VecDeque; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; -use std::sync::{Arc, PoisonError, RwLock}; +use std::sync::{Arc, OnceLock, PoisonError, RwLock}; use std::time::Duration; use serde::{Deserialize, Serialize}; @@ -329,12 +329,27 @@ pub struct TelemetryHub { skips: RwLock>, } +/// The process-wide hub. +/// +/// Cluster executors do not build their operators — the plan arrives over the +/// wire and the codec rebuilds it — so there is no session object to hand a +/// hub to. This is that hub: the codec attaches it to every `Gpu*Exec` it +/// decodes, which is what makes a worker's `/metrics` describe the work the +/// worker actually did rather than an empty struct. Embedded sessions own +/// their own hub and never touch this one. +static GLOBAL: OnceLock> = OnceLock::new(); + impl TelemetryHub { /// Creates a shared hub. pub fn new() -> Arc { Arc::new(Self::default()) } + /// The process-wide hub (see [`GLOBAL`]). + pub fn global() -> &'static Arc { + GLOBAL.get_or_init(TelemetryHub::new) + } + /// Registers an operator and returns its live counters. Ids increase /// monotonically for the life of the hub; the oldest registrations are /// evicted once [`MAX_OPERATORS`] are held. @@ -445,10 +460,162 @@ pub struct TelemetrySnapshot { pub plan: Vec, } +impl TelemetrySnapshot { + /// Renders the snapshot as Prometheus text (`text/plain; version=0.0.4`). + /// + /// Written by hand rather than through a metrics crate: the counters are + /// already the right shape, the output is a dozen lines, and a scrape + /// endpoint is not worth a dependency tree in a query engine. Operator + /// counters carry `operator` and `backend` labels; the tier gauges and + /// spill counters carry none. + /// + /// Label values are escaped per the exposition format, because an + /// operator name is `GpuFilterExec` today and a user-supplied string the + /// moment someone adds one. + pub fn to_prometheus(&self) -> String { + let mut out = String::new(); + let counters: [OperatorCounter; 7] = [ + ("oxide_operator_rows_in_total", "Input rows.", |o| o.rows_in), + ("oxide_operator_rows_out_total", "Output rows.", |o| { + o.rows_out + }), + ("oxide_operator_batches_total", "Batches processed.", |o| { + o.batches + }), + ( + "oxide_operator_fallback_batches_total", + "Batches that ran on the CPU reference although the operator was placed on a device.", + |o| o.fallback_batches, + ), + ( + "oxide_operator_elapsed_nanoseconds_total", + "Processing time.", + |o| o.elapsed_ns, + ), + ( + "oxide_operator_bytes_h2d_total", + "Bytes copied host to device.", + |o| o.bytes_h2d, + ), + ( + "oxide_operator_bytes_d2h_total", + "Bytes copied device to host.", + |o| o.bytes_d2h, + ), + ]; + for (name, help, value) in counters { + out.push_str(&format!("# HELP {name} {help}\n# TYPE {name} counter\n")); + for op in &self.operators { + out.push_str(&format!( + "{name}{{operator=\"{}\",backend=\"{}\"}} {}\n", + escape_label(&op.name), + op.backend, + value(op) + )); + } + } + out.push_str("# HELP oxide_operator_memory_bytes Currently allocated bytes.\n"); + out.push_str("# TYPE oxide_operator_memory_bytes gauge\n"); + for op in &self.operators { + out.push_str(&format!( + "oxide_operator_memory_bytes{{operator=\"{}\",backend=\"{}\"}} {}\n", + escape_label(&op.name), + op.backend, + op.memory_bytes + )); + } + let gauges = [ + ("oxide_tier_device_bytes", self.tiers.device_bytes), + ("oxide_tier_host_bytes", self.tiers.host_bytes), + ("oxide_tier_disk_bytes", self.tiers.disk_bytes), + ]; + for (name, value) in gauges { + out.push_str(&format!( + "# HELP {name} Bytes resident in this memory tier.\n# TYPE {name} gauge\n{name} {value}\n" + )); + } + let spill = [ + ("oxide_spill_demotions_total", self.spill.demotions), + ("oxide_spill_promotions_total", self.spill.promotions), + ("oxide_spill_spilled_bytes_total", self.spill.spilled_bytes), + ( + "oxide_spill_reloaded_bytes_total", + self.spill.reloaded_bytes, + ), + ]; + for (name, value) in spill { + out.push_str(&format!( + "# HELP {name} Spill manager activity.\n# TYPE {name} counter\n{name} {value}\n" + )); + } + out + } +} + +/// One exported operator counter: metric name, HELP text, and how to read it +/// off a snapshot. +type OperatorCounter = (&'static str, &'static str, fn(&OperatorSnapshot) -> u64); + +/// Escapes a Prometheus label value: backslash, double quote, newline. +fn escape_label(value: &str) -> String { + let mut out = String::with_capacity(value.len()); + for c in value.chars() { + match c { + '\\' => out.push_str("\\\\"), + '"' => out.push_str("\\\""), + '\n' => out.push_str("\\n"), + other => out.push(other), + } + } + out +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn a_snapshot_renders_as_prometheus_text() { + let hub = TelemetryHub::new(); + let op = hub.register_operator("GpuFilterExec", BackendKind::Cuda); + op.record_batch(1_000, 400, Duration::from_millis(3)); + op.record_transfer(2_048, 512); + op.record_fallback(); + hub.tiers().set(MemoryTier::Disk, 4_096); + let text = hub.snapshot().to_prometheus(); + + assert!( + text.contains( + "oxide_operator_rows_in_total{operator=\"GpuFilterExec\",backend=\"cuda\"} 1000" + ), + "{text}" + ); + assert!( + text.contains("oxide_operator_fallback_batches_total{operator=\"GpuFilterExec\",backend=\"cuda\"} 1"), + "{text}" + ); + assert!(text.contains("oxide_tier_disk_bytes 4096"), "{text}"); + // Every metric is declared before it is used, which is what a scraper + // needs and what a hand-written exporter is most likely to forget. + for line in text.lines().filter(|l| !l.starts_with('#')) { + let name = line.split(['{', ' ']).next().unwrap_or_default(); + assert!( + text.contains(&format!("# TYPE {name} ")), + "{name} has no TYPE" + ); + } + } + + /// An operator name is a Rust type name today and could be anything + /// tomorrow; an unescaped quote would produce a file a scraper rejects. + #[test] + fn label_values_are_escaped() { + let hub = TelemetryHub::new(); + hub.register_operator("we\"ird\\name", BackendKind::CpuSimd); + let text = hub.snapshot().to_prometheus(); + assert!(text.contains("operator=\"we\\\"ird\\\\name\""), "{text}"); + } + #[test] fn operators_register_in_order_and_accumulate() { let hub = TelemetryHub::new(); diff --git a/crates/oxidelake-planner/src/codec.rs b/crates/oxidelake-planner/src/codec.rs index 88f17d2..5f8cb0b 100644 --- a/crates/oxidelake-planner/src/codec.rs +++ b/crates/oxidelake-planner/src/codec.rs @@ -19,6 +19,7 @@ use oxidelake_compute::{ GpuVectorDistanceExec, Predicate, }; use oxidelake_core::BackendKind; +use oxidelake_core::telemetry::TelemetryHub; use serde::{Deserialize, Serialize}; const MAGIC: &[u8; 4] = b"OXGP"; @@ -212,21 +213,31 @@ impl OxidePhysicalCodec { } /// Rebuilds a node from its serialized form and children. + /// + /// Every node is attached to the process-wide [`TelemetryHub`]. A cluster + /// executor never runs the placement rule — the plan arrives here already + /// lowered — so this is the only place its operators can be given + /// somewhere to report, and without it a worker's `/metrics` would + /// describe no work at all (#33). pub fn build( node: GpuNode, inputs: &[Arc], ) -> Result> { + let hub = TelemetryHub::global(); Ok(match node { GpuNode::Filter { target, predicate, projection, - } => Arc::new(GpuFilterExec::try_new( - Arc::clone(child(inputs, 0, "GpuFilterExec")?), - predicate, - projection, - target, - )?), + } => Arc::new( + GpuFilterExec::try_new( + Arc::clone(child(inputs, 0, "GpuFilterExec")?), + predicate, + projection, + target, + )? + .with_telemetry(Arc::clone(hub)), + ), GpuNode::HashJoin { target, left_key, @@ -240,7 +251,8 @@ impl OxidePhysicalCodec { right_key, target, )? - .with_projection(projection)?, + .with_projection(projection)? + .with_telemetry(Arc::clone(hub)), ), GpuNode::Aggregate { target, @@ -266,7 +278,10 @@ impl OxidePhysicalCodec { .zip(output) .map(|(f, (name, nullable))| Field::new(name, f.data_type().clone(), nullable)) .collect(); - Arc::new(exec.with_output_schema(Arc::new(Schema::new(fields)))?) + Arc::new( + exec.with_output_schema(Arc::new(Schema::new(fields)))? + .with_telemetry(Arc::clone(hub)), + ) } GpuNode::VectorDistance { target, @@ -274,14 +289,17 @@ impl OxidePhysicalCodec { query, metric, output_name, - } => Arc::new(GpuVectorDistanceExec::try_new( - Arc::clone(child(inputs, 0, "GpuVectorDistanceExec")?), - column, - query, - metric, - output_name, - target, - )?), + } => Arc::new( + GpuVectorDistanceExec::try_new( + Arc::clone(child(inputs, 0, "GpuVectorDistanceExec")?), + column, + query, + metric, + output_name, + target, + )? + .with_telemetry(Arc::clone(hub)), + ), }) } } diff --git a/crates/oxidelake-runtime/Cargo.toml b/crates/oxidelake-runtime/Cargo.toml index dc23caa..a4fa09f 100644 --- a/crates/oxidelake-runtime/Cargo.toml +++ b/crates/oxidelake-runtime/Cargo.toml @@ -33,6 +33,10 @@ cuda = ["oxidelake-device/cuda", "oxidelake-memory/cuda", "oxidelake-compute/cud metal = ["oxidelake-device/metal", "oxidelake-memory/metal", "oxidelake-compute/metal"] io-uring = ["oxidelake-storage/io-uring"] predict = ["oxidelake-compute/predict"] +# A Prometheus scrape endpoint on `oxide-worker`/`oxide-scheduler` +# `--metrics-port`. Off by default: it opens a port, and a port a deployment +# did not ask for is not something a query engine should open. +metrics = ["tokio/net", "tokio/io-util"] [dependencies] oxidelake-core.workspace = true diff --git a/crates/oxidelake-runtime/src/bin/oxide.rs b/crates/oxidelake-runtime/src/bin/oxide.rs index 9c0bb1f..0f10ca8 100644 --- a/crates/oxidelake-runtime/src/bin/oxide.rs +++ b/crates/oxidelake-runtime/src/bin/oxide.rs @@ -162,6 +162,10 @@ async fn main() -> anyhow::Result<()> { tracing_subscriber::fmt() .with_env_filter(EnvFilter::from_default_env()) .with_writer(std::io::stderr) + // Colour only when a person is reading. Logs are usually redirected + // to a file or a collector, where escape codes are noise that also + // breaks a grep for `field=value` (#33). + .with_ansi(std::io::IsTerminal::is_terminal(&std::io::stderr())) .init(); let cli = Cli::parse(); match cli.command { @@ -192,9 +196,10 @@ async fn main() -> anyhow::Result<()> { let session = session(cluster.as_deref(), target, batch_size, &tables).await?; // Not `DataFrame::show()`: that is `println!` inside DataFusion, // so a closed pipe panics there and the CLI never sees it (#34). - let frame = session.sql(&query).await?; - let schema = frame.schema().inner().clone(); - let batches = frame.collect().await?; + // `session.collect` rather than the DataFrame's: it logs the one + // INFO line per query that `RUST_LOG=info` is for (#33), and + // hands back the schema an empty result still has to print. + let (schema, batches) = session.collect(&query).await?; write_out(&oxidelake_runtime::output::render( &schema, &batches, output, )?)?; diff --git a/crates/oxidelake-runtime/src/bin/oxide_scheduler.rs b/crates/oxidelake-runtime/src/bin/oxide_scheduler.rs index d18bcce..02ea4a9 100644 --- a/crates/oxidelake-runtime/src/bin/oxide_scheduler.rs +++ b/crates/oxidelake-runtime/src/bin/oxide_scheduler.rs @@ -22,6 +22,14 @@ struct Cli { /// Defaults to OXIDE_CLUSTER_BACKEND, then cpu. #[arg(long)] cluster_backend: Option, + /// Serve Prometheus metrics on this port of `--metrics-host` (requires + /// the `metrics` feature). Unauthenticated: bind it on a private + /// interface, as the Ballista ports already are. + #[arg(long, value_name = "PORT")] + metrics_port: Option, + /// Address the metrics endpoint binds to. + #[arg(long, default_value = "127.0.0.1", value_name = "HOST")] + metrics_host: String, } #[tokio::main] @@ -29,6 +37,10 @@ async fn main() -> anyhow::Result<()> { tracing_subscriber::fmt() .with_env_filter(EnvFilter::from_default_env()) .with_writer(std::io::stderr) + // Colour only when a person is reading. Logs are usually redirected + // to a file or a collector, where escape codes are noise that also + // breaks a grep for `field=value` (#33). + .with_ansi(std::io::IsTerminal::is_terminal(&std::io::stderr())) .init(); let cli = Cli::parse(); let target: BackendKind = match cli.cluster_backend { @@ -36,6 +48,41 @@ async fn main() -> anyhow::Result<()> { None => oxidelake_planner::cluster_target_from_env()?, }; tracing::info!(%target, "placement target for this cluster"); + serve_metrics(cli.metrics_port, &cli.metrics_host).await?; cluster::run_scheduler(cluster::scheduler_config(&cli.bind_host, cli.port, target)).await?; Ok(()) } + +/// Starts the Prometheus endpoint when asked for (#33). +/// +/// A scheduler plans but does not execute, so its hub holds the counters of +/// whatever ran in *this* process — today, nothing. The endpoint is here so a +/// deployment can scrape every OxideLake process the same way rather than +/// special-casing one of them, and it says so by serving an empty set instead +/// of refusing. +#[cfg(feature = "metrics")] +async fn serve_metrics(port: Option, host: &str) -> anyhow::Result<()> { + use std::net::ToSocketAddrs; + + let Some(port) = port else { return Ok(()) }; + let addr = (host, port) + .to_socket_addrs()? + .next() + .ok_or_else(|| anyhow::anyhow!("--metrics-host '{host}' resolved to no address"))?; + let hub = std::sync::Arc::clone(oxidelake_core::telemetry::TelemetryHub::global()); + let (bound, server) = oxidelake_runtime::metrics::serve(addr, hub).await?; + tracing::info!(%bound, path = oxidelake_runtime::metrics::METRICS_PATH, "serving metrics"); + tokio::spawn(server); + Ok(()) +} + +#[cfg(not(feature = "metrics"))] +async fn serve_metrics(port: Option, _host: &str) -> anyhow::Result<()> { + match port { + None => Ok(()), + Some(_) => anyhow::bail!( + "--metrics-port needs the `metrics` feature; rebuild with \ + `cargo build -p oxidelake-runtime --features metrics`" + ), + } +} diff --git a/crates/oxidelake-runtime/src/bin/oxide_worker.rs b/crates/oxidelake-runtime/src/bin/oxide_worker.rs index 00077aa..8b03b4a 100644 --- a/crates/oxidelake-runtime/src/bin/oxide_worker.rs +++ b/crates/oxidelake-runtime/src/bin/oxide_worker.rs @@ -36,6 +36,14 @@ struct Cli { /// GPU capacity it does not have poisons the whole cluster's placement. #[arg(long, value_name = "BACKEND")] backend: Option, + /// Serve Prometheus metrics on this port of `--metrics-host` (requires + /// the `metrics` feature). Unauthenticated: bind it on a private + /// interface, as the Ballista ports already are. + #[arg(long, value_name = "PORT")] + metrics_port: Option, + /// Address the metrics endpoint binds to. + #[arg(long, default_value = "127.0.0.1", value_name = "HOST")] + metrics_host: String, } #[tokio::main] @@ -43,6 +51,10 @@ async fn main() -> anyhow::Result<()> { tracing_subscriber::fmt() .with_env_filter(EnvFilter::from_default_env()) .with_writer(std::io::stderr) + // Colour only when a person is reading. Logs are usually redirected + // to a file or a collector, where escape codes are noise that also + // breaks a grep for `field=value` (#33). + .with_ansi(std::io::IsTerminal::is_terminal(&std::io::stderr())) .init(); let cli = Cli::parse(); // Selected before anything runs: operators read the memoised @@ -53,6 +65,7 @@ async fn main() -> anyhow::Result<()> { None => oxidelake_compute::local_backend()?, }; tracing::info!(backend = %backend.kind(), "worker backend"); + serve_metrics(cli.metrics_port, &cli.metrics_host).await?; cluster::run_executor(cluster::executor_config( &cli.scheduler_host, cli.scheduler_port, @@ -64,3 +77,35 @@ async fn main() -> anyhow::Result<()> { .await?; Ok(()) } + +/// Starts the Prometheus endpoint when asked for, over the process-wide hub +/// the plan codec reports into (#33). +#[cfg(feature = "metrics")] +async fn serve_metrics(port: Option, host: &str) -> anyhow::Result<()> { + use std::net::ToSocketAddrs; + + let Some(port) = port else { return Ok(()) }; + let addr = (host, port) + .to_socket_addrs()? + .next() + .ok_or_else(|| anyhow::anyhow!("--metrics-host '{host}' resolved to no address"))?; + let hub = std::sync::Arc::clone(oxidelake_core::telemetry::TelemetryHub::global()); + let (bound, server) = oxidelake_runtime::metrics::serve(addr, hub).await?; + tracing::info!(%bound, path = oxidelake_runtime::metrics::METRICS_PATH, "serving metrics"); + tokio::spawn(server); + Ok(()) +} + +/// Without the feature the flag is still parsed, and refused rather than +/// ignored: a deployment that asked for metrics and silently got none is the +/// failure this whole issue is about. +#[cfg(not(feature = "metrics"))] +async fn serve_metrics(port: Option, _host: &str) -> anyhow::Result<()> { + match port { + None => Ok(()), + Some(_) => anyhow::bail!( + "--metrics-port needs the `metrics` feature; rebuild with \ + `cargo build -p oxidelake-runtime --features metrics`" + ), + } +} diff --git a/crates/oxidelake-runtime/src/lib.rs b/crates/oxidelake-runtime/src/lib.rs index 8b361a9..bbfa6f7 100644 --- a/crates/oxidelake-runtime/src/lib.rs +++ b/crates/oxidelake-runtime/src/lib.rs @@ -8,6 +8,8 @@ pub mod cluster; pub mod dashboard; +#[cfg(feature = "metrics")] +pub mod metrics; pub mod output; pub mod session; diff --git a/crates/oxidelake-runtime/src/metrics.rs b/crates/oxidelake-runtime/src/metrics.rs new file mode 100644 index 0000000..22f91a0 --- /dev/null +++ b/crates/oxidelake-runtime/src/metrics.rs @@ -0,0 +1,184 @@ +//! A Prometheus scrape endpoint for cluster processes (#33, feature `metrics`). +//! +//! A cluster executor's operators report into the process-wide +//! [`TelemetryHub`] — the plan codec attaches it to every `Gpu*Exec` it +//! decodes — and until now nothing could read those counters from outside the +//! process. `oxide-worker --metrics-port 9100` serves them at `/metrics`. +//! +//! ## Why this is hand-written +//! +//! The endpoint answers one path with one fixed content type and reads no +//! request body. An HTTP framework would be several hundred dependencies for +//! a response whose whole grammar is "GET /metrics". The parsing is therefore +//! deliberately narrow: the request line is read up to a bounded length, only +//! `GET /metrics` is answered, and anything else is a 404 or a 400. +//! +//! ## What it is not +//! +//! There is no authentication and no TLS, exactly as the Ballista ports +//! themselves have none (`STATUS.md`, "not addressed"). Bind it on a private +//! interface. It exposes operator names, backends and counts — no query text, +//! no data. + +use std::io; +use std::net::SocketAddr; +use std::sync::Arc; + +use oxidelake_core::telemetry::TelemetryHub; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; +use tokio::net::{TcpListener, TcpStream}; + +/// The path served. Anything else is a 404. +pub const METRICS_PATH: &str = "/metrics"; + +/// Longest request line accepted, in bytes. A scraper's is about 20; this is +/// the bound that stops a connection that never sends a newline from growing +/// a buffer without limit. +const MAX_REQUEST_LINE: u64 = 8 * 1024; + +/// Serves `hub` as Prometheus text on `addr` until the task is dropped. +/// +/// Returns once the listener is bound, so a caller can report the real port +/// (useful when `addr` asks for port 0) before the loop starts. +pub async fn serve( + addr: SocketAddr, + hub: Arc, +) -> io::Result<(SocketAddr, impl Future)> { + let listener = TcpListener::bind(addr).await?; + let local = listener.local_addr()?; + Ok((local, async move { + loop { + match listener.accept().await { + Ok((stream, peer)) => { + let hub = Arc::clone(&hub); + tokio::spawn(async move { + if let Err(e) = handle(stream, &hub).await { + tracing::debug!(%peer, error = %e, "metrics request failed"); + } + }); + } + // A failed accept is not a reason to stop serving metrics: + // the common causes (fd exhaustion, a peer that vanished + // between the SYN and the accept) are transient, and a + // metrics endpoint that silently stops is worse than one that + // logs and carries on. + Err(e) => tracing::warn!(error = %e, "metrics listener accept failed"), + } + } + })) +} + +async fn handle(stream: TcpStream, hub: &TelemetryHub) -> io::Result<()> { + let (read, mut write) = stream.into_split(); + let mut line = String::new(); + BufReader::new(read.take(MAX_REQUEST_LINE)) + .read_line(&mut line) + .await?; + let mut parts = line.split_whitespace(); + let response = match (parts.next(), parts.next()) { + (Some("GET"), Some(path)) if path.split('?').next() == Some(METRICS_PATH) => { + let body = hub.snapshot().to_prometheus(); + format!( + "HTTP/1.1 200 OK\r\n\ + Content-Type: text/plain; version=0.0.4; charset=utf-8\r\n\ + Content-Length: {}\r\n\ + Connection: close\r\n\r\n{body}", + body.len() + ) + } + (Some("GET"), Some(_)) => status(404, "Not Found", "only /metrics is served\n"), + (Some(_), Some(_)) => status(405, "Method Not Allowed", "only GET is served\n"), + _ => status(400, "Bad Request", "expected a request line\n"), + }; + write.write_all(response.as_bytes()).await?; + write.flush().await +} + +fn status(code: u16, reason: &str, body: &str) -> String { + format!( + "HTTP/1.1 {code} {reason}\r\n\ + Content-Type: text/plain; charset=utf-8\r\n\ + Content-Length: {}\r\n\ + Connection: close\r\n\r\n{body}", + body.len() + ) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use std::net::{IpAddr, Ipv4Addr}; + + use oxidelake_core::BackendKind; + + use super::*; + + async fn request(addr: SocketAddr, line: &str) -> String { + let mut stream = TcpStream::connect(addr).await.unwrap(); + stream.write_all(line.as_bytes()).await.unwrap(); + let mut response = String::new(); + stream.read_to_string(&mut response).await.unwrap(); + response + } + + #[tokio::test] + async fn metrics_are_served_and_other_paths_are_not() { + let hub = TelemetryHub::new(); + let op = hub.register_operator("GpuFilterExec", BackendKind::Cuda); + op.record_batch(100, 40, std::time::Duration::from_millis(1)); + + let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 0); + let (bound, server) = serve(addr, Arc::clone(&hub)).await.unwrap(); + let handle = tokio::spawn(server); + + let ok = request(bound, "GET /metrics HTTP/1.1\r\nHost: x\r\n\r\n").await; + assert!(ok.starts_with("HTTP/1.1 200 OK"), "{ok}"); + assert!(ok.contains("text/plain; version=0.0.4"), "{ok}"); + assert!( + ok.contains("oxide_operator_rows_in_total{operator=\"GpuFilterExec\""), + "{ok}" + ); + + // A scraper that appends a query string still gets the metrics. + let query = request(bound, "GET /metrics?x=1 HTTP/1.1\r\n\r\n").await; + assert!(query.starts_with("HTTP/1.1 200 OK"), "{query}"); + + let missing = request(bound, "GET /health HTTP/1.1\r\n\r\n").await; + assert!(missing.starts_with("HTTP/1.1 404"), "{missing}"); + + let post = request(bound, "POST /metrics HTTP/1.1\r\n\r\n").await; + assert!(post.starts_with("HTTP/1.1 405"), "{post}"); + + handle.abort(); + } + + /// The counters are live: a scrape after more work reports more work. + #[tokio::test] + async fn a_second_scrape_sees_new_counters() { + let hub = TelemetryHub::new(); + let op = hub.register_operator("GpuAggregateExec", BackendKind::Metal); + let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 0); + let (bound, server) = serve(addr, Arc::clone(&hub)).await.unwrap(); + let handle = tokio::spawn(server); + + let before = request(bound, "GET /metrics HTTP/1.1\r\n\r\n").await; + assert!( + before.contains( + "oxide_operator_batches_total{operator=\"GpuAggregateExec\",backend=\"metal\"} 0" + ), + "{before}" + ); + op.record_batch(10, 2, std::time::Duration::from_millis(1)); + op.record_fallback(); + let after = request(bound, "GET /metrics HTTP/1.1\r\n\r\n").await; + assert!( + after.contains( + "oxide_operator_batches_total{operator=\"GpuAggregateExec\",backend=\"metal\"} 1" + ), + "{after}" + ); + assert!(after.contains("oxide_operator_fallback_batches_total{operator=\"GpuAggregateExec\",backend=\"metal\"} 1"), "{after}"); + + handle.abort(); + } +} diff --git a/crates/oxidelake-runtime/src/session.rs b/crates/oxidelake-runtime/src/session.rs index bc9b018..9025295 100644 --- a/crates/oxidelake-runtime/src/session.rs +++ b/crates/oxidelake-runtime/src/session.rs @@ -1,9 +1,12 @@ //! The user-facing session: one query API over embedded and cluster execution. use std::sync::Arc; +use std::time::Instant; use ballista::prelude::SessionContextExt; use ballista_core::extension::SessionConfigExt; +use datafusion::arrow::array::RecordBatch; +use datafusion::arrow::datatypes::SchemaRef; use datafusion::dataframe::DataFrame; use datafusion::execution::SessionStateBuilder; use datafusion::physical_plan::displayable; @@ -35,6 +38,17 @@ pub enum SessionMode { }, } +impl std::fmt::Display for SessionMode { + /// `embedded/cuda` or `cluster/df://host:port` — one field in a log line + /// rather than two, because the second is only meaningful given the first. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SessionMode::Embedded { target } => write!(f, "embedded/{target}"), + SessionMode::Cluster { scheduler_url } => write!(f, "cluster/{scheduler_url}"), + } + } +} + /// Knobs a session is built with. /// /// Every field has a default that matches the no-argument constructors, so @@ -186,16 +200,59 @@ impl OxideSession { &self.ctx } - /// The telemetry hub for this session (local process only). + /// The telemetry hub for this session. + /// + /// **Embedded sessions only.** A cluster session's operators run on + /// executors, in other processes; this hub is created for the shape of + /// the type and stays empty, so a snapshot of it is not "no work + /// happened" but "the work happened somewhere else" (#33). Each worker + /// reports into its own process-wide hub, which `oxide-worker + /// --metrics-port` exposes as Prometheus text. pub fn telemetry(&self) -> &Arc { &self.telemetry } /// Plans a SQL statement into a lazily executed [`DataFrame`]. + /// + /// Nothing runs here, so nothing is logged here: see [`Self::collect`] + /// for the one-line-per-query record. pub async fn sql(&self, query: &str) -> Result { Ok(self.ctx.sql(query).await?) } + /// Runs `query` to completion and logs one `INFO` line describing it + /// (#33): the backend it was planned for, the rows it produced, how long + /// it took, and how many batches fell back to the CPU reference. + /// + /// This is the line an operator reads to answer "is the GPU being used + /// and how long did the query take" without attaching a dashboard. It is + /// on `collect` rather than on [`Self::sql`] because a `DataFrame` has + /// not run yet: a line logged at planning time could only report the + /// plan, and the interesting half is what the plan then did. + /// The schema comes back beside the batches because an empty result has + /// no batch to take it from, and a caller rendering CSV still has to + /// print the header. + pub async fn collect(&self, query: &str) -> Result<(SchemaRef, Vec), EngineError> { + let started = Instant::now(); + let frame = self.sql(query).await?; + let schema = SchemaRef::from(frame.schema().clone()); + let batches = frame.collect().await?; + let rows: usize = batches.iter().map(RecordBatch::num_rows).sum(); + let operators = self.telemetry.snapshot().operators; + let fallback_batches: u64 = operators.iter().map(|o| o.fallback_batches).sum(); + let accelerated = operators.iter().filter(|o| o.backend.is_gpu()).count(); + tracing::info!( + mode = %self.mode, + rows, + batches = batches.len(), + elapsed_ms = started.elapsed().as_secs_f64() * 1_000.0, + gpu_operators = accelerated, + fallback_batches, + "query finished" + ); + Ok((schema, batches)) + } + /// Registers a Parquet file or directory as `name`. pub async fn register_parquet(&self, name: &str, path: &str) -> Result<(), EngineError> { register_parquet_table(&self.ctx, name, path).await diff --git a/crates/oxidelake-runtime/tests/cli.rs b/crates/oxidelake-runtime/tests/cli.rs index 773f55c..2793c63 100644 --- a/crates/oxidelake-runtime/tests/cli.rs +++ b/crates/oxidelake-runtime/tests/cli.rs @@ -611,3 +611,57 @@ fn explain_names_why_a_node_stayed_on_the_cpu() { assert!(stdout.contains("GpuAggregateExec[cuda]"), "{stdout}"); assert!(!stdout.contains("placement notes"), "{stdout}"); } + +// --- observability (#33) ---------------------------------------------------- + +/// `RUST_LOG=info oxide sql …` prints one line per query with the mode, the +/// rows, the time and how much of it fell back to the CPU. +#[test] +fn a_query_logs_one_line_with_rows_and_elapsed() { + let dir = tempfile::tempdir().unwrap(); + gen_data(dir.path(), 4_096); + let table_arg = format!("t={}", dir.path().display()); + + let assert = oxide() + .env("RUST_LOG", "oxidelake_runtime=info") + .args([ + "sql", + "-q", + "SELECT k, SUM(v) FROM t WHERE k >= 2 GROUP BY k", + "--table", + &table_arg, + "--target", + "cuda", + ]) + .assert() + .success(); + let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); + assert!(stderr.contains("query finished"), "{stderr}"); + assert!(stderr.contains("mode=embedded/cuda"), "{stderr}"); + assert!(stderr.contains("rows="), "{stderr}"); + assert!(stderr.contains("elapsed_ms="), "{stderr}"); + assert!(stderr.contains("fallback_batches="), "{stderr}"); + + // The log goes to stderr, so it never mixes into `--output json`. + let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + assert!(!stdout.contains("query finished"), "{stdout}"); +} + +/// Without the `metrics` feature the flag is refused, not ignored. A worker +/// that was asked for metrics and quietly served none is the failure the +/// endpoint exists to prevent. +#[test] +#[cfg_attr(feature = "metrics", ignore = "the flag is honoured in this build")] +fn a_metrics_port_without_the_feature_is_refused() { + let assert = Command::cargo_bin("oxide-worker") + .unwrap() + .args(["--metrics-port", "19999"]) + .assert() + .failure(); + let stderr = String::from_utf8(assert.get_output().stderr.clone()).unwrap(); + assert!( + stderr.contains("--metrics-port needs the `metrics` feature"), + "{stderr}" + ); + assert!(stderr.contains("--features metrics"), "{stderr}"); +} diff --git a/docs/architecture.md b/docs/architecture.md index ce303c7..7fdd3e0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -93,6 +93,16 @@ flowchart LR `TelemetryHub` (in `oxidelake-core`) is the only coupling between engine and dashboard; in v1 it observes the local process only. The TUI is a state machine with a pure `render(state, frame)`; repaints are bracketed in DEC 2026 synchronized updates. Four panels: Plan DAG with hardware tags, Inspector, Telemetry gauges, Describe (P25/P50/P99). +### Observability + +Three surfaces, in increasing cost to the reader (#33): + +- **One `INFO` line per query**, from `OxideSession::collect`: `query finished mode=embedded/cuda rows=98 batches=1 elapsed_ms=3.9 gpu_operators=2 fallback_batches=2`. It is on `collect` and not on `sql`, because a `DataFrame` has not run yet. Logs go to stderr, so they never mix into `oxide sql --output json`, and ANSI is off unless stderr is a terminal. +- **One span per `(operator, partition)`**, `oxide.operator`, entered around each batch, with fields `operator`, `partition`, `target` (what the plan says) and `backend` (what is executing). The pair is the question these logs exist to answer; they differ exactly when the CPU-fallback counter is moving. `GpuOperator`'s own methods run inside it and add no span of their own — a nested span there would carry no field the outer one does not. +- **A Prometheus endpoint**, `oxide-worker --metrics-port` / `oxide-scheduler --metrics-port` (feature `metrics`): `oxide_operator_*` counters labelled by operator and backend, plus the tier gauges and spill counters. A cluster executor never runs the placement rule, so its operators are attached to a process-wide `TelemetryHub` by the plan codec; that hub is what is served. It is unauthenticated, like the Ballista ports themselves — bind it on a private interface. Without the feature the flag is refused rather than ignored. + +`OxideSession::telemetry()` is the embedded hub and stays empty for a cluster session: the work happened in other processes, and each of those has its own endpoint. + The Inspector's `cpu fallback` line is the one counter that cannot be inferred from the others (#32). A GPU deployment that runs every batch on the CPU reference produces identical rows and identical `EXPLAIN` tags, so `fallback_batches / batches` is what tells the two apart; it is green at zero, yellow when some batches fell back and red when all of them did. The matching planner-side answer is the `placement notes` section `oxide explain` prints under the plan, naming each node the rule left on the CPU and why — a skipped node is an ordinary DataFusion operator in the plan above it, indistinguishable from one that was never eligible. Tested two ways: in-process with `ratatui::backend::TestBackend` + `insta`, and end-to-end with `termlens` driving the deterministic `oxidelake-tui-demo` binary in a real PTY ([ADR-0011](decisions/ADR-0011-tui-testing-layers.md)). ## Out of scope for v1 From 753a263c436f4245801f1875235705a19b324e02 Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:47:47 +0700 Subject: [PATCH 7/8] fix(telemetry): report real tier capacities and say the spill manager is library-only Closes #25 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- README.md | 2 +- STATUS.md | 4 +- crates/oxidelake-core/src/telemetry.rs | 39 ++++++++++ crates/oxidelake-runtime/src/session.rs | 75 ++++++++++++++++++- crates/oxidelake-tui/src/model.rs | 11 ++- crates/oxidelake-tui/src/render.rs | 67 ++++++++++++----- ...renders_navigates_resizes_and_quits-2.snap | 6 +- ...renders_navigates_resizes_and_quits-3.snap | 8 +- ...o_renders_navigates_resizes_and_quits.snap | 6 +- ...e_first_frame_carries_its_colours_too.snap | 10 +-- ...tui_render_test__initial_frame_120x40.snap | 8 +- .../tui_render_test__initial_frame_80x24.snap | 6 +- ...est__selection_moves_and_panels_cycle.snap | 6 +- docs/architecture.md | 2 + 14 files changed, 202 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 20c4cdc..60a8e43 100644 --- a/README.md +++ b/README.md @@ -214,7 +214,7 @@ error, never a silent fallback. | Path | Purpose | |---|---| | [`crates/oxidelake-core`](crates/oxidelake-core) | `EngineError`, `BackendKind`, operator parameter types, `TelemetryHub` | -| [`crates/oxidelake-memory`](crates/oxidelake-memory) | 64/128-byte-aligned buffers, pinned/UMA allocators, 3-tier `SpillManager`, Arrow IPC codec | +| [`crates/oxidelake-memory`](crates/oxidelake-memory) | 64/128-byte-aligned buffers, pinned/UMA allocators, 3-tier `SpillManager` (a library: no query path registers batches with it in 0.x — see [architecture](docs/architecture.md#memory-model-and-spill-tiers)), Arrow IPC codec | | [`crates/oxidelake-device`](crates/oxidelake-device) | object-safe `GpuBackend`; CPU reference, CUDA (NVRTC) and Metal (MSL) backends; `HardwareDetector` | | [`crates/oxidelake-compute`](crates/oxidelake-compute) | the four `Gpu*Exec` operators, per-batch CPU fallback, `l2_distance`/`cosine_distance` UDFs, conformance suite | | [`crates/oxidelake-storage`](crates/oxidelake-storage) | Parquet writer/pruning config with *proofs*, Arrow IPC spill files, io_uring `ObjectStore` (Linux — implemented and conformance-tested, not yet used by sessions), `gen-data` generator | diff --git a/STATUS.md b/STATUS.md index 72ca7af..e2b14ed 100644 --- a/STATUS.md +++ b/STATUS.md @@ -13,7 +13,7 @@ Legend: `not started` · `in progress` · `done` · `blocked` | 3 | Kernels + oxidelake-compute operators | done | gate green; conformance suite `cargo test -p oxidelake-compute` (4 tests: CPU backend vs stock DataFusion over 3 seeds × 5 sizes × 5 checks, EXPLAIN tags, construction validation); `cargo test -p oxidelake-device --features cuda` 18 passed / 2 ignored | **Kernel execution verified only where a device exists**: the CUDA launch code type-checks under `--features cuda` but no kernel has been NVRTC-compiled or run here; the Metal dispatch is not compiled on Linux at all | | 4 | oxidelake-storage — Parquet pruning config, Arrow IPC spill files, io_uring ObjectStore | done | gate green incl. `cargo test -p oxidelake-storage --features io-uring`; pruning proof: statistics prune ≥90/100 row groups on `k = 42`, Bloom filters prune ≥90/100 on `id = ` with results equal to the pruning-disabled run; io_uring store passes the same behavioural suite as `LocalFileSystem` (executed here — `io_uring_setup` works in this sandbox); truncated/garbage Parquet → `EngineError::Format` | io_uring store: sequential ring worker (one SQE in flight) — correct, not yet tuned; **thrift CVE-2026-43868 via parquet 58 accepted with reason** — fix is parquet 59, blocked on Ballista > 54 (#4) | | 5 | oxidelake-planner (placement + codec), oxidelake-runtime on Ballista | done | gate green; placement tests (mocked GPU target: Filter/Projection fusion, HashJoin and SortMergeJoin lowering with DataFusion's build/probe order and pushed projections, Final/Partial aggregate pairs with DataFusion's output names; ineligible nodes untouched; CPU target = no rewrites); codec round-trips every node; **in-process Ballista scheduler + 2 executors return exactly the embedded results** on 4 queries with `Gpu*Exec` nodes in the scheduler's plans (executors take the CPU path); `oxide`, `oxide-scheduler`, `oxide-worker` print `--help` | Cluster `EXPLAIN` tags are asserted on the plan the scheduler's session builder produces (the client-side plan is Ballista's `DistributedQueryExec`); Ballista's shuffle/retry behaviour untouched | -| 6 | oxidelake-tui — dashboard, TestBackend + termlens tests | done | gate green; `tests/tui_render_test.rs`: `TestBackend` + `insta` snapshots at 80×24 and 120×40, selection/panel/quit transitions, determinism across 4 sizes; `tests/tui_pty_test.rs`: **termlens 0.10 drives the real `oxidelake-tui-demo` binary in a PTY** — initial screen, `↓` selection, `Tab`, `↓`, SIGWINCH resize to 120×40, `q` exits 0 (3 text snapshots + 1 styled), plus the backend-tag colours at every occurrence and `diff` proving `Tab` changes no text and `↓` leaves the right half alone; `tests/emulation.rs` pins `Screen::unsupported() == ["^[[59m"]`, the terminal modes and the snapshot-text/JSON round trips; `tests/termlens_cli.rs` reads the committed screens back through `termlens-cli` (`#[ignore]`d, CI runs it); `crates/oxidelake-runtime/tests/oxide_tui_pty.rs` does the same for the shipped `oxide tui`; 7 `.snap` files committed; no TTY needed | Dashboard observes the local process only; telemetry gauges use fixed reference capacities until backend `MemoryInfo` is wired in | +| 6 | oxidelake-tui — dashboard, TestBackend + termlens tests | done | gate green; `tests/tui_render_test.rs`: `TestBackend` + `insta` snapshots at 80×24 and 120×40, selection/panel/quit transitions, determinism across 4 sizes; `tests/tui_pty_test.rs`: **termlens 0.10 drives the real `oxidelake-tui-demo` binary in a PTY** — initial screen, `↓` selection, `Tab`, `↓`, SIGWINCH resize to 120×40, `q` exits 0 (3 text snapshots + 1 styled), plus the backend-tag colours at every occurrence and `diff` proving `Tab` changes no text and `↓` leaves the right half alone; `tests/emulation.rs` pins `Screen::unsupported() == ["^[[59m"]`, the terminal modes and the snapshot-text/JSON round trips; `tests/termlens_cli.rs` reads the committed screens back through `termlens-cli` (`#[ignore]`d, CI runs it); `crates/oxidelake-runtime/tests/oxide_tui_pty.rs` does the same for the shipped `oxide tui`; 7 `.snap` files committed; no TTY needed | Dashboard observes the local process only. Tier capacities now come from the local backend's `MemoryInfo` and a tier with no number is drawn without a ratio (#25); the tiers themselves are still only written by `SpillManager`, which no query path calls, and the panel says so | | 7 | oxidelake-api, end-to-end CLI, docs | done | gate green (default + `cuda` + `metal` lanes); UDF tests in `oxidelake-compute` (reference-equality, nulls, coercion errors); vector lowering in `oxidelake-planner/tests/placement.rs` incl. codec round-trip; DataFrame API vs SQL in `oxidelake-api/tests/dataframe.rs`; `oxidelake-runtime/tests/cli.rs` (assert_cmd): `gen-data` verified by an independent `parquet`-crate read, spawned `oxide-scheduler` + `oxide-worker` print byte-identical `--cluster` results, `explain --target` shows tags; `oxidelake-runtime/tests/embedded.rs`: cpu/cuda/metal targets return identical rows; README quickstart commands executed as written (1M rows, embedded + cluster + tui in a PTY) | PyO3 `python` feature deferred (see below); `oxide tui` without `--query` shows the synthetic demo fixture | ## Verification matrix @@ -24,7 +24,7 @@ Filled in as phases complete. Every component goes under exactly one column — |---|---|---|---| | `EngineError`, `BackendKind`, `BufferLayout`, `params`, `TelemetryHub` (oxidelake-core) | unit tests | | | | `AlignedBuf`, `HostAlloc` pinned→pageable fallback, Arrow IPC spill codec | unit tests | | | -| `SpillManager` host ↔ disk with kilobyte budgets, device tier via mocked `DeviceResident` | unit tests (tempdir + `LocalFileSystem`) | | real device residents (Phase 3+) | +| `SpillManager` host ↔ disk with kilobyte budgets, device tier via mocked `DeviceResident` | unit tests (tempdir + `LocalFileSystem`) | | real device residents (Phase 3+); **library only — no operator registers a batch with it, so the tier gauges and spill counters stay at zero in a running query (#25); the TUI says so rather than implying otherwise** | | `CpuBackend` + reference kernels (filter/project, hash join, aggregate, vector distance) | unit tests | | | | `HardwareDetector` (CPU selected; `cuda`/`metal` requests → typed errors) | unit tests; **(T4)** `OXIDE_BACKEND=cuda` and default detection both select `CudaBackend` | | | | `CudaBackend` (dynamic loading, NVRTC pipeline, transfers, upload/download) | driver probe + typed error on GPU-less box; **(T4)** `cargo test -p oxidelake-device --features cuda -- --include-ignored` — transfers, pinned memory round trip, NVRTC compile + launch all executed | | | diff --git a/crates/oxidelake-core/src/telemetry.rs b/crates/oxidelake-core/src/telemetry.rs index 7f20dba..898c070 100644 --- a/crates/oxidelake-core/src/telemetry.rs +++ b/crates/oxidelake-core/src/telemetry.rs @@ -265,6 +265,27 @@ impl SpillCounters { } } +/// What the engine knows about the size of each memory tier, and whether +/// anything on the query path fills them (#25). +/// +/// `None` is "this engine has no number", which is different from zero and +/// different from a plausible constant. The gauges used to be drawn against +/// fixed reference capacities, which made a dashboard that looked like a +/// bounded memory model over an engine that had none. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +pub struct TierCapacity { + /// Device memory the local backend reports, when it drives a device. + pub device_bytes: Option, + /// Host memory the local backend reports. + pub host_bytes: Option, + /// Whether a [`crate::telemetry::SpillCounters`] writer is on the query + /// path. `false` means the tier gauges and spill counters below can only + /// be moved by a caller using the spill manager as a library — no query + /// registers a batch with it — so a reading of zero says nothing about + /// the query's memory use. + pub spill_on_query_path: bool, +} + /// Snapshot of the spill counters. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] pub struct SpillSnapshot { @@ -327,6 +348,7 @@ pub struct TelemetryHub { spill: SpillCounters, plan: RwLock>, skips: RwLock>, + capacity: RwLock, } /// The process-wide hub. @@ -414,6 +436,20 @@ impl TelemetryHub { *self.plan.write().unwrap_or_else(PoisonError::into_inner) = nodes; } + /// Records what the backend says the tiers hold, and whether anything on + /// the query path fills them. + pub fn set_capacity(&self, capacity: TierCapacity) { + *self + .capacity + .write() + .unwrap_or_else(PoisonError::into_inner) = capacity; + } + + /// What was last recorded by [`Self::set_capacity`]. + pub fn capacity(&self) -> TierCapacity { + *self.capacity.read().unwrap_or_else(PoisonError::into_inner) + } + /// Memory tier gauges. pub const fn tiers(&self) -> &TierGauges { &self.tiers @@ -443,6 +479,7 @@ impl TelemetryHub { tiers: self.tiers.snapshot(), spill: self.spill.snapshot(), plan, + capacity: self.capacity(), } } } @@ -458,6 +495,8 @@ pub struct TelemetrySnapshot { pub spill: SpillSnapshot, /// The displayed plan. pub plan: Vec, + /// Tier capacities, and whether the query path fills the tiers at all. + pub capacity: TierCapacity, } impl TelemetrySnapshot { diff --git a/crates/oxidelake-runtime/src/session.rs b/crates/oxidelake-runtime/src/session.rs index 9025295..73eaa5d 100644 --- a/crates/oxidelake-runtime/src/session.rs +++ b/crates/oxidelake-runtime/src/session.rs @@ -12,7 +12,7 @@ use datafusion::execution::SessionStateBuilder; use datafusion::physical_plan::displayable; use datafusion::prelude::{SessionConfig, SessionContext}; use oxidelake_compute::{local_backend, oxide_udfs}; -use oxidelake_core::telemetry::TelemetryHub; +use oxidelake_core::telemetry::{TelemetryHub, TierCapacity}; use oxidelake_core::{BackendKind, EngineError}; use oxidelake_planner::{HardwarePlacementRule, physical_optimizer_rules}; use oxidelake_storage::{ @@ -38,6 +38,41 @@ pub enum SessionMode { }, } +/// What the local backend says its memory tiers hold (#25). +/// +/// A GPU backend's `memory_info` describes the device; the CPU backend's +/// describes host RAM. Neither is a guess, and a backend that cannot answer +/// leaves the field `None` rather than contributing a number the dashboard +/// would draw a gauge against. +/// +/// `spill_on_query_path` is `false` and stays false until an operator +/// registers a batch with a `SpillManager`. Nothing does in 0.2: the hash +/// join's build side is probed by every batch, so demoting it would trade a +/// bounded memory claim for an unbounded one in time, and a spilling +/// aggregate needs the streaming aggregate first. Saying so is the honest +/// half of this release; see `docs/architecture.md`. +fn local_capacity() -> TierCapacity { + let Ok(backend) = local_backend() else { + return TierCapacity::default(); + }; + let Ok(info) = backend.memory_info() else { + return TierCapacity::default(); + }; + if backend.kind().is_gpu() { + TierCapacity { + device_bytes: Some(info.total_bytes), + host_bytes: None, + spill_on_query_path: false, + } + } else { + TierCapacity { + device_bytes: None, + host_bytes: Some(info.total_bytes), + spill_on_query_path: false, + } + } +} + impl std::fmt::Display for SessionMode { /// `embedded/cuda` or `cluster/df://host:port` — one field in a log line /// rather than two, because the second is only meaningful given the first. @@ -136,6 +171,7 @@ impl OxideSession { None => local_backend()?.kind(), }; let telemetry = TelemetryHub::new(); + telemetry.set_capacity(local_capacity()); let rule = HardwarePlacementRule::new(target).with_telemetry(Arc::clone(&telemetry)); let config = options.apply(with_pruning(SessionConfig::new()), Some(target))?; let state = SessionStateBuilder::new() @@ -369,6 +405,43 @@ mod tests { assert!(err.contains("at least 1 row"), "{err}"); } + /// The dashboard's memory gauges are drawn against what the backend says, + /// and the spill flag stays false until a query path registers a batch — + /// which nothing does in 0.2 (#25). Both halves matter: a capacity that + /// was a constant and a gauge that could never move made the dashboard + /// describe a bounded memory model the engine does not have. + #[test] + fn a_session_reports_real_capacities_and_an_honest_spill_flag() { + let session = OxideSession::local().unwrap(); + let capacity = session.telemetry().capacity(); + assert!( + !capacity.spill_on_query_path, + "no operator registers with a SpillManager in 0.2" + ); + let SessionMode::Embedded { target } = session.mode() else { + panic!("local() is embedded"); + }; + let reported = if target.is_gpu() { + capacity.device_bytes + } else { + capacity.host_bytes + }; + // The number comes from the machine, so the assertion is on its + // shape: present, and not the 4 GiB constant the panel used to draw. + assert!( + reported.is_some_and(|bytes| bytes > 0), + "{target} reported no memory: {capacity:?}" + ); + // The other tier has no backend to ask, and says so rather than + // contributing a number. + let absent = if target.is_gpu() { + capacity.host_bytes + } else { + capacity.device_bytes + }; + assert_eq!(absent, None); + } + /// The no-argument constructors and `SessionOptions::default()` are the /// same session, so the builder cannot drift away from the plain one. #[test] diff --git a/crates/oxidelake-tui/src/model.rs b/crates/oxidelake-tui/src/model.rs index 84ae177..af329e2 100644 --- a/crates/oxidelake-tui/src/model.rs +++ b/crates/oxidelake-tui/src/model.rs @@ -3,7 +3,7 @@ use oxidelake_core::BackendKind; use oxidelake_core::telemetry::{ - OperatorSnapshot, PlanNodeSummary, SpillSnapshot, TelemetrySnapshot, TierSnapshot, + OperatorSnapshot, PlanNodeSummary, SpillSnapshot, TelemetrySnapshot, TierCapacity, TierSnapshot, }; /// Profiling summary of one column for the Describe panel. @@ -133,6 +133,15 @@ pub fn demo_model() -> DashboardModel { reloaded_bytes: 256 * 1024 * 1024, }, plan, + // The demo is what a spill manager on the query path would look + // like, which is why its capacities are real numbers and its + // flag is set. A live session reports what its backend says and + // leaves the flag false until a query registers a batch (#25). + capacity: TierCapacity { + device_bytes: Some(8 * 1024 * 1024 * 1024), + host_bytes: Some(4 * 1024 * 1024 * 1024), + spill_on_query_path: true, + }, }, profiles: vec![ ColumnProfile { diff --git a/crates/oxidelake-tui/src/render.rs b/crates/oxidelake-tui/src/render.rs index e20419a..94cfdc5 100644 --- a/crates/oxidelake-tui/src/render.rs +++ b/crates/oxidelake-tui/src/render.rs @@ -181,6 +181,29 @@ fn render_inspector(state: &AppState, model: &DashboardModel, frame: &mut Frame< frame.render_widget(Paragraph::new(lines).block(block), area); } +/// Says whether the three gauges above can move at all (#25). +/// +/// `SpillManager` is a library in 0.x: no query registers a batch with it, so +/// the tiers read zero however much memory a query used. A panel that showed +/// a bounded memory model the engine does not have would be a lie told in +/// colour, so it says which of the two it is showing. +fn spill_scope_line(on_query_path: bool) -> Line<'static> { + if on_query_path { + Line::from(vec![ + Span::raw("spill "), + Span::styled("on the query path", Style::default().fg(Color::Green)), + ]) + } else { + Line::from(vec![ + Span::raw("spill "), + Span::styled( + "library only: no query registers batches", + Style::default().fg(Color::DarkGray), + ), + ]) + } +} + /// The CPU-fallback line (#32). /// /// A GPU deployment that silently runs everything on the CPU produces the @@ -228,31 +251,38 @@ fn render_telemetry(state: &AppState, model: &DashboardModel, frame: &mut Frame< ]) .areas(inner); let tiers = &model.telemetry.tiers; - // Fixed reference capacities keep the gauges deterministic; real capacity - // arrives with backend MemoryInfo in a later release. - const VRAM_CAP: u64 = 8 * 1024 * 1024 * 1024; - const HOST_CAP: u64 = 4 * 1024 * 1024 * 1024; - const DISK_CAP: u64 = 2 * 1024 * 1024 * 1024; + let capacity = model.telemetry.capacity; + // Capacities come from the backend's `MemoryInfo`, and a tier the engine + // has no number for says so rather than being drawn against a plausible + // constant — which is what these gauges used to do (#25). The disk tier + // has no capacity at all: the spill directory's free space is the + // filesystem's business, not the engine's. for (area, label, used, cap, color) in [ - (vram, "VRAM", tiers.device_bytes, VRAM_CAP, Color::Green), - (host, "pinned RAM", tiers.host_bytes, HOST_CAP, Color::Cyan), ( - disk, - "disk spill", - tiers.disk_bytes, - DISK_CAP, - Color::Yellow, + vram, + "VRAM", + tiers.device_bytes, + capacity.device_bytes, + Color::Green, + ), + ( + host, + "host RAM", + tiers.host_bytes, + capacity.host_bytes, + Color::Cyan, ), + (disk, "disk spill", tiers.disk_bytes, None, Color::Yellow), ] { + let label = match cap { + Some(cap) => format!("{label}: {} / {}", human_bytes(used), human_bytes(cap)), + None => format!("{label}: {}", human_bytes(used)), + }; frame.render_widget( Gauge::default() .gauge_style(Style::default().fg(color)) - .ratio(ratio(used, cap)) - .label(format!( - "{label}: {} / {}", - human_bytes(used), - human_bytes(cap) - )), + .ratio(cap.map_or(0.0, |cap| ratio(used, cap))) + .label(label), area, ); } @@ -273,6 +303,7 @@ fn render_telemetry(state: &AppState, model: &DashboardModel, frame: &mut Frame< human_bytes(spill.reloaded_bytes), spill.promotions )), + spill_scope_line(model.telemetry.capacity.spill_on_query_path), ]), rates, ); diff --git a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-2.snap b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-2.snap index 60b0dff..a164b05 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-2.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-2.snap @@ -8,9 +8,9 @@ OxideLake data/t.parquet — SELECT k, sum(v), count(v) FROM t WHERE k >= 2 AND │ GpuAggregateExec [CUDA] ││█████████████████████████████ │ │▶ └─ GpuFilterExec [CUDA] ││███████VRAM: 6.0 GiB / 8.0 GiB │ │ └─ DataSourceExec [CPU] ││██████████████ │ -│ ││████pinned RAM: 1.5 GiB / 4.0 GiB │ -│ ││██████████ │ -│ ││███disk spill: 512.0 MiB / 2.0 GiB │ +│ ││█████host RAM: 1.5 GiB / 4.0 GiB │ +│ ││ │ +│ ││ disk spill: 512.0 MiB │ │ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │ │ ││NVMe spilled 3.0 GiB (42 demotions) │ │ │└──────────────────────────────────────┘ diff --git a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap index ab2440c..d41ca9a 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits-3.snap @@ -9,12 +9,12 @@ OxideLake data/t.parquet — SELECT k, sum(v), count(v) FROM t WHERE k >= 2 AND │ GpuAggregateExec [CUDA] ││████████████████████████████████████████████ │ │ └─ GpuFilterExec [CUDA] ││█████████████████VRAM: 6.0 GiB / 8.0 GiB ███ │ │▶ └─ DataSourceExec [CPU] ││██████████████████████ │ -│ ││██████████████pinned RAM: 1.5 GiB / 4.0 GiB │ -│ ││███████████████ │ -│ ││█████████████disk spill: 512.0 MiB / 2.0 GiB │ +│ ││███████████████host RAM: 1.5 GiB / 4.0 GiB │ +│ ││ │ +│ ││ disk spill: 512.0 MiB │ │ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │ │ ││NVMe spilled 3.0 GiB (42 demotions) reloaded 256.0 MiB (│ -│ ││ │ +│ ││spill on the query path │ │ ││ │ │ ││ │ │ ││ │ diff --git a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits.snap b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits.snap index 9f8977d..512f589 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__demo_renders_navigates_resizes_and_quits.snap @@ -8,9 +8,9 @@ OxideLake data/t.parquet — SELECT k, sum(v), count(v) FROM t WHERE k >= 2 AND │▶ GpuAggregateExec [CUDA] ││█████████████████████████████ │ │ └─ GpuFilterExec [CUDA] ││███████VRAM: 6.0 GiB / 8.0 GiB │ │ └─ DataSourceExec [CPU] ││██████████████ │ -│ ││████pinned RAM: 1.5 GiB / 4.0 GiB │ -│ ││██████████ │ -│ ││███disk spill: 512.0 MiB / 2.0 GiB │ +│ ││█████host RAM: 1.5 GiB / 4.0 GiB │ +│ ││ │ +│ ││ disk spill: 512.0 MiB │ │ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │ │ ││NVMe spilled 3.0 GiB (42 demotions) │ │ │└──────────────────────────────────────┘ diff --git a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__the_first_frame_carries_its_colours_too.snap b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__the_first_frame_carries_its_colours_too.snap index ece3766..4f92cc2 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_pty_test__the_first_frame_carries_its_colours_too.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_pty_test__the_first_frame_carries_its_colours_too.snap @@ -8,9 +8,9 @@ OxideLake data/t.parquet — SELECT k, sum(v), count(v) FROM t WHERE k >= 2 AND │▶ GpuAggregateExec [CUDA] ││█████████████████████████████ │ │ └─ GpuFilterExec [CUDA] ││███████VRAM: 6.0 GiB / 8.0 GiB │ │ └─ DataSourceExec [CPU] ││██████████████ │ -│ ││████pinned RAM: 1.5 GiB / 4.0 GiB │ -│ ││██████████ │ -│ ││███disk spill: 512.0 MiB / 2.0 GiB │ +│ ││█████host RAM: 1.5 GiB / 4.0 GiB │ +│ ││ │ +│ ││ disk spill: 512.0 MiB │ │ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │ │ ││NVMe spilled 3.0 GiB (42 demotions) │ │ │└──────────────────────────────────────┘ @@ -34,9 +34,9 @@ styles: 2: 0 fg=3 bold; 1-19 bg=8 bold; 20-25 fg=2 bg=8 bold; 26-38 bg=8 bold; 39 fg=3 bold; 41-78 fg=2 3: 0 fg=3 bold; 8-20 bold; 22-27 fg=2 bold; 39 fg=3 bold; 41-47 fg=2; 48-69 bg=2; 70-78 fg=2 4: 0 fg=3 bold; 10-23 bold; 25-29 fg=4 bold; 39 fg=3 bold; 41-78 fg=6 -5: 0 fg=3 bold; 39 fg=3 bold; 41-44 fg=6; 45-54 bg=6; 55-78 fg=6 +5: 0 fg=3 bold; 39 fg=3 bold; 41-45 fg=6; 46-54 bg=6; 55-78 fg=6 6: 0 fg=3 bold; 39 fg=3 bold; 41-78 fg=3 -7: 0 fg=3 bold; 39 fg=3 bold; 41-43 fg=3; 44-50 bg=3; 51-78 fg=3 +7: 0 fg=3 bold; 39 fg=3 bold; 41-78 fg=3 8: 0 fg=3 bold; 39 fg=3 bold 9: 0 fg=3 bold; 39 fg=3 bold 10: 0 fg=3 bold; 39 fg=3 bold diff --git a/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap b/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap index 8845a4c..5bee8d4 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_120x40.snap @@ -7,12 +7,12 @@ expression: text "│▶ GpuAggregateExec [CUDA] ││████████████████████████████████████████████ │" "│ └─ GpuFilterExec [CUDA] ││█████████████████VRAM: 6.0 GiB / 8.0 GiB ███ │" "│ └─ DataSourceExec [CPU] ││██████████████████████ │" -"│ ││██████████████pinned RAM: 1.5 GiB / 4.0 GiB │" -"│ ││███████████████ │" -"│ ││█████████████disk spill: 512.0 MiB / 2.0 GiB │" +"│ ││███████████████host RAM: 1.5 GiB / 4.0 GiB │" +"│ ││ │" +"│ ││ disk spill: 512.0 MiB │" "│ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │" "│ ││NVMe spilled 3.0 GiB (42 demotions) reloaded 256.0 MiB (│" -"│ ││ │" +"│ ││spill on the query path │" "│ ││ │" "│ ││ │" "│ ││ │" diff --git a/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_80x24.snap b/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_80x24.snap index 19aed82..6a68ee8 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_80x24.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_render_test__initial_frame_80x24.snap @@ -7,9 +7,9 @@ expression: text "│▶ GpuAggregateExec [CUDA] ││█████████████████████████████ │" "│ └─ GpuFilterExec [CUDA] ││███████VRAM: 6.0 GiB / 8.0 GiB │" "│ └─ DataSourceExec [CPU] ││██████████████ │" -"│ ││████pinned RAM: 1.5 GiB / 4.0 GiB │" -"│ ││██████████ │" -"│ ││███disk spill: 512.0 MiB / 2.0 GiB │" +"│ ││█████host RAM: 1.5 GiB / 4.0 GiB │" +"│ ││ │" +"│ ││ disk spill: 512.0 MiB │" "│ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │" "│ ││NVMe spilled 3.0 GiB (42 demotions) │" "│ │└──────────────────────────────────────┘" diff --git a/crates/oxidelake-tui/tests/snapshots/tui_render_test__selection_moves_and_panels_cycle.snap b/crates/oxidelake-tui/tests/snapshots/tui_render_test__selection_moves_and_panels_cycle.snap index de2e163..725e7ef 100644 --- a/crates/oxidelake-tui/tests/snapshots/tui_render_test__selection_moves_and_panels_cycle.snap +++ b/crates/oxidelake-tui/tests/snapshots/tui_render_test__selection_moves_and_panels_cycle.snap @@ -7,9 +7,9 @@ expression: text "│ GpuAggregateExec [CUDA] ││█████████████████████████████ │" "│ └─ GpuFilterExec [CUDA] ││███████VRAM: 6.0 GiB / 8.0 GiB │" "│▶ └─ DataSourceExec [CPU] ││██████████████ │" -"│ ││████pinned RAM: 1.5 GiB / 4.0 GiB │" -"│ ││██████████ │" -"│ ││███disk spill: 512.0 MiB / 2.0 GiB │" +"│ ││█████host RAM: 1.5 GiB / 4.0 GiB │" +"│ ││ │" +"│ ││ disk spill: 512.0 MiB │" "│ ││PCIe H2D 35.3 MiB D2H 18.6 MiB │" "│ ││NVMe spilled 3.0 GiB (42 demotions) │" "│ │└──────────────────────────────────────┘" diff --git a/docs/architecture.md b/docs/architecture.md index 7fdd3e0..91d28ba 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -66,6 +66,8 @@ flowchart LR - **The operator layer does not use it in 0.x.** `upload_bytes` copies from a pageable Arrow `Buffer` and `download_column` copies into a pageable `Vec`, so the operator path pays the staging copy that pinned memory exists to avoid; `alloc_pinned_host` is exercised only by the T4 device tests. Pinned staging for operator transfers is audit item P6 (#26), tracked in the roadmap's Phase 8. Any transfer-throughput number measured today is a pageable number. - On Apple Silicon, `MTLResourceStorageModeShared` buffers give CPU and GPU one physical allocation. - `SpillManager` holds per-tier budgets, demotes cold batches on high-watermark pressure, promotes on access, and exports metrics to the TUI. On a GPU-less machine the device tier is empty and the host ↔ disk path is fully testable. +- **It is a library, not part of the query path** (#25). No operator registers a batch with it, so in a running query the tier gauges and the spill counters stay at zero however much memory the query used — a reading of zero there says nothing about the query. The dashboard states this rather than implying otherwise: the Telemetry panel's last line reads `spill library only: no query registers batches`, and `TelemetrySnapshot::capacity.spill_on_query_path` is the flag it renders. The gauges' capacities now come from the local backend's `MemoryInfo` (the device's total for a GPU backend, host RAM for the CPU one) instead of the fixed constants they used to be drawn against, and a tier the engine has no number for is drawn without a ratio. +- Wiring it in is not a matter of calling it from the join. `GpuHashJoinExec`'s build side is probed by *every* batch, so demoting it would trade an unbounded memory claim for an unbounded time one, and promoting it once puts it straight back in host RAM; `GpuAggregateExec` collects its whole input before aggregating, so bounding it needs the streaming aggregate first. The honest version of this in 0.2 is the label; the engineering is Phase 8. ## Hardware abstraction From 8f2202801b5a000f95c57f4f27e362b0fb0eb17c Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:15:52 +0700 Subject: [PATCH 8/8] release: v0.2.0 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 138 ++++++++++++++++++++++++- Cargo.lock | 18 ++-- Cargo.toml | 20 ++-- README.md | 2 +- STATUS.md | 2 +- crates/oxidelake-api/src/lib.rs | 4 +- crates/oxidelake-core/src/telemetry.rs | 18 ++-- docs/roadmap.md | 71 ++++++++----- 8 files changed, 215 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 252a298..49b6609 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,141 @@ versions (0.x) may contain breaking changes; they are always listed under a ## [Unreleased] +## [0.2.0] - 2026-09-21 + +The observability release. A GPU deployment that silently ran everything on +the CPU used to produce identical rows, identical `EXPLAIN` tags and nothing +above `debug` — this release makes that visible, from a counter in the +dashboard to a Prometheus endpoint on every worker. + +**It does not complete the production-readiness milestone.** Ten lines of +[Phase 8](docs/roadmap.md) remain open; three of them (#28, #45, #46) need a +CUDA device to verify on and one (#29) is what the spill manager's real +wiring waits for. + +### Added + +- **The CPU-fallback counter** (#32). `fallback_batches` on `OperatorStats` + and `OperatorSnapshot` counts every batch that took the CPU reference path + although its operator was placed on a device. The TUI Inspector shows + `cpu fallback N / M` — green at zero, yellow when some batches fell back, + red when all of them did — and each operator logs one `warn!` on its first + fallback naming the reason. A silent fallback was previously detectable + only by re-running with `RUST_LOG=oxidelake_compute=debug`. + +- **Placement notes in `oxide explain`** (#32). A node the rule leaves on the + CPU is an ordinary DataFusion operator in the plan, indistinguishable from + one that was never eligible. `oxide explain` now prints, under the plan, + every node that was skipped and why — `AggregateExec: \`avg\` is not one of + the aggregates the device kernels implement (sum, count, min, max)`, `the + grouping key \`s\` is Utf8View; the device kernel groups on Int64`. A plan + that lowered completely prints nothing. + +- **Spans, a per-query log line and a metrics endpoint** (#33). An + `oxide.operator` span per operator and partition carrying `operator`, + `partition`, `target` (what the plan says) and `backend` (what is + executing). `OxideSession::collect` logs one `INFO` line per query with the + mode, rows, elapsed time and fallback count; `oxide sql` uses it, so + `RUST_LOG=oxidelake_runtime=info oxide sql …` prints one line per query. + Behind the new `metrics` feature, `oxide-worker --metrics-port` and + `oxide-scheduler --metrics-port` serve `oxide_*` counters as Prometheus + text. Cluster executors never run the placement rule, so the plan codec + attaches a process-wide `TelemetryHub` to every `Gpu*Exec` it decodes — + without that a worker's `/metrics` would describe no work at all. The + endpoint is unauthenticated, like the Ballista ports themselves: bind it on + a private interface. Without the feature `--metrics-port` is refused, never + ignored. + +- **`--batch-size`, `--output table|json|csv`, worker `--backend`** (#49). + `--batch-size` overrides the CPU default (8192) and the GPU-target default + (65536) on `sql`, `explain` and `tui`; zero is refused rather than silently + returning nothing. `--output json` writes an array of objects and + `--output csv` RFC 4180 with a header — an empty result still prints the + header and `[]`, so a script can tell "no rows" from "the query failed". + `oxide-worker --backend` mirrors `OXIDE_BACKEND` and is applied before the + first task, so it reaches the operators rather than being accepted and + ignored. Every flag and environment variable is now in one README table. + +- **`SessionOptions`** (#49), with `OxideSession::local_with_options` and + `connect_with_options`. `#[non_exhaustive]` with builder methods, so a knob + added later is a minor release. + +### Changed + +- **`predict` reads its activation from the model file** (#47). **Breaking + for existing model files.** A `safetensors` header declares + `{"__metadata__": {"oxidelake.activation": "relu"}}` — `relu`, `gelu`, + `sigmoid`, `tanh` or `none`, applied between the `Linear` layers and never + after the last. A file that declares nothing is refused with a message + naming the key and the alternative: `predict(path, features, 'relu')`, a + new third argument that must agree with the header when the file has one. + + Fixing this found a worse bug. The loader built a stack of bare `Linear` + layers and applied **no activation at all**, while the module docs, the + README and ADR-0015 all said "ReLU between them" — so every non-linear + model returned a linear model's numbers, and the only test of the numbers + compared them against the same bare stack. The `none` case is now checked + against oxmera's own `Sequential::forward`, which does not share the + loader's loop, and `relu`, `gelu` and `none` must disagree with each other. + +- **Public enums are `#[non_exhaustive]`** (#41). `BackendKind`, + `Compression`, `SessionMode`, `MemoryClass`, `MemoryTier`, `Tier`, + `ModelSpec`, `Activation` and the TUI's `Panel` / `KeyInput` / `Transition` + now need a `_` arm in a downstream `match`. `ModelSpec::SequentialMlpRelu` + is gone, replaced by `ModelSpec::SequentialMlp { activation }` (see + `predict`, below). The **plan vocabulary stays + exhaustive on purpose** — `Predicate`, `Comparison`, `Literal`, + `AggregateFunction`, `DistanceMetric` and the codec's `GpuNode`: every + kernel must handle every variant, so a `_` arm in the CPU correctness + reference is a wrong answer that compiles. `docs/RELEASING.md` records the + distinction, and the DataFusion coupling that `cargo-semver-checks` cannot + see. + +- **The plan codec carries a version fingerprint** (#42). `postcard` is not + self-describing, so a `GpuNode` field added or reordered between builds + decodes without complaint into the wrong parameters and a mixed-build fleet + computes a confident wrong answer. Every encoded plan now carries a magic, + a format version and a fingerprint over the crate version and the compute + layer's feature mask; a plan from a different build is refused at decode + with both fingerprints named. A committed snapshot pins the bytes of all + four variants, so a wire-format change cannot land unnoticed. + +- **Memory-tier gauges show what the backend reports** (#25). Capacities come + from the local backend's `MemoryInfo` — the device's total for a GPU + backend, host RAM for the CPU one — instead of the fixed 8/4/2 GiB + constants the gauges used to be drawn against, and a tier the engine has no + number for is drawn without a ratio rather than against a plausible one. + +- **The dashboard says the spill manager is library-only** (#25). + `SpillManager` is not on the query path: no operator registers a batch with + it, so the tier gauges and spill counters stay at zero however much memory + a query used. The Telemetry panel says so (`spill library only: no query + registers batches`), as do the README, `STATUS.md` and + `docs/architecture.md`. Wiring it in is not a matter of calling it from the + join — the build side is probed by every batch — and waits on the streaming + aggregate (#29). + +- **`OperatorSnapshot` and `TelemetrySnapshot` gained public fields** + (`fallback_batches`, `capacity`). Both are exhaustively constructible, so a + downstream struct literal over them no longer compiles. They are snapshot + types read field by field in practice, which is why they stay plain structs + rather than becoming `#[non_exhaustive]` — a reader should be able to + destructure one. + +- **An unavailable backend names the backend, not the way it was asked for.** + `oxide-worker --backend cuda` reaches the same code as `OXIDE_BACKEND`, so + the message no longer sends the reader to a variable they did not set. + +- **Logs are uncoloured when stderr is not a terminal.** ANSI escapes in a + redirected log file are noise, and they break a grep for `field=value`. + +### Fixed + +- **`oxide sql` prints the columns of an empty result.** The CSV header and + the JSON `[]` are written from the plan's schema, which an empty result + still has. + + ## [0.1.4] - 2026-09-20 ### Fixed @@ -362,7 +497,8 @@ Metal executes on Apple silicon (verified on an M4 Pro and on GitHub's macOS runners); CUDA compiles and lints without a CUDA installation but has not yet run on a CUDA machine. -[Unreleased]: https://github.com/vyncint/oxidelake/compare/v0.1.4...HEAD +[Unreleased]: https://github.com/vyncint/oxidelake/compare/v0.2.0...HEAD +[0.2.0]: https://github.com/vyncint/oxidelake/compare/v0.1.4...v0.2.0 [0.1.4]: https://github.com/vyncint/oxidelake/compare/v0.1.3...v0.1.4 [0.1.3]: https://github.com/vyncint/oxidelake/compare/v0.1.2...v0.1.3 [0.1.2]: https://github.com/vyncint/oxidelake/compare/v0.1.1...v0.1.2 diff --git a/Cargo.lock b/Cargo.lock index 8814bda..d8ebe6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4009,7 +4009,7 @@ checksum = "1a80800c0488c3a21695ea981a54918fbb37abf04f4d0720c453632255e2ff0e" [[package]] name = "oxidelake-api" -version = "0.1.4" +version = "0.2.0" dependencies = [ "datafusion", "oxidelake-core", @@ -4021,7 +4021,7 @@ dependencies = [ [[package]] name = "oxidelake-compute" -version = "0.1.4" +version = "0.2.0" dependencies = [ "arrow", "datafusion", @@ -4039,7 +4039,7 @@ dependencies = [ [[package]] name = "oxidelake-core" -version = "0.1.4" +version = "0.2.0" dependencies = [ "datafusion", "serde", @@ -4048,7 +4048,7 @@ dependencies = [ [[package]] name = "oxidelake-device" -version = "0.1.4" +version = "0.2.0" dependencies = [ "arrow", "cudarc", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "oxidelake-memory" -version = "0.1.4" +version = "0.2.0" dependencies = [ "arrow", "bytes", @@ -4080,7 +4080,7 @@ dependencies = [ [[package]] name = "oxidelake-planner" -version = "0.1.4" +version = "0.2.0" dependencies = [ "arrow", "datafusion", @@ -4096,7 +4096,7 @@ dependencies = [ [[package]] name = "oxidelake-runtime" -version = "0.1.4" +version = "0.2.0" dependencies = [ "anyhow", "arrow", @@ -4125,7 +4125,7 @@ dependencies = [ [[package]] name = "oxidelake-storage" -version = "0.1.4" +version = "0.2.0" dependencies = [ "arrow", "async-trait", @@ -4145,7 +4145,7 @@ dependencies = [ [[package]] name = "oxidelake-tui" -version = "0.1.4" +version = "0.2.0" dependencies = [ "crossterm", "insta", diff --git a/Cargo.toml b/Cargo.toml index 0eba4f3..df89a8a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = ["crates/*"] [workspace.package] -version = "0.1.4" +version = "0.2.0" edition = "2024" # The lockfile's floor (Ballista's bundled AWS SDK crates need 1.94.1), not # the 1.88 of the direct dependencies; verified by `make msrv` / the CI job. @@ -24,15 +24,15 @@ exclude = [".github/*", ".claude/*", "docs/*", "target/*"] [workspace.dependencies] # internal crates -oxidelake-core = { path = "crates/oxidelake-core", version = "0.1.4" } -oxidelake-memory = { path = "crates/oxidelake-memory", version = "0.1.4" } -oxidelake-device = { path = "crates/oxidelake-device", version = "0.1.4" } -oxidelake-compute = { path = "crates/oxidelake-compute", version = "0.1.4" } -oxidelake-storage = { path = "crates/oxidelake-storage", version = "0.1.4" } -oxidelake-planner = { path = "crates/oxidelake-planner", version = "0.1.4" } -oxidelake-runtime = { path = "crates/oxidelake-runtime", version = "0.1.4" } -oxidelake-tui = { path = "crates/oxidelake-tui", version = "0.1.4" } -oxidelake-api = { path = "crates/oxidelake-api", version = "0.1.4" } +oxidelake-core = { path = "crates/oxidelake-core", version = "0.2.0" } +oxidelake-memory = { path = "crates/oxidelake-memory", version = "0.2.0" } +oxidelake-device = { path = "crates/oxidelake-device", version = "0.2.0" } +oxidelake-compute = { path = "crates/oxidelake-compute", version = "0.2.0" } +oxidelake-storage = { path = "crates/oxidelake-storage", version = "0.2.0" } +oxidelake-planner = { path = "crates/oxidelake-planner", version = "0.2.0" } +oxidelake-runtime = { path = "crates/oxidelake-runtime", version = "0.2.0" } +oxidelake-tui = { path = "crates/oxidelake-tui", version = "0.2.0" } +oxidelake-api = { path = "crates/oxidelake-api", version = "0.2.0" } # data plane — Ballista 54 is the root of this chain ballista = "54" # client: SessionContext::remote / standalone diff --git a/README.md b/README.md index 60a8e43..7b74bbe 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A GPU-accelerated, Arrow-native distributed analytical query engine, written in Rust. It reads and writes Parquet on object storage; a catalog, a metastore and transactions are out of scope, so it is a query engine over a lake rather than a lakehouse in the full sense. -**Status: internal phases 0–7 complete; published 0.1.3. Production readiness is milestone v0.2.0.** The default build is pure CPU and the whole gate is green on Linux x86_64 and macOS arm64. Both GPU backends have executed on real hardware: **Metal** on Apple silicon (conformance-tested against stock DataFusion on an M4 Pro) and **CUDA** on an NVIDIA T4 since 2026-09-06, where every kernel was NVRTC-compiled and launched — and where a correctness bug in the aggregation kernel was found. [STATUS.md](STATUS.md) holds the verification matrix, component by component, and is the ledger ADR-0012 makes it. The specification the engine was built to is [docs/SPEC.md](docs/SPEC.md). +**Status: internal phases 0–7 complete; published 0.2.0. Production readiness is milestone v0.2.0, and 0.2.0 did not finish it** — ten [Phase 8](docs/roadmap.md) lines remain open, three of them needing a CUDA device to verify on. What 0.2.0 does add is the ability to tell what a deployment is actually doing: a CPU-fallback counter, placement notes under `oxide explain`, query spans, a per-query log line and a Prometheus endpoint. The default build is pure CPU and the whole gate is green on Linux x86_64 and macOS arm64. Both GPU backends have executed on real hardware: **Metal** on Apple silicon (conformance-tested against stock DataFusion on an M4 Pro) and **CUDA** on an NVIDIA T4 since 2026-09-06, where every kernel was NVRTC-compiled and launched — and where a correctness bug in the aggregation kernel was found. [STATUS.md](STATUS.md) holds the verification matrix, component by component, and is the ledger ADR-0012 makes it. The specification the engine was built to is [docs/SPEC.md](docs/SPEC.md). ## Install diff --git a/STATUS.md b/STATUS.md index e2b14ed..cdac18e 100644 --- a/STATUS.md +++ b/STATUS.md @@ -1,6 +1,6 @@ # Status -_Last updated: 2026-09-20 — **Internal phases 0–7 done; published 0.1.4 (2026-09-20). Production readiness is milestone v0.2.0.** "v1 complete" below and in earlier notes means the internal phase plan of `docs/SPEC.md`, not a 1.0 release; the published version is the one in `Cargo.toml`. 0.1.4 fixes three correctness bugs — an io_uring use-after-free (#27), ignored `GetOptions` preconditions (#44) and a panic on a closed pipe (#34) — and makes the README, SPEC and roadmap say what the code actually does. The repository moved to `vyncint/oxidelake` as a public project with a single-commit history (the private development history is `vyncint/oxidelake-old`); crates were renamed `oxide-*` → `oxidelake-*` because two of the old names were taken on crates.io, and the GPU kernel sources moved inside `oxidelake-device` so the published crate is self-contained (ADR-0016). Development moved from the Linux CPU box to a macOS machine (Apple M4 Pro, 24 GiB unified memory, Metal 4, Rust 1.98.0), where the whole default gate re-ran green and the Metal backend compiles, lints and **executes**; since 2026-09-06 an AWS `g4dn.xlarge` (NVIDIA Tesla T4) has executed the **CUDA** path too. Both GPU backends have now run on real hardware — the matrix below says which rows that covers. What stands between this and something to run in production is **Phase 8** of [docs/roadmap.md](docs/roadmap.md), milestone `v0.2.0`, which lists every open issue with what would close it._ +_Last updated: 2026-09-21 — **Internal phases 0–7 done; published 0.2.0 (2026-09-21). Production readiness is milestone v0.2.0, which 0.2.0 did not finish: ten Phase 8 lines remain open.** "v1 complete" below and in earlier notes means the internal phase plan of `docs/SPEC.md`, not a 1.0 release; the published version is the one in `Cargo.toml`. 0.2.0 is the observability release — a CPU-fallback counter and placement notes (#32), query spans, a per-query log line and a Prometheus endpoint (#33), the plan-codec fingerprint (#42), the CLI knobs (#49), `predict`'s activation header (#47, which also found that the loader applied no activation at all), the `non_exhaustive` policy (#41), and an honest account of the spill manager's scope (#25). 0.1.4 fixed three correctness bugs — an io_uring use-after-free (#27), ignored `GetOptions` preconditions (#44) and a panic on a closed pipe (#34) — and makes the README, SPEC and roadmap say what the code actually does. The repository moved to `vyncint/oxidelake` as a public project with a single-commit history (the private development history is `vyncint/oxidelake-old`); crates were renamed `oxide-*` → `oxidelake-*` because two of the old names were taken on crates.io, and the GPU kernel sources moved inside `oxidelake-device` so the published crate is self-contained (ADR-0016). Development moved from the Linux CPU box to a macOS machine (Apple M4 Pro, 24 GiB unified memory, Metal 4, Rust 1.98.0), where the whole default gate re-ran green and the Metal backend compiles, lints and **executes**; since 2026-09-06 an AWS `g4dn.xlarge` (NVIDIA Tesla T4) has executed the **CUDA** path too. Both GPU backends have now run on real hardware — the matrix below says which rows that covers. What stands between this and something to run in production is **Phase 8** of [docs/roadmap.md](docs/roadmap.md), milestone `v0.2.0`, which lists every open issue with what would close it._ Legend: `not started` · `in progress` · `done` · `blocked` diff --git a/crates/oxidelake-api/src/lib.rs b/crates/oxidelake-api/src/lib.rs index 61a68f4..a974db2 100644 --- a/crates/oxidelake-api/src/lib.rs +++ b/crates/oxidelake-api/src/lib.rs @@ -29,7 +29,7 @@ mod frame; pub use frame::{OxideFrame, OxideSessionExt}; pub use oxidelake_core::params::DistanceMetric; pub use oxidelake_core::{BackendKind, EngineError}; -pub use oxidelake_runtime::{OxideSession, SessionMode}; +pub use oxidelake_runtime::{OutputFormat, OxideSession, SessionMode, SessionOptions}; /// Commonly used imports: the session and frame types plus the DataFusion /// expression builders the verbs take. @@ -39,6 +39,6 @@ pub mod prelude { pub use crate::{ BackendKind, DistanceMetric, EngineError, OxideFrame, OxideSession, OxideSessionExt, - SessionMode, + SessionMode, SessionOptions, }; } diff --git a/crates/oxidelake-core/src/telemetry.rs b/crates/oxidelake-core/src/telemetry.rs index 898c070..98021b3 100644 --- a/crates/oxidelake-core/src/telemetry.rs +++ b/crates/oxidelake-core/src/telemetry.rs @@ -351,14 +351,7 @@ pub struct TelemetryHub { capacity: RwLock, } -/// The process-wide hub. -/// -/// Cluster executors do not build their operators — the plan arrives over the -/// wire and the codec rebuilds it — so there is no session object to hand a -/// hub to. This is that hub: the codec attaches it to every `Gpu*Exec` it -/// decodes, which is what makes a worker's `/metrics` describe the work the -/// worker actually did rather than an empty struct. Embedded sessions own -/// their own hub and never touch this one. +/// Backs [`TelemetryHub::global`]. static GLOBAL: OnceLock> = OnceLock::new(); impl TelemetryHub { @@ -367,7 +360,14 @@ impl TelemetryHub { Arc::new(Self::default()) } - /// The process-wide hub (see [`GLOBAL`]). + /// The process-wide hub. + /// + /// Cluster executors do not build their operators — the plan arrives over + /// the wire and the codec rebuilds it — so there is no session object to + /// hand a hub to. The codec attaches this one to every `Gpu*Exec` it + /// decodes, which is what makes a worker's `/metrics` describe the work + /// the worker actually did. Embedded sessions own their own hub and never + /// touch this one. pub fn global() -> &'static Arc { GLOBAL.get_or_init(TelemetryHub::new) } diff --git a/docs/roadmap.md b/docs/roadmap.md index 8f3cc2f..0195252 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -122,6 +122,16 @@ by issue number rather than kept in a second document that drifts. Every line names an open issue and what would close it. Ticked lines link the release that shipped them. +**0.2.0 did not finish this milestone.** It shipped the operability half — +the CPU-fallback counter and placement notes (#32), spans, a per-query log +line and a metrics endpoint (#33), the plan-codec fingerprint (#42), the CLI +knobs (#49), `predict`'s activation header (#47), the `non_exhaustive` / +semver policy (#41) — and settled the spill manager's scope honestly (#25). +Ten lines remain, and the three that matter most cannot be closed here: #28, +#45 and #46 need a CUDA device to verify on, and #29's streaming aggregate is +what #25's real wiring waits for. Do not read 0.2.0 as "production ready"; +read it as "a running deployment can now tell whether its GPU is being used". + ### Correctness - [x] io_uring worker reaps its CQE before returning (#27) — *Acceptance:* a @@ -143,53 +153,64 @@ the release that shipped them. memory completes. - [ ] CUDA `SUM` detects Int64 overflow instead of wrapping (#46) — *Acceptance:* an overflowing sum is an error, not a wrong number. -- [ ] `SpillManager` and `MemoryInfo` reach the operators, or the docs and - the TUI say they are library-only (#25). +- [x] `SpillManager` and `MemoryInfo` reach the operators, or the docs and + the TUI say they are library-only (#25) — *closed the second way*: + tier capacities come from the backend's `MemoryInfo`, and the panel, + README, STATUS and architecture say the spill manager is a library + that no query path calls. Wiring it in needs the streaming aggregate + (#29) first. **0.2.0** ### Performance -- [ ] Pinned staging for operator transfers (#26, audit P6) — - *Acceptance:* `Gpu*Exec` uploads and downloads through pinned memory, - with the before/after number recorded. +- [x] Pinned staging for operator transfers (#26, audit P6) — *closed as + documentation*: the operator path is pageable and says so; the + measurement and the change are Phase 9. **0.1.4** - [ ] Double-buffered stream pipelines (audit P6-rest) — *Acceptance:* transfer and compute overlap for a multi-batch scan, measured. - [ ] CUDA join table persists across probe batches (#45) — *Acceptance:* the build side is uploaded once per join, not once per batch. -- [ ] io_uring store wired into sessions, or the claim stays demoted (#24, - audit P12) — *Acceptance:* a test proves a Parquet scan goes through - the ring thread **and** the measurement says it is not slower. +- [x] io_uring store wired into sessions, or the claim stays demoted (#24, + audit P12) — *the claim stays demoted*: no session constructs it, and + every document says so. **0.1.4** ### Operability -- [ ] Per-batch CPU fallbacks and planner skip reasons are countable (#32) - — *Acceptance:* `fallback_batches` in the TUI Inspector; `EXPLAIN - VERBOSE` names why a shape stayed on the CPU. -- [ ] Tracing spans on the query path and an exportable metrics surface - (#33). +- [x] Per-batch CPU fallbacks and planner skip reasons are countable (#32) + — `fallback_batches` on `OperatorStats`, a `cpu fallback` line in the + TUI Inspector, one `warn!` per operator on first fallback, and + `oxide explain` printing `placement notes` naming every node the rule + left on the CPU and why. **0.2.0** +- [x] Tracing spans on the query path and an exportable metrics surface + (#33) — an `oxide.operator` span per operator and partition, one + `INFO` line per query from `OxideSession::collect`, and + `--metrics-port` on the worker and scheduler behind the `metrics` + feature. **0.2.0** - [ ] Worker loss, Ballista retry settings and `SIGTERM` handling (#31, audit P13). - [ ] TLS/auth for cluster mode, or an enforced private-network posture (#30) — *Acceptance:* the production cluster instructions do not describe an unauthenticated listener. -- [ ] Plan codec version fingerprint, failing fast on mixed builds (#42). -- [ ] `--batch-size`, `--output table|json|csv`, worker `--backend` (#49) — - *Acceptance:* every knob in one README table, each with an - `assert_cmd` test. -- [ ] `predict` reads its activation from the model header (#47) — - *Acceptance:* a header-less model is refused by name; a GELU model - works. +- [x] Plan codec version fingerprint, failing fast on mixed builds (#42). + **0.2.0** +- [x] `--batch-size`, `--output table|json|csv`, worker `--backend` (#49) — + every knob in one README table, each with an `assert_cmd` test. + **0.2.0** +- [x] `predict` reads its activation from the model header (#47) — a + header-less model is refused by name and a GELU model works. It also + found that the loader applied *no* activation at all while three + documents said ReLU. **0.2.0** ### Supply chain and release - [ ] Ballista 55 / DataFusion 55 / arrow 59 chain bump (#40) — blocks #4 and #54. - [ ] thrift advisory retired once parquet ≥ 59 lands (#4, #54). -- [ ] Public enums `non_exhaustive`, DataFusion-coupling semver policy - written (#41) — *Acceptance:* lands in a minor, not a patch. +- [x] Public enums `non_exhaustive`, DataFusion-coupling semver policy + written (#41) — landed in a minor, with the plan vocabulary left + exhaustive on purpose. **0.2.0** - [ ] On-demand CUDA execution job, made a release prerequisite (#38). -- [ ] CI build-cache policy decided and documented (#39) — *Acceptance:* - SECURITY.md, ADR-0016/0017 and the workflows say the same thing. -- [ ] termlens-cli installed prebuilt in CI (#48). +- [x] CI build-cache policy decided and documented (#39). **0.1.4** +- [x] termlens-cli installed prebuilt in CI (#48). **0.1.4** - [x] README, STATUS and CHANGELOG agree on CUDA execution and the published version (#36). **0.1.4** - [x] Page-index pruning proved beside statistics and Bloom filters (#43).