diff --git a/Cargo.lock b/Cargo.lock index d8660b1..21c5e69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -889,12 +889,6 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d8b14ccef22fc6f5a8f4d7d768562a182c04ce9a3b3157b91390b52ddfdf1a76" -[[package]] -name = "dyn-clone" -version = "1.0.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0881ea181b1df73ff77ffaaf9c7544ecc11e82fba9b5f27b262a3c73a332555" - [[package]] name = "ecolor" version = "0.33.3" @@ -2725,7 +2719,6 @@ dependencies = [ name = "photon" version = "0.1.1" dependencies = [ - "anyhow", "chrono", "crossbeam-channel", "lasso", @@ -2835,8 +2828,8 @@ dependencies = [ name = "photon-persist" version = "0.1.1" dependencies = [ - "anyhow", "bytes", + "crc32fast", "futures-util", "photon-core", "photon-protocol", @@ -2852,7 +2845,6 @@ dependencies = [ name = "photon-protocol" version = "0.1.1" dependencies = [ - "anyhow", "brotli", "bytes", "criterion", @@ -2885,6 +2877,7 @@ dependencies = [ name = "photon-server" version = "0.1.1" dependencies = [ + "anyhow", "photon", "photon-core", "photon-downsample", @@ -2908,7 +2901,6 @@ dependencies = [ name = "photon-store" version = "0.1.1" dependencies = [ - "anyhow", "chrono", "clickhouse", "dashmap", @@ -2935,7 +2927,6 @@ dependencies = [ name = "photon-transport" version = "0.1.1" dependencies = [ - "anyhow", "async-channel", "async-trait", "axum", @@ -2955,7 +2946,6 @@ dependencies = [ name = "photon-uplink" version = "0.1.1" dependencies = [ - "anyhow", "crossbeam-channel", "photon-core", "photon-transport", @@ -2969,10 +2959,8 @@ dependencies = [ name = "photon-wal" version = "0.1.1" dependencies = [ - "anyhow", "bytes", "crc32fast", - "dyn-clone", "photon-core", "serde", "serde_json", @@ -3248,7 +3236,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -4088,7 +4076,6 @@ dependencies = [ "bytes", "libc", "mio", - "parking_lot", "pin-project-lite", "signal-hook-registry", "socket2", diff --git a/Cargo.toml b/Cargo.toml index d0588cc..270292e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,8 +26,28 @@ license = "MIT OR Apache-2.0" repository = "https://github.com/tensaur/photon" publish = false +[workspace.lints.rust] +unreachable_pub = "warn" + +[workspace.lints.clippy] +all = { level = "warn", priority = -1 } +pedantic = { level = "warn", priority = -1 } +module_name_repetitions = "allow" +must_use_candidate = "allow" +return_self_not_must_use = "allow" +missing_errors_doc = "allow" +missing_panics_doc = "allow" +cast_possible_truncation = "allow" +cast_precision_loss = "allow" +cast_sign_loss = "allow" +cast_possible_wrap = "allow" +too_many_arguments = "allow" +too_many_lines = "allow" +type_complexity = "allow" + [workspace.dependencies] -tokio = { version = "1", features = ["full"] } +tokio = { version = "1", features = ["rt", "sync", "time", "macros", "signal"] } +tokio-util = { version = "0.7", features = ["rt"] } thiserror = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" @@ -65,7 +85,6 @@ wasm-bindgen-futures = "0.4" tracing-web = "0.1" httparse = "1" async-trait = "0.1" -dyn-clone = "1" clickhouse = { version = "0.13", features = ["uuid", "inserter"] } criterion = { version = "0.5", features = ["html_reports"] } diff --git a/crates/batch/Cargo.toml b/crates/batch/Cargo.toml index 16dc33a..45e301b 100644 --- a/crates/batch/Cargo.toml +++ b/crates/batch/Cargo.toml @@ -27,3 +27,6 @@ criterion.workspace = true [[bench]] name = "service" harness = false + +[lints] +workspace = true diff --git a/crates/batch/benches/service.rs b/crates/batch/benches/service.rs index 3d6b27f..cbfc441 100644 --- a/crates/batch/benches/service.rs +++ b/crates/batch/benches/service.rs @@ -6,7 +6,7 @@ use lasso::ThreadedRodeo; use photon_batch::domain::service::{BatchService, Service}; use photon_batch::domain::types::BatchStats; use photon_core::types::id::RunId; -use photon_core::types::metric::{MetricKey, RawPoint}; +use photon_core::types::metric::{MetricKey, RawPoint, Step}; use photon_core::types::sequence::SequenceNumber; use photon_protocol::codec::CodecKind; use photon_protocol::compressor::CompressorKind; @@ -35,7 +35,7 @@ fn make_raw_points(interner: &Arc, n: usize) -> Vec { .map(|i| RawPoint { key: keys[i % num_keys], value: 1.0 / (1.0 + i as f64 * 0.001), - step: i as u64, + step: Step::new(i as u64), timestamp_ns: i as u64 * 1_000_000, }) .collect() diff --git a/crates/batch/src/domain/assembler.rs b/crates/batch/src/domain/assembler.rs index 1b0b51f..8c0fa10 100644 --- a/crates/batch/src/domain/assembler.rs +++ b/crates/batch/src/domain/assembler.rs @@ -14,7 +14,7 @@ pub(crate) struct BatchAssembler { } impl BatchAssembler { - pub fn new(interner: Arc) -> Self { + pub(crate) fn new(interner: Arc) -> Self { Self { interner, key_map: HashMap::new(), @@ -23,7 +23,7 @@ impl BatchAssembler { } } - pub fn assemble(&mut self, run_id: RunId, pending: &[RawPoint]) -> MetricBatch { + pub(crate) fn assemble(&mut self, run_id: RunId, pending: &[RawPoint]) -> MetricBatch { self.key_map.clear(); self.keys.clear(); self.points.clear(); @@ -52,7 +52,7 @@ impl BatchAssembler { } } - pub fn reclaim(&mut self, batch: MetricBatch) { + pub(crate) fn reclaim(&mut self, batch: MetricBatch) { self.keys = batch.keys; self.points = batch.points; } diff --git a/crates/batch/src/inbound/thread.rs b/crates/batch/src/inbound/thread.rs index 803a93a..2be7afd 100644 --- a/crates/batch/src/inbound/thread.rs +++ b/crates/batch/src/inbound/thread.rs @@ -15,6 +15,7 @@ use photon_wal::WalAppender; use crate::domain::service::{BatchError, BatchService, Service}; use crate::domain::types::BatchStats; +#[allow(clippy::needless_pass_by_value)] pub fn run_batch_thread( run_id: RunId, interner: Arc, @@ -39,32 +40,29 @@ where loop { select! { recv(rx) -> msg => { - match msg { - Ok(point) => { - pending.push(point); + if let Ok(point) = msg { + pending.push(point); - while pending.len() < config.max_points { - match rx.try_recv() { - Ok(p) => pending.push(p), - Err(_) => break, - } - } - - if pending.len() >= config.max_points { - let wire = service.batch(&pending, &mut stats)?; - let _ = batch_tx.send(wire); - pending.clear(); + while pending.len() < config.max_points { + match rx.try_recv() { + Ok(p) => pending.push(p), + Err(_) => break, } } - Err(_) => { - if !pending.is_empty() { - let wire = service.batch(&pending, &mut stats)?; - let _ = batch_tx.send(wire); - pending.clear(); - } - return Ok(stats); + if pending.len() >= config.max_points { + let wire = service.batch(&pending, &mut stats)?; + let _ = batch_tx.send(wire); + pending.clear(); } + } else { + if !pending.is_empty() { + let wire = service.batch(&pending, &mut stats)?; + let _ = batch_tx.send(wire); + pending.clear(); + } + + return Ok(stats); } } diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 6dfc4d1..7169484 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -16,3 +16,6 @@ uuid.workspace = true [target.'cfg(target_arch = "wasm32")'.dependencies] uuid = { workspace = true, features = ["js"] } + +[lints] +workspace = true diff --git a/crates/core/src/domain/mod.rs b/crates/core/src/domain/mod.rs index cb08d28..0791907 100644 --- a/crates/core/src/domain/mod.rs +++ b/crates/core/src/domain/mod.rs @@ -2,3 +2,22 @@ pub mod experiment; pub mod project; pub mod run; pub mod tenant; + +use crate::types::id::{ExperimentId, ProjectId, RunId}; + +/// Marker trait for domain entities with a typed identifier. +pub trait Entity: Send + 'static { + type Id: Send + Sync; +} + +impl Entity for run::Run { + type Id = RunId; +} + +impl Entity for project::Project { + type Id = ProjectId; +} + +impl Entity for experiment::Experiment { + type Id = ExperimentId; +} diff --git a/crates/core/src/domain/run.rs b/crates/core/src/domain/run.rs index ad86430..890f3f3 100644 --- a/crates/core/src/domain/run.rs +++ b/crates/core/src/domain/run.rs @@ -3,20 +3,107 @@ use serde::{Deserialize, Serialize}; use crate::types::id::{ExperimentId, ProjectId, RunId, UserId}; -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct Run { - pub id: RunId, +/// Intent to create a new run. Separates user-provided fields from +/// system-generated fields (id, timestamps, status). +pub struct CreateRunRequest { + pub name: String, pub project_id: ProjectId, pub experiment_id: Option, pub user_id: UserId, - pub name: String, - pub status: RunStatus, pub tags: Vec, - pub created_at: DateTime, - pub updated_at: DateTime, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Run { + id: RunId, + project_id: ProjectId, + experiment_id: Option, + user_id: UserId, + name: String, + status: RunStatus, + tags: Vec, + created_at: DateTime, + updated_at: DateTime, } impl Run { + /// Create a new run from a request. + pub fn create(req: CreateRunRequest) -> Self { + let now = Utc::now(); + Self { + id: RunId::new(), + project_id: req.project_id, + experiment_id: req.experiment_id, + user_id: req.user_id, + name: req.name, + status: RunStatus::Created, + tags: req.tags, + created_at: now, + updated_at: now, + } + } + + /// Reconstruct a run from persisted data. + pub fn restore( + id: RunId, + project_id: ProjectId, + experiment_id: Option, + user_id: UserId, + name: String, + status: RunStatus, + tags: Vec, + created_at: DateTime, + updated_at: DateTime, + ) -> Self { + Self { + id, + project_id, + experiment_id, + user_id, + name, + status, + tags, + created_at, + updated_at, + } + } + + pub fn id(&self) -> RunId { + self.id + } + + pub fn project_id(&self) -> ProjectId { + self.project_id + } + + pub fn experiment_id(&self) -> Option { + self.experiment_id + } + + pub fn user_id(&self) -> UserId { + self.user_id + } + + pub fn name(&self) -> &str { + &self.name + } + + pub fn status(&self) -> &RunStatus { + &self.status + } + + pub fn tags(&self) -> &[String] { + &self.tags + } + + pub fn created_at(&self) -> DateTime { + self.created_at + } + + pub fn updated_at(&self) -> DateTime { + self.updated_at + } + pub fn start(&mut self) -> Result<(), RunStatusError> { match self.status { RunStatus::Created => { @@ -81,3 +168,90 @@ pub enum RunStatusError { #[error("cannot transition from {from:?} to {to:?}")] InvalidTransition { from: RunStatus, to: RunStatus }, } + +#[cfg(test)] +mod tests { + use super::*; + use crate::types::id::{ProjectId, UserId}; + + fn make_run(status: RunStatus) -> Run { + let mut run = Run::create(CreateRunRequest { + name: "test-run".to_string(), + project_id: ProjectId::new(), + experiment_id: None, + user_id: UserId::new(), + tags: vec![], + }); + run.status = status; + run + } + + #[test] + fn test_start_from_created() { + let mut run = make_run(RunStatus::Created); + assert!(run.start().is_ok()); + assert_eq!(*run.status(), RunStatus::Running); + } + + #[test] + fn test_start_from_running_fails() { + let mut run = make_run(RunStatus::Running); + assert!(run.start().is_err()); + } + + #[test] + fn test_finish_from_running() { + let mut run = make_run(RunStatus::Running); + assert!(run.finish().is_ok()); + assert_eq!(*run.status(), RunStatus::Finished); + } + + #[test] + fn test_finish_from_created_fails() { + let mut run = make_run(RunStatus::Created); + assert!(run.finish().is_err()); + } + + #[test] + fn test_fail_from_created() { + let mut run = make_run(RunStatus::Created); + assert!(run.fail("boom".to_string()).is_ok()); + assert_eq!( + *run.status(), + RunStatus::Failed { + reason: "boom".to_string() + } + ); + } + + #[test] + fn test_fail_from_running() { + let mut run = make_run(RunStatus::Running); + assert!(run.fail("timeout".to_string()).is_ok()); + assert_eq!( + *run.status(), + RunStatus::Failed { + reason: "timeout".to_string() + } + ); + } + + #[test] + fn test_fail_from_finished_fails() { + let mut run = make_run(RunStatus::Finished); + assert!(run.fail("late error".to_string()).is_err()); + } + + #[test] + fn test_is_active() { + assert!(make_run(RunStatus::Created).is_active()); + assert!(make_run(RunStatus::Running).is_active()); + assert!(!make_run(RunStatus::Finished).is_active()); + assert!( + !make_run(RunStatus::Failed { + reason: "err".to_string() + }) + .is_active() + ); + } +} diff --git a/crates/core/src/types/bucket.rs b/crates/core/src/types/bucket.rs index 68a3c73..7ead7aa 100644 --- a/crates/core/src/types/bucket.rs +++ b/crates/core/src/types/bucket.rs @@ -1,9 +1,9 @@ -use crate::types::metric::Metric; +use crate::types::metric::{Metric, Step}; #[derive(Clone, Debug, PartialEq)] pub struct Bucket { - pub step_start: u64, - pub step_end: u64, + pub step_start: Step, + pub step_end: Step, pub value: f64, pub min: f64, pub max: f64, diff --git a/crates/core/src/types/config.rs b/crates/core/src/types/config.rs index f05a4da..01ca6eb 100644 --- a/crates/core/src/types/config.rs +++ b/crates/core/src/types/config.rs @@ -69,7 +69,7 @@ impl Default for RetryConfig { } /// Controls how aggressively the WAL fsyncs to disk. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub enum WalSyncPolicy { /// fsync after every batch write. Maximum durability, ~10% throughput cost. EveryBatch, @@ -78,15 +78,10 @@ pub enum WalSyncPolicy { Periodic { batches: u32, interval: Duration }, /// Let the OS decide. Maximum throughput, weakest guarantee. /// Survives process crashes but not power loss. + #[default] OsManaged, } -impl Default for WalSyncPolicy { - fn default() -> Self { - Self::OsManaged - } -} - /// Configuration for the WAL storage backend. #[derive(Clone, Debug)] pub struct WalConfig { diff --git a/crates/core/src/types/error.rs b/crates/core/src/types/error.rs new file mode 100644 index 0000000..d6639d0 --- /dev/null +++ b/crates/core/src/types/error.rs @@ -0,0 +1,26 @@ +use std::fmt; + +use serde::{Deserialize, Serialize}; + +/// Transport-level error returned to clients. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub enum ApiError { + /// The requested resource was not found. + NotFound { message: String }, + /// The request was invalid. + BadRequest { message: String }, + /// An internal error occurred. Details are logged server-side. + Internal, +} + +impl fmt::Display for ApiError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NotFound { message } => write!(f, "not found: {message}"), + Self::BadRequest { message } => write!(f, "bad request: {message}"), + Self::Internal => write!(f, "internal error"), + } + } +} + +impl std::error::Error for ApiError {} diff --git a/crates/core/src/types/ingest.rs b/crates/core/src/types/ingest.rs index 3d86dbf..a138027 100644 --- a/crates/core/src/types/ingest.rs +++ b/crates/core/src/types/ingest.rs @@ -5,6 +5,7 @@ use crate::domain::project::Project; use crate::domain::run::Run; use crate::types::ack::AckResult; use crate::types::batch::WireBatch; +use crate::types::error::ApiError; use crate::types::id::{ExperimentId, ProjectId, RunId}; use crate::types::sequence::SequenceNumber; @@ -26,5 +27,5 @@ pub enum IngestResult { ExperimentRegistered(ExperimentId), ProjectRegistered(ProjectId), Watermark(SequenceNumber), - Error(String), + Error(ApiError), } diff --git a/crates/core/src/types/metric.rs b/crates/core/src/types/metric.rs index dfb88f3..cdac85d 100644 --- a/crates/core/src/types/metric.rs +++ b/crates/core/src/types/metric.rs @@ -65,7 +65,7 @@ impl<'de> Deserialize<'de> for Metric { pub struct MetricPoint { pub key_index: u32, pub value: f64, - pub step: u64, + pub step: Step, pub timestamp_ms: u64, } @@ -90,6 +90,55 @@ impl MetricBatch { } } +/// Training step or iteration number. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +pub struct Step(u64); + +impl Step { + pub const ZERO: Self = Self(0); + pub const MAX: Self = Self(u64::MAX); + + pub fn new(value: u64) -> Self { + Self(value) + } + + pub fn as_u64(self) -> u64 { + self.0 + } +} + +impl From for Step { + fn from(value: u64) -> Self { + Self(value) + } +} + +impl From for u64 { + fn from(step: Step) -> Self { + step.0 + } +} + +impl std::ops::Add for Step { + type Output = Self; + fn add(self, rhs: u64) -> Self { + Self(self.0 + rhs) + } +} + +impl std::ops::Sub for Step { + type Output = u64; + fn sub(self, rhs: Self) -> u64 { + self.0 - rhs.0 + } +} + +impl std::fmt::Display for Step { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + /// Compact handle for an interned metric key. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct MetricKey(usize); @@ -109,6 +158,64 @@ impl MetricKey { pub struct RawPoint { pub key: MetricKey, pub value: f64, - pub step: u64, + pub step: Step, pub timestamp_ns: u64, } + +#[cfg(test)] +mod tests { + use super::*; + + fn metric(name: &str) -> Result { + Metric::new(name) + } + + #[test] + fn test_valid_metric_names() { + let valid = [ + "loss", + "train/loss", + "eval.accuracy", + "lr-schedule", + "layer_0/grad_norm", + "a", + "ABC123", + "train/epoch.2/loss-val", + ]; + for name in valid { + assert!(metric(name).is_ok(), "expected {name:?} to be valid"); + } + } + + #[test] + fn test_empty_metric_name_fails() { + assert!(matches!(metric(""), Err(MetricError::Empty))); + } + + #[test] + fn test_metric_name_too_long_fails() { + let long = "a".repeat(257); + assert!(matches!(metric(&long), Err(MetricError::TooLong(257)))); + + // exactly 256 should be fine + let at_limit = "b".repeat(256); + assert!(metric(&at_limit).is_ok()); + } + + #[test] + fn test_invalid_characters_fail() { + let invalid = [ + "has space", + "emoji\u{1F600}", + "colon:bad", + "semi;col", + "q?mark", + ]; + for name in invalid { + assert!( + matches!(metric(name), Err(MetricError::InvalidChar(_))), + "expected {name:?} to fail with InvalidChar" + ); + } + } +} diff --git a/crates/core/src/types/mod.rs b/crates/core/src/types/mod.rs index 1c67e01..aa47353 100644 --- a/crates/core/src/types/mod.rs +++ b/crates/core/src/types/mod.rs @@ -2,6 +2,7 @@ pub mod ack; pub mod batch; pub mod bucket; pub mod config; +pub mod error; pub mod id; pub mod ingest; pub mod metric; diff --git a/crates/core/src/types/query.rs b/crates/core/src/types/query.rs index 69a29f7..7d0cd14 100644 --- a/crates/core/src/types/query.rs +++ b/crates/core/src/types/query.rs @@ -5,14 +5,15 @@ use serde::{Deserialize, Serialize}; use crate::domain::experiment::Experiment; use crate::domain::project::Project; use crate::domain::run::Run; +use crate::types::error::ApiError; use crate::types::id::RunId; -use crate::types::metric::Metric; +use crate::types::metric::{Metric, Step}; #[derive(Clone, Debug, Serialize, Deserialize)] pub struct MetricQuery { pub run_id: RunId, pub key: Metric, - pub step_range: Range, + pub step_range: Range, pub target_points: usize, } @@ -44,20 +45,20 @@ impl SeriesData { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct DataPoint { - pub step: u64, + pub step: Step, pub value: f64, } impl From<&DataPoint> for [f64; 2] { fn from(p: &DataPoint) -> Self { - [p.step as f64, p.value] + [p.step.as_u64() as f64, p.value] } } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct RangePoint { - pub step_start: u64, - pub step_end: u64, + pub step_start: Step, + pub step_end: Step, pub min: f64, pub max: f64, } @@ -92,5 +93,5 @@ pub enum QueryResult { Metrics(Vec), Series(MetricSeries), BatchResponse(QueryResponse), - Error(String), + Error(ApiError), } diff --git a/crates/dashboard/Cargo.toml b/crates/dashboard/Cargo.toml index 60ef5c6..b12aff9 100644 --- a/crates/dashboard/Cargo.toml +++ b/crates/dashboard/Cargo.toml @@ -39,3 +39,6 @@ wasm-bindgen-futures.workspace = true web-sys = { version = "0.3", features = ["HtmlCanvasElement", "Window", "Location"] } tracing-subscriber.workspace = true tracing-web.workspace = true + +[lints] +workspace = true diff --git a/crates/dashboard/src/domain/service.rs b/crates/dashboard/src/domain/service.rs index c146f7a..20971c7 100644 --- a/crates/dashboard/src/domain/service.rs +++ b/crates/dashboard/src/domain/service.rs @@ -14,7 +14,7 @@ use crate::domain::error::{ use crate::domain::ports::{MetricQuerier, MetricSubscriber}; #[cfg(not(target_arch = "wasm32"))] -pub trait DashboardService: Send + Sync + 'static { +pub trait DashboardService: Clone + Send + Sync + 'static { fn list_runs(&self) -> impl Future, ListRunsError>> + Send; fn list_experiments( diff --git a/crates/dashboard/src/inbound/channel.rs b/crates/dashboard/src/inbound/channel.rs index cdf81a7..8cc65c9 100644 --- a/crates/dashboard/src/inbound/channel.rs +++ b/crates/dashboard/src/inbound/channel.rs @@ -54,8 +54,9 @@ pub enum Response { } #[cfg(not(target_arch = "wasm32"))] +#[allow(unreachable_pub)] mod platform { - use super::*; + use super::{Command, Response}; pub type CommandSender = tokio::sync::mpsc::UnboundedSender; pub type CommandReceiver = tokio::sync::mpsc::UnboundedReceiver; @@ -85,6 +86,7 @@ mod platform { } #[cfg(target_arch = "wasm32")] +#[allow(unreachable_pub)] mod platform { use super::*; @@ -115,11 +117,11 @@ mod platform { } } -pub use platform::{CommandSender, ResponseReceiver, send_cmd}; +pub(crate) use platform::{CommandSender, ResponseReceiver, send_cmd}; use platform::{channels, recv_cmd, send_resp, spawn}; type ResponseSender = platform::ResponseSender; -pub fn spawn_service( +pub(crate) fn spawn_service( ctx: egui::Context, service: S, subscription_transport: Option, diff --git a/crates/dashboard/src/inbound/ui/app.rs b/crates/dashboard/src/inbound/ui/app.rs index 02d69d7..572a197 100644 --- a/crates/dashboard/src/inbound/ui/app.rs +++ b/crates/dashboard/src/inbound/ui/app.rs @@ -6,7 +6,7 @@ use photon_core::domain::experiment::Experiment; use photon_core::domain::project::Project; use photon_core::domain::run::Run; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use photon_core::types::query::{MetricQuery, MetricSeries, SeriesData}; use crate::inbound::channel::{self, Command, CommandSender, Response, ResponseReceiver}; @@ -16,19 +16,15 @@ use super::sidebar::{self, SidebarAction, SidebarState}; use super::theme; use super::viewport::ViewportBehavior; +#[derive(Default)] pub enum RequestState { + #[default] Idle, Pending, Loaded(T), Failed(String), } -impl Default for RequestState { - fn default() -> Self { - Self::Idle - } -} - pub struct DataCache { pub runs: RequestState>, pub experiments: RequestState>, @@ -128,24 +124,20 @@ impl DashboardApp { self.cache.series.insert(key, series); } } - Response::BatchSeries { .. } => { - // Not used in v1 UI. - } + Response::BatchSeries { .. } | Response::SubscriptionEnded { .. } => {} Response::LivePoints { run_id, metric, points, } => { - if let Some(series) = self.cache.series.get_mut(&(run_id, metric)) { - if let SeriesData::Raw { + if let Some(series) = self.cache.series.get_mut(&(run_id, metric)) + && let SeriesData::Raw { points: ref mut existing, } = series.data - { - existing.extend(points); - } + { + existing.extend(points); } } - Response::SubscriptionEnded { .. } => {} Response::RunsChanged => { // Re-fetch the run and experiment lists self.cache.runs = RequestState::Pending; @@ -156,6 +148,7 @@ impl DashboardApp { } } + #[allow(clippy::needless_pass_by_value)] fn handle_sidebar_action(&mut self, action: SidebarAction) { match action { SidebarAction::SelectRun(run_id) => { @@ -191,12 +184,11 @@ impl DashboardApp { } fn subscribe_if_active(&self, run_id: RunId) { - if let RequestState::Loaded(runs) = &self.cache.runs { - if let Some(run) = runs.iter().find(|r| r.id == run_id) { - if run.is_active() { - channel::send_cmd(&self.commands, Command::Subscribe { run_id }); - } - } + if let RequestState::Loaded(runs) = &self.cache.runs + && let Some(run) = runs.iter().find(|r| r.id() == run_id) + && run.is_active() + { + channel::send_cmd(&self.commands, Command::Subscribe { run_id }); } } @@ -207,8 +199,8 @@ impl DashboardApp { } fn ensure_metrics_loaded(&mut self, run_id: RunId) { - if !self.cache.metrics.contains_key(&run_id) { - self.cache.metrics.insert(run_id, RequestState::Pending); + if let std::collections::hash_map::Entry::Vacant(e) = self.cache.metrics.entry(run_id) { + e.insert(RequestState::Pending); channel::send_cmd(&self.commands, Command::ListMetrics { run_id }); } } @@ -248,7 +240,7 @@ impl DashboardApp { query: MetricQuery { run_id: *run_id, key: m.clone(), - step_range: 0..u64::MAX, + step_range: Step::ZERO..Step::MAX, target_points: 1000, }, }, @@ -330,7 +322,7 @@ impl eframe::App for DashboardApp { RequestState::Failed(msg) => { ui.colored_label(theme::STATUS_FAILED, format!("Error: {msg}")); if ui.button("Retry").clicked() { - let _ = channel::send_cmd(&self.commands, Command::ListRuns); + let () = channel::send_cmd(&self.commands, Command::ListRuns); self.cache.runs = RequestState::Pending; } } diff --git a/crates/dashboard/src/inbound/ui/panes/line_chart.rs b/crates/dashboard/src/inbound/ui/panes/line_chart.rs index 8c68724..2799a95 100644 --- a/crates/dashboard/src/inbound/ui/panes/line_chart.rs +++ b/crates/dashboard/src/inbound/ui/panes/line_chart.rs @@ -18,6 +18,9 @@ pub fn show(ui: &mut egui::Ui, state: &LineChartState, cache: &DataCache) { format!("{}\nstep: {:.0}\nvalue: {:.6}", metric_name, pt.x, pt.y) }) .show(ui, |plot_ui| { - plot_ui.line(Line::new(state.metric.as_str(), PlotPoints::new(points.clone()))); + plot_ui.line(Line::new( + state.metric.as_str(), + PlotPoints::new(points.clone()), + )); }); } diff --git a/crates/dashboard/src/inbound/ui/sidebar.rs b/crates/dashboard/src/inbound/ui/sidebar.rs index 5b9d4bb..7a19bc9 100644 --- a/crates/dashboard/src/inbound/ui/sidebar.rs +++ b/crates/dashboard/src/inbound/ui/sidebar.rs @@ -85,7 +85,7 @@ pub fn show( let mut by_experiment: HashMap, Vec<&Run>> = HashMap::new(); for run in &filtered { by_experiment - .entry(run.experiment_id) + .entry(run.experiment_id()) .or_default() .push(run); } @@ -101,8 +101,7 @@ pub fn show( Some(id) => experiments .iter() .find(|e| e.id == id) - .map(|e| e.name.clone()) - .unwrap_or_else(|| format!("Experiment {}", id.short())), + .map_or_else(|| format!("Experiment {}", id.short()), |e| e.name.clone()), None => "Ungrouped".to_string(), }; let header = egui::CollapsingHeader::new(RichText::new(header_text).color(theme::TEXT_DIM)) @@ -110,8 +109,8 @@ pub fn show( .default_open(*expanded) .show(ui, |ui| { for run in exp_runs { - let is_selected = state.selected_runs.contains(&run.id); - let status_color = status_color(&run.status); + let is_selected = state.selected_runs.contains(&run.id()); + let status_color = status_color(run.status()); ui.horizontal(|ui| { // Status dot. @@ -119,7 +118,7 @@ pub fn show( ui.allocate_exact_size(egui::vec2(8.0, 8.0), egui::Sense::hover()); ui.painter().circle_filled(rect.center(), 4.0, status_color); - let label = RichText::new(&run.name).color(if is_selected { + let label = RichText::new(run.name()).color(if is_selected { theme::ACCENT } else { theme::TEXT_PRIMARY @@ -130,13 +129,13 @@ pub fn show( let modifier = ui.input(|i| i.modifiers.command); if modifier { // Ctrl/Cmd+click: toggle run in/out of selection - action = Some(SidebarAction::ToggleRun(run.id)); + action = Some(SidebarAction::ToggleRun(run.id())); } else if is_selected && state.selected_runs.len() == 1 { // Click on sole selected run: deselect action = Some(SidebarAction::ClearSelection); } else { // Normal click: replace selection with this run - action = Some(SidebarAction::SelectRun(run.id)); + action = Some(SidebarAction::SelectRun(run.id())); } } }); @@ -165,11 +164,10 @@ fn status_color(status: &RunStatus) -> Color32 { } fn matches_status(run: &Run, state: &SidebarState) -> bool { - match &run.status { - RunStatus::Running => state.show_running, + match run.status() { + RunStatus::Running | RunStatus::Created => state.show_running, RunStatus::Finished => state.show_finished, RunStatus::Failed { .. } => state.show_failed, - RunStatus::Created => state.show_running, // group with running } } @@ -178,5 +176,6 @@ fn matches_search(run: &Run, query: &str) -> bool { return true; } let q = query.to_lowercase(); - run.name.to_lowercase().contains(&q) || run.tags.iter().any(|t| t.to_lowercase().contains(&q)) + run.name().to_lowercase().contains(&q) + || run.tags().iter().any(|t| t.to_lowercase().contains(&q)) } diff --git a/crates/dashboard/src/inbound/ui/viewport.rs b/crates/dashboard/src/inbound/ui/viewport.rs index 2eaca76..6198411 100644 --- a/crates/dashboard/src/inbound/ui/viewport.rs +++ b/crates/dashboard/src/inbound/ui/viewport.rs @@ -9,7 +9,7 @@ pub struct ViewportBehavior<'a> { pub cache: &'a DataCache, } -impl<'a> egui_tiles::Behavior for ViewportBehavior<'a> { +impl egui_tiles::Behavior for ViewportBehavior<'_> { fn pane_ui(&mut self, ui: &mut Ui, _tile_id: TileId, pane: &mut Pane) -> UiResponse { match pane { Pane::LineChart(state) => panes::line_chart::show(ui, state, self.cache), diff --git a/crates/dashboard/src/lib.rs b/crates/dashboard/src/lib.rs index be9609e..6e660fc 100644 --- a/crates/dashboard/src/lib.rs +++ b/crates/dashboard/src/lib.rs @@ -39,19 +39,18 @@ pub fn run(server_url: String) -> eframe::Result { .replace("/api/query", "/api/ws"); let ws_transport = handle.block_on(async { WebSocketTransport::connect(&ws_url).await.ok() }); - let ws_codec_transport = ws_transport.map(|bt| CodecTransport::new(codec.clone(), bt)); + let ws_codec_transport = ws_transport.map(|bt| CodecTransport::new(codec, bt)); // Build subscriber + reader transport from the same clone - let (subscriber, reader_transport) = match ws_codec_transport { - Some(t) => (WsSubscriber::new(t.clone()), Some(t)), - None => { - let (out_tx, _out_rx) = async_channel::bounded(1); - let (_in_tx, in_rx) = async_channel::bounded(1); - let dummy = WebSocketTransport::from_channels(out_tx, in_rx); - let t = CodecTransport::new(codec.clone(), dummy); - - (WsSubscriber::new(t), None) - } + let (subscriber, reader_transport) = if let Some(t) = ws_codec_transport { + (WsSubscriber::new(t.clone()), Some(t)) + } else { + let (out_tx, _out_rx) = async_channel::bounded(1); + let (_in_tx, in_rx) = async_channel::bounded(1); + let dummy = WebSocketTransport::from_channels(out_tx, in_rx); + let t = CodecTransport::new(codec, dummy); + + (WsSubscriber::new(t), None) }; // Create querier (HTTP) diff --git a/crates/dashboard/src/outbound/http.rs b/crates/dashboard/src/outbound/http.rs index eec5a70..40bd8d3 100644 --- a/crates/dashboard/src/outbound/http.rs +++ b/crates/dashboard/src/outbound/http.rs @@ -42,7 +42,7 @@ where match result { QueryResult::Runs(runs) => Ok(runs), - QueryResult::Error(msg) => Err(ListRunsError::Unknown(msg.into())), + QueryResult::Error(e) => Err(ListRunsError::Unknown(e.into())), other => Err(ListRunsError::Unknown( format!("unexpected response: {other:?}").into(), )), @@ -63,7 +63,7 @@ where match result { QueryResult::Experiments(experiments) => Ok(experiments), - QueryResult::Error(msg) => Err(ListExperimentsError::Unknown(msg.into())), + QueryResult::Error(e) => Err(ListExperimentsError::Unknown(e.into())), other => Err(ListExperimentsError::Unknown( format!("unexpected response: {other:?}").into(), )), @@ -84,7 +84,7 @@ where match result { QueryResult::Projects(projects) => Ok(projects), - QueryResult::Error(msg) => Err(ListProjectsError::Unknown(msg.into())), + QueryResult::Error(e) => Err(ListProjectsError::Unknown(e.into())), other => Err(ListProjectsError::Unknown( format!("unexpected response: {other:?}").into(), )), @@ -105,7 +105,7 @@ where match result { QueryResult::Metrics(metrics) => Ok(metrics), - QueryResult::Error(msg) => Err(ListMetricsError::Unknown(msg.into())), + QueryResult::Error(e) => Err(ListMetricsError::Unknown(e.into())), other => Err(ListMetricsError::Unknown( format!("unexpected response: {other:?}").into(), )), @@ -126,7 +126,7 @@ where match result { QueryResult::Series(series) => Ok(series), - QueryResult::Error(msg) => Err(QueryMetricsError::Unknown(msg.into())), + QueryResult::Error(e) => Err(QueryMetricsError::Unknown(e.into())), other => Err(QueryMetricsError::Unknown( format!("unexpected response: {other:?}").into(), )), @@ -150,7 +150,7 @@ where match result { QueryResult::BatchResponse(response) => Ok(response), - QueryResult::Error(msg) => Err(QueryMetricsError::Unknown(msg.into())), + QueryResult::Error(e) => Err(QueryMetricsError::Unknown(e.into())), other => Err(QueryMetricsError::Unknown( format!("unexpected response: {other:?}").into(), )), diff --git a/crates/downsample/Cargo.toml b/crates/downsample/Cargo.toml index f7d2716..d16cff4 100644 --- a/crates/downsample/Cargo.toml +++ b/crates/downsample/Cargo.toml @@ -13,3 +13,6 @@ photon-store.workspace = true tokio.workspace = true tracing.workspace = true thiserror.workspace = true + +[lints] +workspace = true diff --git a/crates/downsample/src/aggregator/m4.rs b/crates/downsample/src/aggregator/m4.rs index 56084d4..cbad2ac 100644 --- a/crates/downsample/src/aggregator/m4.rs +++ b/crates/downsample/src/aggregator/m4.rs @@ -1,4 +1,5 @@ use photon_core::types::bucket::Bucket; +use photon_core::types::metric::Step; use crate::ports::aggregator::Aggregator; @@ -8,9 +9,9 @@ pub struct M4Aggregator; #[derive(Clone, Debug)] pub struct M4Bucket { - first_step: u64, + first_step: Step, first_value: f64, - last_step: u64, + last_step: Step, last_value: f64, min_value: f64, max_value: f64, @@ -19,7 +20,7 @@ pub struct M4Bucket { impl Aggregator for M4Aggregator { type Bucket = M4Bucket; - fn new_bucket(&self, step: u64, value: f64) -> M4Bucket { + fn new_bucket(&self, step: Step, value: f64) -> M4Bucket { M4Bucket { first_step: step, first_value: value, @@ -30,7 +31,7 @@ impl Aggregator for M4Aggregator { } } - fn push(&self, bucket: &mut M4Bucket, step: u64, value: f64) { + fn push(&self, bucket: &mut M4Bucket, step: Step, value: f64) { bucket.last_step = step; bucket.last_value = value; bucket.min_value = bucket.min_value.min(value); @@ -48,7 +49,7 @@ impl Aggregator for M4Aggregator { } } - fn close(&self, bucket: &M4Bucket, step_start: u64, step_end: u64) -> Bucket { + fn close(&self, bucket: &M4Bucket, step_start: Step, step_end: Step) -> Bucket { Bucket { step_start, step_end, diff --git a/crates/downsample/src/aggregator/noop.rs b/crates/downsample/src/aggregator/noop.rs index c4f5cfb..0b24301 100644 --- a/crates/downsample/src/aggregator/noop.rs +++ b/crates/downsample/src/aggregator/noop.rs @@ -1,4 +1,5 @@ use photon_core::types::bucket::Bucket; +use photon_core::types::metric::Step; use crate::ports::aggregator::Aggregator; @@ -13,11 +14,11 @@ pub struct NoOpBucket { impl Aggregator for NoOpAggregator { type Bucket = NoOpBucket; - fn new_bucket(&self, _step: u64, value: f64) -> NoOpBucket { + fn new_bucket(&self, _step: Step, value: f64) -> NoOpBucket { NoOpBucket { value } } - fn push(&self, bucket: &mut NoOpBucket, _step: u64, value: f64) { + fn push(&self, bucket: &mut NoOpBucket, _step: Step, value: f64) { bucket.value = value; } @@ -25,7 +26,7 @@ impl Aggregator for NoOpAggregator { NoOpBucket { value: b.value } } - fn close(&self, bucket: &NoOpBucket, step_start: u64, step_end: u64) -> Bucket { + fn close(&self, bucket: &NoOpBucket, step_start: Step, step_end: Step) -> Bucket { Bucket { step_start, step_end, diff --git a/crates/downsample/src/compactor.rs b/crates/downsample/src/compactor.rs index e12fcb6..053d8fd 100644 --- a/crates/downsample/src/compactor.rs +++ b/crates/downsample/src/compactor.rs @@ -1,6 +1,6 @@ use photon_core::types::bucket::BucketEntry; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use photon_store::ports::bucket::BucketWriter; use photon_store::ports::compaction::CompactionCursor; use photon_store::ports::metric::MetricReader; @@ -60,12 +60,12 @@ where .get(run_id, key, tier_index) .await .map_err(CompactionError::Read)? - .unwrap_or(0); + .unwrap_or(Step::ZERO); // Read raw points past the cursor let points = self .metric_reader - .read_points(run_id, key, cursor_step..u64::MAX) + .read_points(run_id, key, cursor_step..Step::MAX) .await .map_err(CompactionError::Read)?; @@ -128,14 +128,14 @@ where for (tier_index, &divisor) in self.divisors.iter().enumerate() { let cursor_step = self .cursor - .get(run_id, &key, tier_index) + .get(run_id, key, tier_index) .await .map_err(CompactionError::Read)? - .unwrap_or(0); + .unwrap_or(Step::ZERO); let points = self .metric_reader - .read_points(run_id, &key, cursor_step..u64::MAX) + .read_points(run_id, key, cursor_step..Step::MAX) .await .map_err(CompactionError::Read)?; @@ -176,7 +176,7 @@ where let last_step = entries.last().unwrap().bucket.step_end; self.cursor - .advance(run_id, &key, tier_index, last_step + 1) + .advance(run_id, key, tier_index, last_step + 1) .await .map_err(CompactionError::Write)?; diff --git a/crates/downsample/src/ports/aggregator.rs b/crates/downsample/src/ports/aggregator.rs index 37354ee..cee8e4a 100644 --- a/crates/downsample/src/ports/aggregator.rs +++ b/crates/downsample/src/ports/aggregator.rs @@ -1,4 +1,5 @@ use photon_core::types::bucket::Bucket; +use photon_core::types::metric::Step; /// Incremental bucketing for the resolution pyramid. /// @@ -11,14 +12,14 @@ pub trait Aggregator: Send + Sync + Clone + 'static { type Bucket: Clone + Send + Sync; /// Create a new bucket from the first observation. - fn new_bucket(&self, step: u64, value: f64) -> Self::Bucket; + fn new_bucket(&self, step: Step, value: f64) -> Self::Bucket; /// Add a point to an open bucket. - fn push(&self, bucket: &mut Self::Bucket, step: u64, value: f64); + fn push(&self, bucket: &mut Self::Bucket, step: Step, value: f64); /// Combine two buckets. Must be associative. fn merge(&self, a: &Self::Bucket, b: &Self::Bucket) -> Self::Bucket; /// Extract the final summary. Called once when a bucket closes. - fn close(&self, bucket: &Self::Bucket, step_start: u64, step_end: u64) -> Bucket; + fn close(&self, bucket: &Self::Bucket, step_start: Step, step_end: Step) -> Bucket; } diff --git a/crates/downsample/src/ports/selector.rs b/crates/downsample/src/ports/selector.rs index 3d20f93..3ea24ab 100644 --- a/crates/downsample/src/ports/selector.rs +++ b/crates/downsample/src/ports/selector.rs @@ -1,4 +1,6 @@ +use photon_core::types::metric::Step; + /// Thins a point sequence to a target count. pub trait Selector: Send + Sync + Clone + 'static { - fn select(&self, points: &[(u64, f64)], target: usize) -> Vec<(u64, f64)>; + fn select(&self, points: &[(Step, f64)], target: usize) -> Vec<(Step, f64)>; } diff --git a/crates/downsample/src/reducer.rs b/crates/downsample/src/reducer.rs index 05f79a0..e0bdc33 100644 --- a/crates/downsample/src/reducer.rs +++ b/crates/downsample/src/reducer.rs @@ -1,4 +1,5 @@ use photon_core::types::bucket::Bucket; +use photon_core::types::metric::Step; use crate::ports::aggregator::Aggregator; @@ -11,8 +12,8 @@ struct Tier { divisor: usize, count: usize, open: Option, - first_step: u64, - last_step: u64, + first_step: Step, + last_step: Step, } impl Reducer { @@ -23,26 +24,23 @@ impl Reducer { divisor, count: 0, open: None, - first_step: 0, - last_step: 0, + first_step: Step::ZERO, + last_step: Step::ZERO, }) .collect(); Self { aggregator, tiers } } - pub fn push(&mut self, step: u64, value: f64) -> Vec<(usize, Bucket)> { + pub fn push(&mut self, step: Step, value: f64) -> Vec<(usize, Bucket)> { let mut closed = Vec::new(); for (i, tier) in self.tiers.iter_mut().enumerate() { - match &mut tier.open { - Some(bucket) => { - self.aggregator.push(bucket, step, value); - } - None => { - tier.open = Some(self.aggregator.new_bucket(step, value)); - tier.first_step = step; - } + if let Some(bucket) = &mut tier.open { + self.aggregator.push(bucket, step, value); + } else { + tier.open = Some(self.aggregator.new_bucket(step, value)); + tier.first_step = step; } tier.last_step = step; diff --git a/crates/downsample/src/selector/noop.rs b/crates/downsample/src/selector/noop.rs index 9adfdc6..3181b7e 100644 --- a/crates/downsample/src/selector/noop.rs +++ b/crates/downsample/src/selector/noop.rs @@ -1,10 +1,12 @@ +use photon_core::types::metric::Step; + use crate::ports::selector::Selector; #[derive(Clone)] pub struct NoOpSelector; impl Selector for NoOpSelector { - fn select(&self, points: &[(u64, f64)], _target: usize) -> Vec<(u64, f64)> { + fn select(&self, points: &[(Step, f64)], _target: usize) -> Vec<(Step, f64)> { points.to_vec() } } diff --git a/crates/hook/Cargo.toml b/crates/hook/Cargo.toml index db07443..c9788a2 100644 --- a/crates/hook/Cargo.toml +++ b/crates/hook/Cargo.toml @@ -10,3 +10,6 @@ publish = true [dependencies] photon-core.workspace = true tokio = { workspace = true, features = ["sync"] } + +[lints] +workspace = true diff --git a/crates/ingest/Cargo.toml b/crates/ingest/Cargo.toml index a9a5a5d..a94a11e 100644 --- a/crates/ingest/Cargo.toml +++ b/crates/ingest/Cargo.toml @@ -29,3 +29,6 @@ criterion.workspace = true [[bench]] name = "service" harness = false + +[lints] +workspace = true diff --git a/crates/ingest/benches/service.rs b/crates/ingest/benches/service.rs index 7998dde..73da077 100644 --- a/crates/ingest/benches/service.rs +++ b/crates/ingest/benches/service.rs @@ -7,7 +7,7 @@ use criterion::{BatchSize, BenchmarkId, Criterion, criterion_group, criterion_ma use photon_core::types::batch::WireBatch; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricBatch, MetricPoint}; +use photon_core::types::metric::{Metric, MetricBatch, MetricPoint, Step}; use photon_core::types::sequence::SequenceNumber; use photon_ingest::domain::service::{IngestService, Service}; use photon_protocol::codec::CodecKind; @@ -42,7 +42,7 @@ fn make_wire_batch( .map(|i| MetricPoint { key_index: (i % num_keys) as u32, value: 1.0 / (1.0 + i as f64 * 0.001), - step: i as u64, + step: Step::new(i as u64), timestamp_ms: i as u64, }) .collect(); diff --git a/crates/ingest/src/domain/dedup.rs b/crates/ingest/src/domain/dedup.rs index 8324d82..031c65f 100644 --- a/crates/ingest/src/domain/dedup.rs +++ b/crates/ingest/src/domain/dedup.rs @@ -10,14 +10,21 @@ pub enum Verdict { } /// In-memory cache of the highest sequence number seen per run. +#[derive(Clone)] pub struct DeduplicationCache { - seen: DashMap, + seen: std::sync::Arc>, +} + +impl Default for DeduplicationCache { + fn default() -> Self { + Self::new() + } } impl DeduplicationCache { pub fn new() -> Self { Self { - seen: DashMap::new(), + seen: std::sync::Arc::new(DashMap::new()), } } @@ -31,8 +38,7 @@ impl DeduplicationCache { let highest = self .seen .get(run_id) - .map(|w| *w.value()) - .unwrap_or(SequenceNumber::ZERO); + .map_or(SequenceNumber::ZERO, |w| *w.value()); if seq <= highest { Verdict::Duplicate @@ -44,8 +50,7 @@ impl DeduplicationCache { pub fn watermark(&self, run_id: &RunId) -> SequenceNumber { self.seen .get(run_id) - .map(|w| *w.value()) - .unwrap_or(SequenceNumber::ZERO) + .map_or(SequenceNumber::ZERO, |w| *w.value()) } pub fn advance(&self, run_id: &RunId, seq: SequenceNumber) { diff --git a/crates/ingest/src/domain/service.rs b/crates/ingest/src/domain/service.rs index 44322cc..f014636 100644 --- a/crates/ingest/src/domain/service.rs +++ b/crates/ingest/src/domain/service.rs @@ -18,10 +18,10 @@ pub struct IngestResult { #[derive(Debug, thiserror::Error)] pub enum IngestError { #[error("WAL append failed")] - Wal(#[source] photon_wal::WalError), + Wal(#[from] photon_wal::WalError), } -pub trait IngestService: Send + Sync + 'static { +pub trait IngestService: Clone + Send + Sync + 'static { fn ingest( &self, batch: &WireBatch, @@ -38,15 +38,25 @@ pub trait IngestService: Send + Sync + 'static { /// WAL-backed ingest service. pub struct Service { dedup: DeduplicationCache, - wal: Mutex, + wal: Arc>, notify: Arc, } +impl Clone for Service { + fn clone(&self) -> Self { + Self { + dedup: self.dedup.clone(), + wal: Arc::clone(&self.wal), + notify: Arc::clone(&self.notify), + } + } +} + impl Service { pub fn new(wal: A, notify: Arc) -> Self { Self { dedup: DeduplicationCache::new(), - wal: Mutex::new(wal), + wal: Arc::new(Mutex::new(wal)), notify, } } @@ -78,11 +88,7 @@ impl IngestService for Service { } // 3. WAL append - self.wal - .lock() - .unwrap() - .append(batch) - .map_err(IngestError::Wal)?; + self.wal.lock().unwrap().append(batch)?; // 4. Wake persist consumer self.notify.notify_one(); @@ -104,3 +110,135 @@ impl IngestService for Service { self.dedup.evict(run_id); } } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + use std::time::SystemTime; + + use bytes::Bytes; + + use photon_core::types::ack::AckStatus; + use photon_core::types::batch::WireBatch; + use photon_core::types::id::RunId; + use photon_core::types::sequence::SequenceNumber; + use photon_wal::open_in_memory_wal; + + use super::{IngestService, Service}; + + /// Build a [`WireBatch`] with a correct CRC for the given payload. + fn make_batch(run_id: RunId, seq: u64, payload: &[u8]) -> WireBatch { + let compressed_payload = Bytes::copy_from_slice(payload); + let crc32 = crc32fast::hash(&compressed_payload); + WireBatch { + run_id, + sequence_number: SequenceNumber::from(seq), + compressed_payload, + crc32, + created_at: SystemTime::now(), + point_count: 1, + uncompressed_size: payload.len(), + } + } + + /// Build a [`WireBatch`] whose CRC intentionally does not match the payload. + fn make_batch_bad_crc(run_id: RunId, seq: u64, payload: &[u8]) -> WireBatch { + let compressed_payload = Bytes::copy_from_slice(payload); + let crc32 = crc32fast::hash(&compressed_payload).wrapping_add(1); + WireBatch { + run_id, + sequence_number: SequenceNumber::from(seq), + compressed_payload, + crc32, + created_at: SystemTime::now(), + point_count: 1, + uncompressed_size: payload.len(), + } + } + + fn new_service() -> Service { + let (appender, _mgr) = open_in_memory_wal(); + let notify = Arc::new(tokio::sync::Notify::new()); + Service::new(appender, notify) + } + + #[tokio::test] + async fn test_ingest_accepts_valid_batch() { + let svc = new_service(); + let batch = make_batch(RunId::new(), 1, b"hello"); + + let result = svc.ingest(&batch).await.expect("ingest should succeed"); + + assert_eq!(result.status, AckStatus::Ok); + assert_eq!(result.sequence_number, SequenceNumber::from(1)); + } + + #[tokio::test] + async fn test_ingest_rejects_bad_crc() { + let svc = new_service(); + let batch = make_batch_bad_crc(RunId::new(), 1, b"hello"); + + let result = svc.ingest(&batch).await.expect("ingest should succeed"); + + assert_eq!(result.status, AckStatus::Rejected); + } + + #[tokio::test] + async fn test_ingest_dedup_rejects_duplicate() { + let svc = new_service(); + let run_id = RunId::new(); + let batch = make_batch(run_id, 1, b"payload"); + + let first = svc + .ingest(&batch) + .await + .expect("first ingest should succeed"); + assert_eq!(first.status, AckStatus::Ok); + + let second = svc + .ingest(&batch) + .await + .expect("second ingest should succeed"); + assert_eq!(second.status, AckStatus::Duplicate); + } + + #[tokio::test] + async fn test_watermark_returns_highest_sequence() { + let svc = new_service(); + let run_id = RunId::new(); + + for seq in 1..=3 { + let batch = make_batch(run_id, seq, b"data"); + svc.ingest(&batch).await.expect("ingest should succeed"); + } + + let wm = svc + .watermark(&run_id) + .await + .expect("watermark should succeed"); + assert_eq!(u64::from(wm), 3); + } + + #[tokio::test] + async fn test_evict_run_resets_watermark() { + let svc = new_service(); + let run_id = RunId::new(); + + let batch = make_batch(run_id, 1, b"data"); + svc.ingest(&batch).await.expect("ingest should succeed"); + + let wm = svc + .watermark(&run_id) + .await + .expect("watermark should succeed"); + assert_eq!(u64::from(wm), 1); + + svc.evict_run(&run_id); + + let wm = svc + .watermark(&run_id) + .await + .expect("watermark should succeed"); + assert_eq!(u64::from(wm), 0); + } +} diff --git a/crates/ingest/src/inbound/handler.rs b/crates/ingest/src/inbound/handler.rs index 9f5d84f..33830b6 100644 --- a/crates/ingest/src/inbound/handler.rs +++ b/crates/ingest/src/inbound/handler.rs @@ -1,47 +1,63 @@ -use std::sync::Arc; - +use photon_core::domain::experiment::Experiment; +use photon_core::domain::project::Project; +use photon_core::domain::run::Run; use photon_core::types::ack::{AckResult, AckStatus}; use photon_core::types::batch::WireBatch; +use photon_core::types::error::ApiError; use photon_core::types::ingest::{IngestMessage, IngestResult}; -use photon_store::ports::experiment::ExperimentWriter; -use photon_store::ports::project::ProjectWriter; -use photon_store::ports::run::RunWriter; +use photon_store::ports::WriteRepository; use photon_transport::ports::{Transport, TransportError}; use crate::domain::service::IngestService; -/// Map an ingest message to a result using the given service, run writer, experiment writer, and project writer. -pub async fn dispatch( +/// Map an ingest message to a result using the given service and entity writers. +pub async fn dispatch( service: &S, run_writer: &W, experiment_writer: &E, project_writer: &P, msg: IngestMessage, -) -> IngestResult { +) -> IngestResult +where + S: IngestService, + W: WriteRepository, + E: WriteRepository, + P: WriteRepository, +{ match msg { IngestMessage::Batch(batch) => { let ack = ingest_batch(service, &batch).await; IngestResult::Ack(ack) } - IngestMessage::RegisterRun(run) => match run_writer.upsert_run(&run).await { - Ok(()) => IngestResult::RunRegistered(run.id), - Err(e) => IngestResult::Error(e.to_string()), + IngestMessage::RegisterRun(run) => match run_writer.upsert(&run).await { + Ok(()) => IngestResult::RunRegistered(run.id()), + Err(e) => { + tracing::error!("register run failed: {e}"); + IngestResult::Error(ApiError::Internal) + } }, IngestMessage::RegisterExperiment(experiment) => { - match experiment_writer.upsert_experiment(&experiment).await { + match experiment_writer.upsert(&experiment).await { Ok(()) => IngestResult::ExperimentRegistered(experiment.id), - Err(e) => IngestResult::Error(e.to_string()), + Err(e) => { + tracing::error!("register experiment failed: {e}"); + IngestResult::Error(ApiError::Internal) + } } } - IngestMessage::RegisterProject(project) => { - match project_writer.upsert_project(&project).await { - Ok(()) => IngestResult::ProjectRegistered(project.id), - Err(e) => IngestResult::Error(e.to_string()), + IngestMessage::RegisterProject(project) => match project_writer.upsert(&project).await { + Ok(()) => IngestResult::ProjectRegistered(project.id), + Err(e) => { + tracing::error!("register project failed: {e}"); + IngestResult::Error(ApiError::Internal) } - } + }, IngestMessage::QueryWatermark(run_id) => match service.watermark(&run_id).await { Ok(seq) => IngestResult::Watermark(seq), - Err(e) => IngestResult::Error(e.to_string()), + Err(e) => { + tracing::error!("watermark query failed: {e}"); + IngestResult::Error(ApiError::Internal) + } }, } } @@ -65,16 +81,16 @@ async fn ingest_batch(service: &S, batch: &WireBatch) -> AckRe /// Transport-agnostic ingest handler using envelope types. pub async fn handle_envelope( - service: &Arc, + service: &S, run_writer: &W, experiment_writer: &E, project_writer: &P, transport: &T, ) where S: IngestService, - W: RunWriter, - E: ExperimentWriter, - P: ProjectWriter, + W: WriteRepository, + E: WriteRepository, + P: WriteRepository, T: Transport, { loop { @@ -87,14 +103,7 @@ pub async fn handle_envelope( } }; - let result = dispatch( - &**service, - run_writer, - experiment_writer, - project_writer, - msg, - ) - .await; + let result = dispatch(service, run_writer, experiment_writer, project_writer, msg).await; if transport.send(&result).await.is_err() { break; diff --git a/crates/persist/Cargo.toml b/crates/persist/Cargo.toml index 5bab352..ed8d4cd 100644 --- a/crates/persist/Cargo.toml +++ b/crates/persist/Cargo.toml @@ -12,10 +12,16 @@ photon-protocol.workspace = true photon-store.workspace = true photon-wal.workspace = true -anyhow.workspace = true futures-util.workspace = true thiserror.workspace = true bytes.workspace = true tokio.workspace = true -tokio-util = { version = "0.7", features = ["rt"] } +tokio-util.workspace = true tracing.workspace = true + +[dev-dependencies] +tokio = { workspace = true, features = ["rt", "macros"] } +crc32fast.workspace = true + +[lints] +workspace = true diff --git a/crates/persist/src/domain/service.rs b/crates/persist/src/domain/service.rs index 61401a0..832600e 100644 --- a/crates/persist/src/domain/service.rs +++ b/crates/persist/src/domain/service.rs @@ -27,11 +27,12 @@ pub enum PersistError { WatermarkWrite(#[source] photon_store::ports::WriteError), } -pub trait PersistService: Send + Sync + 'static { +pub trait PersistService: Clone + Send + Sync + 'static { fn write(&self, batches: &[WireBatch]) -> impl Future> + Send; } +#[derive(Clone)] pub struct Service where C: Compressor, @@ -107,3 +108,154 @@ where Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + use bytes::BytesMut; + use std::time::SystemTime; + + use photon_core::types::id::RunId; + use photon_core::types::metric::{Metric, MetricBatch, MetricPoint, Step}; + use photon_core::types::sequence::SequenceNumber; + use photon_protocol::codec::PostcardCodec; + use photon_protocol::compressor::NoopCompressor; + use photon_protocol::ports::codec::Codec; + use photon_protocol::ports::compress::Compressor; + use photon_store::memory::metric::InMemoryMetricStore; + use photon_store::memory::watermark::InMemoryWatermarkStore; + use photon_store::ports::metric::MetricReader; + use photon_store::ports::watermark::WatermarkReader; + + /// Encode and compress a MetricBatch into a WireBatch (the inverse of what + /// the persist service does on the read path). + fn make_wire_batch(batch: &MetricBatch, run_id: RunId, seq: SequenceNumber) -> WireBatch { + let codec = PostcardCodec; + let compressor = NoopCompressor; + + let mut encoded = BytesMut::new(); + codec.encode(batch, &mut encoded).expect("encode"); + + let uncompressed_size = encoded.len(); + + let mut compressed = BytesMut::new(); + compressor + .compress(&encoded, &mut compressed) + .expect("compress"); + + let crc = crc32fast::hash(&compressed); + + WireBatch { + run_id, + sequence_number: seq, + compressed_payload: compressed.freeze(), + crc32: crc, + created_at: SystemTime::now(), + point_count: batch.points.len(), + uncompressed_size, + } + } + + fn new_service( + metrics: InMemoryMetricStore, + watermarks: InMemoryWatermarkStore, + ) -> Service { + Service::new(NoopCompressor, PostcardCodec, metrics, watermarks) + } + + #[tokio::test] + async fn test_write_single_batch() { + let metric_store = InMemoryMetricStore::new(); + let watermark_store = InMemoryWatermarkStore::new(); + let svc = new_service(metric_store.clone(), watermark_store.clone()); + + let run_id = RunId::new(); + let metric = Metric::new("train/loss").unwrap(); + + let batch = MetricBatch { + run_id, + keys: vec![metric.clone()], + points: vec![ + MetricPoint { + key_index: 0, + value: 0.5, + step: Step::new(1), + timestamp_ms: 1000, + }, + MetricPoint { + key_index: 0, + value: 0.3, + step: Step::new(2), + timestamp_ms: 2000, + }, + ], + }; + + let wire = make_wire_batch(&batch, run_id, SequenceNumber::ZERO); + svc.write(&[wire]).await.expect("write should succeed"); + + let points = metric_store + .read_points(&run_id, &metric, Step::ZERO..Step::MAX) + .await + .expect("read_points"); + + assert_eq!(points.len(), 2); + assert_eq!(points[0], (Step::new(1), 0.5)); + assert_eq!(points[1], (Step::new(2), 0.3)); + } + + #[tokio::test] + async fn test_write_updates_watermarks() { + let metric_store = InMemoryMetricStore::new(); + let watermark_store = InMemoryWatermarkStore::new(); + let svc = new_service(metric_store, watermark_store); + + let run_a = RunId::new(); + let run_b = RunId::new(); + let metric = Metric::new("eval/acc").unwrap(); + + let batch_a = MetricBatch { + run_id: run_a, + keys: vec![metric.clone()], + points: vec![MetricPoint { + key_index: 0, + value: 0.9, + step: Step::new(1), + timestamp_ms: 1000, + }], + }; + let batch_b = MetricBatch { + run_id: run_b, + keys: vec![metric.clone()], + points: vec![MetricPoint { + key_index: 0, + value: 0.8, + step: Step::new(1), + timestamp_ms: 2000, + }], + }; + + let wire_a = make_wire_batch(&batch_a, run_a, SequenceNumber::from(3)); + let wire_b = make_wire_batch(&batch_b, run_b, SequenceNumber::from(7)); + + svc.write(&[wire_a, wire_b]) + .await + .expect("write should succeed"); + + // Read from the service's owned watermark writer. + let mut watermarks = svc.watermark_writer.read_all().await.expect("read_all"); + watermarks.sort_by_key(|(_, seq)| u64::from(*seq)); + + assert_eq!(watermarks.len(), 2); + + let (wm_run_a, wm_seq_a) = watermarks[0]; + let (wm_run_b, wm_seq_b) = watermarks[1]; + + assert_eq!(wm_run_a, run_a); + assert_eq!(u64::from(wm_seq_a), 3); + + assert_eq!(wm_run_b, run_b); + assert_eq!(u64::from(wm_seq_b), 7); + } +} diff --git a/crates/persist/src/inbound/thread.rs b/crates/persist/src/inbound/thread.rs index 83155d2..ec7bba9 100644 --- a/crates/persist/src/inbound/thread.rs +++ b/crates/persist/src/inbound/thread.rs @@ -31,7 +31,7 @@ pub struct PersistStats { } pub async fn run( - mut wal: W, + wal: W, notify: Arc, service: S, config: PersistConfig, @@ -97,9 +97,9 @@ where } tokio::select! { - _ = notify.notified() => {} - _ = tokio::time::sleep(config.poll_interval) => {} - _ = cancel.cancelled() => break, + () = notify.notified() => {} + () = tokio::time::sleep(config.poll_interval) => {} + () = cancel.cancelled() => break, } } diff --git a/crates/protocol/Cargo.toml b/crates/protocol/Cargo.toml index cb76d53..2c42f18 100644 --- a/crates/protocol/Cargo.toml +++ b/crates/protocol/Cargo.toml @@ -14,7 +14,6 @@ bytes.workspace = true postcard.workspace = true serde.workspace = true thiserror.workspace = true -anyhow.workspace = true brotli.workspace = true lz4_flex.workspace = true @@ -37,3 +36,6 @@ harness = false [[bench]] name = "compressor" harness = false + +[lints] +workspace = true diff --git a/crates/protocol/benches/codec.rs b/crates/protocol/benches/codec.rs index 149fabb..7f9f23b 100644 --- a/crates/protocol/benches/codec.rs +++ b/crates/protocol/benches/codec.rs @@ -2,7 +2,7 @@ use bytes::BytesMut; use criterion::{BatchSize, BenchmarkId, Criterion, criterion_group, criterion_main}; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricBatch, MetricPoint}; +use photon_core::types::metric::{Metric, MetricBatch, MetricPoint, Step}; use photon_protocol::codec::CodecKind; use photon_protocol::ports::codec::Codec; @@ -26,7 +26,7 @@ fn make_metric_batch(run_id: RunId, n: usize) -> MetricBatch { .map(|i| MetricPoint { key_index: (i % num_keys) as u32, value: 1.0 / (1.0 + i as f64 * 0.001), - step: i as u64, + step: Step::new(i as u64), timestamp_ms: i as u64, }) .collect(); diff --git a/crates/protocol/benches/compressor.rs b/crates/protocol/benches/compressor.rs index 0efda28..6eff61e 100644 --- a/crates/protocol/benches/compressor.rs +++ b/crates/protocol/benches/compressor.rs @@ -2,7 +2,7 @@ use bytes::BytesMut; use criterion::{BatchSize, BenchmarkId, Criterion, criterion_group, criterion_main}; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricBatch, MetricPoint}; +use photon_core::types::metric::{Metric, MetricBatch, MetricPoint, Step}; use photon_protocol::codec::CodecKind; use photon_protocol::compressor::CompressorKind; use photon_protocol::ports::codec::Codec; @@ -28,7 +28,7 @@ fn make_metric_batch(run_id: RunId, n: usize) -> MetricBatch { .map(|i| MetricPoint { key_index: (i % num_keys) as u32, value: 1.0 / (1.0 + i as f64 * 0.001), - step: i as u64, + step: Step::new(i as u64), timestamp_ms: i as u64, }) .collect(); diff --git a/crates/protocol/src/compressor/brotli.rs b/crates/protocol/src/compressor/brotli.rs index 658265a..cde9f88 100644 --- a/crates/protocol/src/compressor/brotli.rs +++ b/crates/protocol/src/compressor/brotli.rs @@ -36,7 +36,7 @@ impl Compressor for BrotliCompressor { encoder .write_all(input) - .map_err(|e| CompressionError::Unknown(e.into()))?; + .map_err(|e| CompressionError::Internal(e.to_string()))?; drop(encoder); output.extend_from_slice(&compressed); diff --git a/crates/protocol/src/compressor/zstd.rs b/crates/protocol/src/compressor/zstd.rs index 8f0ca91..ce963be 100644 --- a/crates/protocol/src/compressor/zstd.rs +++ b/crates/protocol/src/compressor/zstd.rs @@ -38,7 +38,7 @@ mod native { .lock() .unwrap() .compress(input) - .map_err(|e| CompressionError::Unknown(e.into()))?; + .map_err(|e| CompressionError::Internal(e.to_string()))?; output.extend_from_slice(&compressed); Ok(()) @@ -48,8 +48,7 @@ mod native { let capacity = zstd::zstd_safe::get_frame_content_size(input) .ok() .flatten() - .map(|s| s as usize) - .unwrap_or(input.len() * 10); + .map_or(input.len() * 10, |s| s as usize); let decompressed = self .decompressor @@ -108,12 +107,12 @@ mod wasm { fn decompress(&self, input: &[u8], output: &mut BytesMut) -> Result<(), CompressionError> { let mut decoder = StreamingDecoder::new(input) - .map_err(|e| CompressionError::Unknown(anyhow::anyhow!("zstd frame init: {e}")))?; + .map_err(|e| CompressionError::Internal(format!("zstd frame init: {e}")))?; let mut buf = Vec::new(); decoder .read_to_end(&mut buf) - .map_err(|e| CompressionError::Unknown(anyhow::anyhow!("zstd decompress: {e}")))?; + .map_err(|e| CompressionError::Internal(format!("zstd decompress: {e}")))?; output.extend_from_slice(&buf); Ok(()) @@ -142,6 +141,7 @@ impl Clone for ZstdCompressor { } } +#[allow(clippy::missing_fields_in_debug)] impl fmt::Debug for ZstdCompressor { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ZstdCompressor") diff --git a/crates/protocol/src/ports/codec.rs b/crates/protocol/src/ports/codec.rs index 61fe0cf..eff6e8e 100644 --- a/crates/protocol/src/ports/codec.rs +++ b/crates/protocol/src/ports/codec.rs @@ -5,6 +5,7 @@ pub trait Codec: Send + Sync + Clone + 'static { fn decode(&self, input: &[u8]) -> Result; } +#[non_exhaustive] #[derive(Debug, thiserror::Error)] pub enum CodecError { #[error("failed to encode batch: {reason}")] @@ -12,7 +13,4 @@ pub enum CodecError { #[error("failed to decode batch: {reason}")] DecodeFailed { reason: String }, - - #[error(transparent)] - Unknown(#[from] anyhow::Error), } diff --git a/crates/protocol/src/ports/compress.rs b/crates/protocol/src/ports/compress.rs index 5f3f6c8..029e72f 100644 --- a/crates/protocol/src/ports/compress.rs +++ b/crates/protocol/src/ports/compress.rs @@ -10,6 +10,7 @@ pub trait Compressor: Send + Sync + Clone + 'static { fn name(&self) -> &'static str; } +#[non_exhaustive] #[derive(Debug, thiserror::Error)] pub enum CompressionError { #[error("output buffer too small (needed {needed}, available {available})")] @@ -18,6 +19,6 @@ pub enum CompressionError { #[error("corrupt payload for compressor {compressor_name:?}")] CorruptPayload { compressor_name: String }, - #[error(transparent)] - Unknown(#[from] anyhow::Error), + #[error("internal error: {0}")] + Internal(String), } diff --git a/crates/query/Cargo.toml b/crates/query/Cargo.toml index b534352..71db30d 100644 --- a/crates/query/Cargo.toml +++ b/crates/query/Cargo.toml @@ -18,3 +18,6 @@ serde.workspace = true thiserror.workspace = true tokio.workspace = true tracing.workspace = true + +[lints] +workspace = true diff --git a/crates/query/src/domain/service.rs b/crates/query/src/domain/service.rs index 4636050..68f742a 100644 --- a/crates/query/src/domain/service.rs +++ b/crates/query/src/domain/service.rs @@ -3,20 +3,19 @@ use std::future::Future; use photon_core::domain::experiment::Experiment; use photon_core::domain::project::Project; use photon_core::domain::run::Run; +use photon_core::types::error::ApiError; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use photon_core::types::query::{ DataPoint, MetricQuery, MetricSeries, QueryMessage, QueryRequest, QueryResponse, QueryResult, RangePoint, SeriesData, }; use photon_downsample::ports::selector::Selector; use photon_store::ports::ReadError; +use photon_store::ports::ReadRepository; use photon_store::ports::bucket::BucketReader; use photon_store::ports::compaction::CompactionCursor; -use photon_store::ports::experiment::ExperimentReader; use photon_store::ports::metric::MetricReader; -use photon_store::ports::project::ProjectReader; -use photon_store::ports::run::RunReader; use crate::domain::tier::{Resolution, TierSelector}; @@ -26,7 +25,7 @@ pub enum QueryError { Read(#[from] ReadError), } -pub trait QueryService: Send + Sync + 'static { +pub trait QueryService: Clone + Send + Sync + 'static { fn list_runs(&self) -> impl Future, QueryError>> + Send; fn list_experiments(&self) -> impl Future, QueryError>> + Send; @@ -54,40 +53,59 @@ pub async fn dispatch(service: &S, msg: QueryMessage) -> QueryR match msg { QueryMessage::ListRuns => match service.list_runs().await { Ok(runs) => QueryResult::Runs(runs), - Err(e) => QueryResult::Error(e.to_string()), + Err(e) => { + tracing::error!("query failed: {e}"); + QueryResult::Error(ApiError::Internal) + } }, QueryMessage::ListExperiments => match service.list_experiments().await { Ok(experiments) => QueryResult::Experiments(experiments), - Err(e) => QueryResult::Error(e.to_string()), + Err(e) => { + tracing::error!("query failed: {e}"); + QueryResult::Error(ApiError::Internal) + } }, QueryMessage::ListProjects => match service.list_projects().await { Ok(projects) => QueryResult::Projects(projects), - Err(e) => QueryResult::Error(e.to_string()), + Err(e) => { + tracing::error!("query failed: {e}"); + QueryResult::Error(ApiError::Internal) + } }, QueryMessage::ListMetrics(run_id) => match service.list_metrics(&run_id).await { Ok(metrics) => QueryResult::Metrics(metrics), - Err(e) => QueryResult::Error(e.to_string()), + Err(e) => { + tracing::error!("query failed: {e}"); + QueryResult::Error(ApiError::Internal) + } }, QueryMessage::Query(query) => match service.query(&query).await { Ok(series) => QueryResult::Series(series), - Err(e) => QueryResult::Error(e.to_string()), + Err(e) => { + tracing::error!("query failed: {e}"); + QueryResult::Error(ApiError::Internal) + } }, QueryMessage::QueryBatch(request) => match service.query_batch(&request).await { Ok(response) => QueryResult::BatchResponse(response), - Err(e) => QueryResult::Error(e.to_string()), + Err(e) => { + tracing::error!("query failed: {e}"); + QueryResult::Error(ApiError::Internal) + } }, } } +#[derive(Clone)] pub struct Service where S: Selector, B: BucketReader, M: MetricReader, C: CompactionCursor, - R: RunReader, - E: ExperimentReader, - P: ProjectReader, + R: ReadRepository, + E: ReadRepository, + P: ReadRepository, { selector: S, bucket_reader: B, @@ -105,9 +123,9 @@ where B: BucketReader, M: MetricReader, C: CompactionCursor, - R: RunReader, - E: ExperimentReader, - P: ProjectReader, + R: ReadRepository, + E: ReadRepository, + P: ReadRepository, { pub fn new( selector: S, @@ -138,20 +156,20 @@ where B: BucketReader, M: MetricReader, C: CompactionCursor, - R: RunReader, - E: ExperimentReader, - P: ProjectReader, + R: ReadRepository, + E: ReadRepository, + P: ReadRepository, { async fn list_runs(&self) -> Result, QueryError> { - Ok(self.run_reader.list_runs().await?) + Ok(self.run_reader.list().await?) } async fn list_experiments(&self) -> Result, QueryError> { - Ok(self.experiment_reader.list_experiments().await?) + Ok(self.experiment_reader.list().await?) } async fn list_projects(&self) -> Result, QueryError> { - Ok(self.project_reader.list_projects().await?) + Ok(self.project_reader.list().await?) } async fn list_metrics(&self, run_id: &RunId) -> Result, QueryError> { @@ -182,7 +200,7 @@ where SeriesData::Raw { points: selected .into_iter() - .map(|(s, v)| DataPoint { step: s, value: v }) + .map(|(step, value)| DataPoint { step, value }) .collect(), } } @@ -191,7 +209,7 @@ where .compaction_cursor .get(&q.run_id, &q.key, tier_index) .await? - .unwrap_or(0); + .unwrap_or(Step::ZERO); let bucket_end = compacted_through.min(q.step_range.end); let raw_start = compacted_through.max(q.step_range.start); @@ -253,7 +271,7 @@ where }; // Combine bucket values with raw tail - let combined: Vec<(u64, f64)> = buckets + let combined: Vec<(Step, f64)> = buckets .iter() .map(|b| (b.step_start, b.value)) .chain(raw_tail.into_iter()) @@ -268,7 +286,7 @@ where SeriesData::Aggregated { points: selected .into_iter() - .map(|(s, v)| DataPoint { step: s, value: v }) + .map(|(step, value)| DataPoint { step, value }) .collect(), envelope, } diff --git a/crates/query/src/domain/tier.rs b/crates/query/src/domain/tier.rs index 1a92e14..b7cfc21 100644 --- a/crates/query/src/domain/tier.rs +++ b/crates/query/src/domain/tier.rs @@ -12,6 +12,7 @@ pub struct ResolutionPlan { /// Picks the right resolution for the line and envelope based on /// the step range and target point count. +#[derive(Clone)] pub struct TierSelector { divisors: Vec, } @@ -37,8 +38,7 @@ impl TierSelector { .iter() .enumerate() .find(|(_, d)| point_count / **d >= target) - .map(|(i, _)| Resolution::Bucketed(i)) - .unwrap_or(Resolution::Raw); + .map_or(Resolution::Raw, |(i, _)| Resolution::Bucketed(i)); let envelope = match line { Resolution::Raw => Resolution::Raw, @@ -55,3 +55,44 @@ impl TierSelector { ResolutionPlan { line, envelope } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_raw_when_few_points() { + let selector = TierSelector::new(vec![60, 3600]); + // 100 points with a target of 200 and no divisor can satisfy + // point_count / d >= target, so we get Raw. + let plan = selector.pick(100, 200); + assert_eq!(plan.line, Resolution::Raw); + assert_eq!(plan.envelope, Resolution::Raw); + } + + #[test] + fn test_bucketed_when_many_points() { + let selector = TierSelector::new(vec![60, 3600]); + // 720_000 points, target 200. + // divisor 60: 720_000 / 60 = 12_000 >= 200 -> Bucketed(0) (line picks first match) + // divisor 3600: 720_000 / 3600 = 200 >= 200 -> Bucketed(1) (envelope picks last match) + let plan = selector.pick(720_000, 200); + assert_eq!(plan.line, Resolution::Bucketed(0)); + assert_eq!(plan.envelope, Resolution::Bucketed(1)); + } + + #[test] + fn test_default_tier_selector() { + let selector = TierSelector::default(); + // Default divisors are [60, 3600] with 1-minute and 1-hour buckets. + // A small point count should still resolve to Raw. + let plan = selector.pick(50, 500); + assert_eq!(plan.line, Resolution::Raw); + assert_eq!(plan.envelope, Resolution::Raw); + + // A large point count should resolve to Bucketed. + let plan = selector.pick(1_000_000, 500); + assert_eq!(plan.line, Resolution::Bucketed(0)); + assert!(matches!(plan.envelope, Resolution::Bucketed(_))); + } +} diff --git a/crates/query/src/inbound/handler.rs b/crates/query/src/inbound/handler.rs index 553963a..d38617f 100644 --- a/crates/query/src/inbound/handler.rs +++ b/crates/query/src/inbound/handler.rs @@ -1,12 +1,10 @@ -use std::sync::Arc; - use photon_core::types::query::{QueryMessage, QueryResult}; use photon_transport::ports::{Transport, TransportError}; use crate::domain::service::{QueryService, dispatch}; /// Transport-agnostic query handler. -pub async fn handle(service: &Arc, transport: &T) +pub async fn handle(service: &S, transport: &T) where S: QueryService, T: Transport, @@ -21,7 +19,7 @@ where } }; - let result = dispatch(&**service, msg).await; + let result = dispatch(service, msg).await; if transport.send(&result).await.is_err() { break; diff --git a/crates/sdk/Cargo.toml b/crates/sdk/Cargo.toml index 1832a70..8eb2e0d 100644 --- a/crates/sdk/Cargo.toml +++ b/crates/sdk/Cargo.toml @@ -19,7 +19,6 @@ photon-wal.workspace = true photon-batch.workspace = true photon-uplink.workspace = true -anyhow.workspace = true chrono.workspace = true crossbeam-channel.workspace = true lasso.workspace = true @@ -33,3 +32,6 @@ bench = false [[bench]] name = "throughput" harness = false + +[lints] +workspace = true diff --git a/crates/sdk/src/accumulator.rs b/crates/sdk/src/accumulator.rs index da0ec8b..5d139d5 100644 --- a/crates/sdk/src/accumulator.rs +++ b/crates/sdk/src/accumulator.rs @@ -1,12 +1,12 @@ use crossbeam_channel::{Receiver, Sender, TrySendError, bounded}; -pub struct Accumulator { +pub(crate) struct Accumulator { tx: Sender

, points_dropped: u64, } impl Accumulator

{ - pub fn new(channel_capacity: usize) -> (Self, Receiver

) { + pub(crate) fn new(channel_capacity: usize) -> (Self, Receiver

) { let (tx, rx) = bounded(channel_capacity); let accumulator = Self { @@ -17,7 +17,7 @@ impl Accumulator

{ (accumulator, rx) } - pub fn push(&mut self, point: P) { + pub(crate) fn push(&mut self, point: P) { match self.tx.try_send(point) { Ok(()) => {} Err(TrySendError::Full(point)) => { @@ -32,7 +32,7 @@ impl Accumulator

{ } } - pub fn points_dropped(&self) -> u64 { + pub(crate) fn points_dropped(&self) -> u64 { self.points_dropped } } diff --git a/crates/sdk/src/builder.rs b/crates/sdk/src/builder.rs index a3d2e7f..b8f0b8e 100644 --- a/crates/sdk/src/builder.rs +++ b/crates/sdk/src/builder.rs @@ -19,7 +19,7 @@ use photon_protocol::codec::CodecKind; use photon_protocol::compressor::CompressorKind; use photon_transport::TransportKind; use photon_transport::codec::CodecTransport; -use photon_uplink::{TransportError as UplinkTransportError, UplinkThreadError, run_uplink}; +use photon_uplink::{UplinkThreadError, UplinkTransportError, run_uplink}; use photon_wal::{DiskWalConfig, Wal, WalKind}; use crate::accumulator::Accumulator; @@ -189,19 +189,18 @@ impl RunBuilder { } }); - let domain_run = DomainRun { - id: run_id, - project_id: project.id, - experiment_id: self.experiment.as_ref().map(|e| e.id), - user_id: self.user_id.unwrap_or_default(), - name: self - .name + let domain_run = DomainRun::restore( + run_id, + project.id, + self.experiment.as_ref().map(|e| e.id), + self.user_id.unwrap_or_default(), + self.name .unwrap_or_else(|| format!("run-{}", run_id.short())), - status: RunStatus::Running, - tags: self.tags, - created_at: now, - updated_at: now, - }; + RunStatus::Running, + self.tags, + now, + now, + ); let experiment = self.experiment; let wal_dir = self @@ -237,54 +236,57 @@ impl RunBuilder { self.batch, ) }) - .expect("failed to spawn batch thread"); - - let uplink_handle = self.endpoint.map(|endpoint| { - let (shutdown_tx, shutdown_rx) = oneshot::channel(); - let uplink_wal = wal.clone(); - let uplink_config = UplinkConfig::default(); - let transport_kind = self.transport; - let run = domain_run.clone(); - let project = project.clone(); - let experiment = experiment.clone(); - - let handle = thread::Builder::new() - .name(format!("photon-uplink-{run_id}")) - .spawn(move || { - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .map_err(UplinkThreadError::Runtime)?; - - rt.block_on(async { - let bt = transport_kind - .connect(&endpoint) + .map_err(|e| StartError::ThreadSpawn(e.to_string()))?; + + let uplink_handle = self + .endpoint + .map(|endpoint| -> Result { + let (shutdown_tx, shutdown_rx) = oneshot::channel(); + let uplink_wal = wal.clone(); + let uplink_config = UplinkConfig::default(); + let transport_kind = self.transport; + let run = domain_run.clone(); + let project = project.clone(); + let experiment = experiment.clone(); + + let handle = thread::Builder::new() + .name(format!("photon-uplink-{run_id}")) + .spawn(move || { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .map_err(UplinkThreadError::Runtime)?; + + rt.block_on(async { + let bt = transport_kind + .connect(&endpoint) + .await + .map_err(UplinkTransportError::from)?; + + let transport = CodecTransport::new(codec, bt); + + run_uplink( + transport, + run, + project, + experiment, + uplink_wal, + uplink_config, + start_sequence, + shutdown_rx, + batch_rx, + ) .await - .map_err(UplinkTransportError::from)?; - - let transport = CodecTransport::new(codec, bt); - - run_uplink( - transport, - run, - project, - experiment, - uplink_wal, - uplink_config, - start_sequence, - shutdown_rx, - batch_rx, - ) - .await + }) }) - }) - .expect("failed to spawn uplink thread"); + .map_err(|e| StartError::ThreadSpawn(e.to_string()))?; - UplinkHandle { - shutdown_tx: Some(shutdown_tx), - handle, - } - }); + Ok(UplinkHandle { + shutdown_tx: Some(shutdown_tx), + handle, + }) + }) + .transpose()?; Ok(Run::new( run_id, diff --git a/crates/sdk/src/error.rs b/crates/sdk/src/error.rs index 965453f..da44bd3 100644 --- a/crates/sdk/src/error.rs +++ b/crates/sdk/src/error.rs @@ -11,8 +11,8 @@ pub enum StartError { #[error("invalid config for {field}: {reason}")] Config { field: String, reason: String }, - #[error(transparent)] - Unknown(#[from] anyhow::Error), + #[error("failed to spawn thread: {0}")] + ThreadSpawn(String), } #[derive(Debug, thiserror::Error)] diff --git a/crates/sdk/src/run.rs b/crates/sdk/src/run.rs index 48916ac..f65976c 100644 --- a/crates/sdk/src/run.rs +++ b/crates/sdk/src/run.rs @@ -6,7 +6,7 @@ use lasso::ThreadedRodeo; use photon_batch::{BatchError, BatchStats}; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricKey, RawPoint}; +use photon_core::types::metric::{Metric, MetricKey, RawPoint, Step}; use photon_uplink::{UplinkStats, UplinkThreadError}; use photon_wal::Wal; use tokio::sync::oneshot; @@ -32,26 +32,26 @@ pub(crate) struct UplinkHandle { } pub struct Run { - run_id: RunId, + id: RunId, accumulator: Accumulator, interner: Arc, batch_handle: JoinHandle>, uplink_handle: Option, - wal: Box, + wal: Arc, points_logged: u64, } impl Run { pub(crate) fn new( - run_id: RunId, + id: RunId, accumulator: Accumulator, interner: Arc, batch_handle: JoinHandle>, uplink_handle: Option, - wal: Box, + wal: Arc, ) -> Self { Self { - run_id, + id, accumulator, interner, batch_handle, @@ -75,7 +75,7 @@ impl Run { self.accumulator.push(RawPoint { key: metric_key, value, - step, + step: Step::new(step), timestamp_ns: now, }); @@ -92,7 +92,7 @@ impl Run { } pub fn id(&self) -> RunId { - self.run_id + self.id } /// Flushes remaining points and waits for the pipeline to drain. diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index 87f21b3..591e063 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -27,8 +27,9 @@ photon-transport.workspace = true photon-hook.workspace = true photon-wal.workspace = true -tokio.workspace = true -tokio-util = { version = "0.7", features = ["rt"] } +anyhow.workspace = true +tokio = { workspace = true, features = ["net", "io-util", "rt-multi-thread"] } +tokio-util.workspace = true tracing.workspace = true tracing-subscriber.workspace = true @@ -43,3 +44,6 @@ harness = false [features] dashboard = ["rust-embed"] + +[lints] +workspace = true diff --git a/crates/server/benches/throughput.rs b/crates/server/benches/throughput.rs index d7d79dd..807a3db 100644 --- a/crates/server/benches/throughput.rs +++ b/crates/server/benches/throughput.rs @@ -7,6 +7,9 @@ use tokio::net::TcpListener; use photon_ingest::domain::service::Service as IngestService; use photon_ingest::inbound::handler; use photon_protocol::codec::CodecKind; +use photon_store::memory::experiment::InMemoryExperimentStore; +use photon_store::memory::project::InMemoryProjectStore; +use photon_store::memory::run::InMemoryRunStore; use photon_transport::codec::CodecTransport; use photon_transport::tcp::TcpTransport; use photon_wal::open_in_memory_wal; @@ -26,17 +29,23 @@ async fn main() -> Result<(), Box> { let (wal_appender, _wal) = open_in_memory_wal(); let notify = Arc::new(tokio::sync::Notify::new()); - let ingest_service = Arc::new(IngestService::new(wal_appender, notify)); + let ingest_service = IngestService::new(wal_appender, notify); + let run_store = InMemoryRunStore::new(); + let experiment_store = InMemoryExperimentStore::new(); + let project_store = InMemoryProjectStore::new(); tokio::spawn(async move { loop { let (stream, _) = listener.accept().await.expect("accept failed"); let bt = TcpTransport::accept(stream); let transport = CodecTransport::new(codec, bt); - let service = Arc::clone(&ingest_service); + let service = ingest_service.clone(); + let rs = run_store.clone(); + let es = experiment_store.clone(); + let ps = project_store.clone(); tokio::spawn(async move { - handler::handle(&service, &transport).await; + handler::handle_envelope(&service, &rs, &es, &ps, &transport).await; }); } }); diff --git a/crates/server/examples/wire.rs b/crates/server/examples/wire.rs index 5dc05e4..3ddfd88 100644 --- a/crates/server/examples/wire.rs +++ b/crates/server/examples/wire.rs @@ -15,12 +15,12 @@ use photon_persist::inbound::thread::PersistConfig; use photon_protocol::codec::CodecKind; use photon_protocol::compressor::CompressorKind; use photon_protocol::compressor::ZstdCompressor; -use photon_store::clickhouse::metric::ClickHouseMetricStore; -use photon_store::clickhouse::watermark::ClickHouseWatermarkStore; -use photon_store::clickhouse::{ClientBuilder, migrate}; use photon_store::clickhouse::experiment::ClickHouseExperimentStore; +use photon_store::clickhouse::metric::ClickHouseMetricStore; use photon_store::clickhouse::project::ClickHouseProjectStore; use photon_store::clickhouse::run::ClickHouseRunStore; +use photon_store::clickhouse::watermark::ClickHouseWatermarkStore; +use photon_store::clickhouse::{ClientBuilder, migrate}; use photon_store::ports::watermark::WatermarkReader; use photon_transport::codec::CodecTransport; use photon_transport::tcp::TcpTransport; @@ -59,7 +59,7 @@ async fn main() -> Result<(), Box> { open_disk_wal(".photon/server-wal", DiskWalConfig::default())?; // Ingest service (WAL-backed) - let ingest_service = Arc::new(IngestService::new(wal_appender, notify.clone())); + let ingest_service = IngestService::new(wal_appender, notify.clone()); // Seed dedup cache from persisted watermarks let watermarks = watermark_store.read_all().await?; @@ -93,7 +93,7 @@ async fn main() -> Result<(), Box> { let (stream, _) = listener.accept().await.expect("accept failed"); let bt = TcpTransport::accept(stream); let transport = CodecTransport::new(codec, bt); - let service = Arc::clone(&ingest_service); + let service = ingest_service.clone(); let run_store = run_store.clone(); let experiment_store = experiment_store.clone(); let project_store = project_store.clone(); diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 19959c5..7872ab2 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -17,12 +17,12 @@ use photon_query::domain::tier::TierSelector; use photon_query::inbound::handler as query_handler; use photon_store::clickhouse::bucket::ClickHouseBucketStore; use photon_store::clickhouse::compaction::ClickHouseCompactionCursor; -use photon_store::clickhouse::metric::ClickHouseMetricStore; -use photon_store::clickhouse::watermark::ClickHouseWatermarkStore; -use photon_store::clickhouse::{ClientBuilder, migrate}; use photon_store::clickhouse::experiment::ClickHouseExperimentStore; +use photon_store::clickhouse::metric::ClickHouseMetricStore; use photon_store::clickhouse::project::ClickHouseProjectStore; use photon_store::clickhouse::run::ClickHouseRunStore; +use photon_store::clickhouse::watermark::ClickHouseWatermarkStore; +use photon_store::clickhouse::{ClientBuilder, migrate}; use photon_store::ports::watermark::WatermarkReader; use photon_subscription::SubscriptionHook; use photon_transport::router::Router; @@ -32,15 +32,15 @@ use photon_wal::{DiskWalConfig, open_disk_wal}; mod dashboard; #[tokio::main] -async fn main() -> Result<(), Box> { +async fn main() -> anyhow::Result<()> { tracing_subscriber::fmt() .with_env_filter( EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info")), ) .init(); - let ingest_addr = "[::1]:50051"; - let api_addr = "[::1]:50052"; + let ingest_addr = std::env::var("PHOTON_INGEST_ADDR").unwrap_or_else(|_| "[::1]:50051".into()); + let api_addr = std::env::var("PHOTON_API_ADDR").unwrap_or_else(|_| "[::1]:50052".into()); let codec = CodecKind::default(); let cancel = CancellationToken::new(); @@ -64,7 +64,7 @@ async fn main() -> Result<(), Box> { open_disk_wal(".photon/server-wal", DiskWalConfig::default())?; // Ingest hexagon (WAL-backed) - let ingest_service = Arc::new(IngestService::new(wal_appender, notify.clone())); + let ingest_service = IngestService::new(wal_appender, notify.clone()); // Seed dedup cache from persisted watermarks let watermarks = watermark_store.read_all().await?; @@ -91,7 +91,7 @@ async fn main() -> Result<(), Box> { cancel.clone(), )); - let query_service = Arc::new(QueryService::new( + let query_service = QueryService::new( NoOpSelector, bucket_store, metric_store, @@ -100,19 +100,25 @@ async fn main() -> Result<(), Box> { experiment_store.clone(), project_store.clone(), TierSelector::default(), - )); + ); - let ingest_listener = TcpListener::bind(ingest_addr).await?; + let ingest_listener = TcpListener::bind(&ingest_addr).await?; tracing::info!("ingest listening on {ingest_addr}"); - tokio::spawn(photon_transport::serve(ingest_listener, codec, move |t| { + let ingest_handle = tokio::spawn(photon_transport::serve(ingest_listener, codec, move |t| { let svc = ingest_service.clone(); let run_store = run_store.clone(); let experiment_store = experiment_store.clone(); let project_store = project_store.clone(); async move { - ingest_handler::handle_envelope(&svc, &run_store, &experiment_store, &project_store, &t) - .await + ingest_handler::handle_envelope( + &svc, + &run_store, + &experiment_store, + &project_store, + &t, + ) + .await; } })); @@ -129,15 +135,15 @@ async fn main() -> Result<(), Box> { #[cfg(feature = "dashboard")] let router = router.fallback(dashboard::get_file); - let api_listener = TcpListener::bind(api_addr).await?; + let api_listener = TcpListener::bind(&api_addr).await?; tracing::info!("api listening on http://{api_addr}"); - tokio::spawn(async move { router.serve(api_listener).await }); + let api_handle = tokio::spawn(async move { router.serve(api_listener).await }); tokio::signal::ctrl_c().await?; tracing::info!("shutting down"); cancel.cancel(); - let _ = persist_handle.await; + let _ = tokio::join!(persist_handle, ingest_handle, api_handle); Ok(()) } diff --git a/crates/store/Cargo.toml b/crates/store/Cargo.toml index b372a5f..269d703 100644 --- a/crates/store/Cargo.toml +++ b/crates/store/Cargo.toml @@ -9,7 +9,6 @@ publish.workspace = true [dependencies] photon-core.workspace = true -anyhow.workspace = true chrono.workspace = true clickhouse.workspace = true dashmap.workspace = true @@ -18,3 +17,6 @@ thiserror.workspace = true tokio.workspace = true tracing.workspace = true uuid.workspace = true + +[lints] +workspace = true diff --git a/crates/store/src/clickhouse/bucket.rs b/crates/store/src/clickhouse/bucket.rs index 1cdadf0..b4e2096 100644 --- a/crates/store/src/clickhouse/bucket.rs +++ b/crates/store/src/clickhouse/bucket.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use photon_core::types::bucket::{Bucket, BucketEntry}; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use crate::ports::bucket::{BucketReader, BucketWriter}; use crate::ports::{ReadError, WriteError}; @@ -56,7 +56,7 @@ impl BucketWriter for ClickHouseBucketStore { let mut insert = self .client .insert("buckets") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; let run_uuid: uuid::Uuid = (*run_id).into(); for entry in entries { @@ -65,20 +65,20 @@ impl BucketWriter for ClickHouseBucketStore { run_id: run_uuid, key: entry.key.as_str().to_owned(), tier: entry.tier as u32, - step_start: entry.bucket.step_start, - step_end: entry.bucket.step_end, + step_start: entry.bucket.step_start.as_u64(), + step_end: entry.bucket.step_end.as_u64(), value: entry.bucket.value, min: entry.bucket.min, max: entry.bucket.max, }) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; } insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } @@ -90,7 +90,7 @@ impl BucketReader for ClickHouseBucketStore { run_id: &RunId, key: &Metric, tier: usize, - step_range: Range, + step_range: Range, ) -> Result, ReadError> { let run_uuid: uuid::Uuid = (*run_id).into(); @@ -105,17 +105,17 @@ impl BucketReader for ClickHouseBucketStore { .bind(run_uuid) .bind(key.as_str()) .bind(tier as u32) - .bind(step_range.end) - .bind(step_range.start) + .bind(step_range.end.as_u64()) + .bind(step_range.start.as_u64()) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows .into_iter() .map(|r| Bucket { - step_start: r.step_start, - step_end: r.step_end, + step_start: Step::new(r.step_start), + step_end: Step::new(r.step_end), value: r.value, min: r.min, max: r.max, diff --git a/crates/store/src/clickhouse/compaction.rs b/crates/store/src/clickhouse/compaction.rs index ff7943c..833f86d 100644 --- a/crates/store/src/clickhouse/compaction.rs +++ b/crates/store/src/clickhouse/compaction.rs @@ -2,7 +2,7 @@ use clickhouse::Row; use serde::{Deserialize, Serialize}; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use crate::ports::compaction::CompactionCursor; use crate::ports::{ReadError, WriteError}; @@ -33,7 +33,7 @@ impl CompactionCursor for ClickHouseCompactionCursor { run_id: &RunId, key: &Metric, tier: usize, - ) -> Result, ReadError> { + ) -> Result, ReadError> { let run_uuid: uuid::Uuid = (*run_id).into(); let rows: Vec = self @@ -47,9 +47,9 @@ impl CompactionCursor for ClickHouseCompactionCursor { .bind(tier as u32) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; - Ok(rows.first().map(|r| r.offset)) + Ok(rows.first().map(|r| Step::new(r.offset))) } async fn advance( @@ -57,27 +57,27 @@ impl CompactionCursor for ClickHouseCompactionCursor { run_id: &RunId, key: &Metric, tier: usize, - offset: u64, + through: Step, ) -> Result<(), WriteError> { let mut insert = self .client .insert("compaction_cursors") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .write(&CompactionCursorRow { run_id: (*run_id).into(), key: key.as_str().to_owned(), tier: tier as u32, - offset, + offset: through.as_u64(), }) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } diff --git a/crates/store/src/clickhouse/experiment.rs b/crates/store/src/clickhouse/experiment.rs index cc14038..aec1d17 100644 --- a/crates/store/src/clickhouse/experiment.rs +++ b/crates/store/src/clickhouse/experiment.rs @@ -4,8 +4,7 @@ use serde::{Deserialize, Serialize}; use photon_core::domain::experiment::Experiment; use photon_core::types::id::{ExperimentId, ProjectId}; -use crate::ports::experiment::{ExperimentReader, ExperimentWriter}; -use crate::ports::{ReadError, WriteError}; +use crate::ports::{ReadError, ReadRepository, WriteError, WriteRepository}; #[derive(Row, Serialize, Deserialize)] struct ExperimentRow { @@ -39,10 +38,8 @@ impl From for Experiment { project_id: ProjectId::from(r.project_id), name: r.name, tags: r.tags, - created_at: chrono::DateTime::from_timestamp_millis(r.created_at) - .unwrap_or_default(), - updated_at: chrono::DateTime::from_timestamp_millis(r.updated_at) - .unwrap_or_default(), + created_at: chrono::DateTime::from_timestamp_millis(r.created_at).unwrap_or_default(), + updated_at: chrono::DateTime::from_timestamp_millis(r.updated_at).unwrap_or_default(), } } } @@ -58,39 +55,39 @@ impl ClickHouseExperimentStore { } } -impl ExperimentWriter for ClickHouseExperimentStore { - async fn upsert_experiment(&self, experiment: &Experiment) -> Result<(), WriteError> { +impl WriteRepository for ClickHouseExperimentStore { + async fn upsert(&self, experiment: &Experiment) -> Result<(), WriteError> { let mut insert = self .client .insert("experiments") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .write(&ExperimentRow::from(experiment)) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } } -impl ExperimentReader for ClickHouseExperimentStore { - async fn list_experiments(&self) -> Result, ReadError> { +impl ReadRepository for ClickHouseExperimentStore { + async fn list(&self) -> Result, ReadError> { let rows: Vec = self .client .query("SELECT ?fields FROM experiments FINAL") .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows.into_iter().map(Experiment::from).collect()) } - async fn get_experiment(&self, id: &ExperimentId) -> Result, ReadError> { + async fn get(&self, id: &ExperimentId) -> Result, ReadError> { let uuid: uuid::Uuid = (*id).into(); let rows: Vec = self @@ -99,7 +96,7 @@ impl ExperimentReader for ClickHouseExperimentStore { .bind(uuid) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows.into_iter().next().map(Experiment::from)) } diff --git a/crates/store/src/clickhouse/metric.rs b/crates/store/src/clickhouse/metric.rs index 267ab75..345d91d 100644 --- a/crates/store/src/clickhouse/metric.rs +++ b/crates/store/src/clickhouse/metric.rs @@ -4,7 +4,7 @@ use clickhouse::Row; use serde::{Deserialize, Serialize}; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricBatch}; +use photon_core::types::metric::{Metric, MetricBatch, Step}; use crate::ports::metric::{MetricReader, MetricWriter}; use crate::ports::{ReadError, WriteError}; @@ -59,7 +59,7 @@ impl MetricWriter for ClickHouseMetricStore { let mut insert = self .client .insert("metrics") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; for batch in batches { let run_uuid: uuid::Uuid = batch.run_id.into(); @@ -69,19 +69,19 @@ impl MetricWriter for ClickHouseMetricStore { .write(&MetricWriteRow { run_id: run_uuid, key: batch.key(point).as_str().to_owned(), - step: point.step, + step: point.step.as_u64(), value: point.value, timestamp_ms: point.timestamp_ms, }) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; } } insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } } @@ -91,8 +91,8 @@ impl MetricReader for ClickHouseMetricStore { &self, run_id: &RunId, key: &Metric, - step_range: Range, - ) -> Result, ReadError> { + step_range: Range, + ) -> Result, ReadError> { let run_uuid: uuid::Uuid = (*run_id).into(); let rows: Vec = self @@ -104,13 +104,16 @@ impl MetricReader for ClickHouseMetricStore { ) .bind(run_uuid) .bind(key.as_str()) - .bind(step_range.start) - .bind(step_range.end) + .bind(step_range.start.as_u64()) + .bind(step_range.end.as_u64()) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; - Ok(rows.into_iter().map(|r| (r.step, r.value)).collect()) + Ok(rows + .into_iter() + .map(|r| (Step::new(r.step), r.value)) + .collect()) } async fn list_metrics(&self, run_id: &RunId) -> Result, ReadError> { @@ -122,7 +125,7 @@ impl MetricReader for ClickHouseMetricStore { .bind(run_uuid) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows .into_iter() @@ -134,7 +137,7 @@ impl MetricReader for ClickHouseMetricStore { &self, run_id: &RunId, key: &Metric, - step_range: Range, + step_range: Range, ) -> Result { let run_uuid: uuid::Uuid = (*run_id).into(); @@ -146,12 +149,12 @@ impl MetricReader for ClickHouseMetricStore { ) .bind(run_uuid) .bind(key.as_str()) - .bind(step_range.start) - .bind(step_range.end) + .bind(step_range.start.as_u64()) + .bind(step_range.end.as_u64()) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; - Ok(rows.first().map(|r| r.count as usize).unwrap_or(0)) + Ok(rows.first().map_or(0, |r| r.count as usize)) } } diff --git a/crates/store/src/clickhouse/mod.rs b/crates/store/src/clickhouse/mod.rs index 37b9dde..ae57656 100644 --- a/crates/store/src/clickhouse/mod.rs +++ b/crates/store/src/clickhouse/mod.rs @@ -13,6 +13,12 @@ pub struct ClientBuilder { password: String, } +impl Default for ClientBuilder { + fn default() -> Self { + Self::new() + } +} + impl ClientBuilder { pub fn new() -> Self { Self { diff --git a/crates/store/src/clickhouse/project.rs b/crates/store/src/clickhouse/project.rs index 4bcf89d..3db7135 100644 --- a/crates/store/src/clickhouse/project.rs +++ b/crates/store/src/clickhouse/project.rs @@ -4,8 +4,7 @@ use serde::{Deserialize, Serialize}; use photon_core::domain::project::Project; use photon_core::types::id::{ProjectId, TenantId}; -use crate::ports::project::{ProjectReader, ProjectWriter}; -use crate::ports::{ReadError, WriteError}; +use crate::ports::{ReadError, ReadRepository, WriteError, WriteRepository}; #[derive(Row, Serialize, Deserialize)] struct ProjectRow { @@ -36,10 +35,8 @@ impl From for Project { id: ProjectId::from(r.id), tenant_id: TenantId::from(r.tenant_id), name: r.name, - created_at: chrono::DateTime::from_timestamp_millis(r.created_at) - .unwrap_or_default(), - updated_at: chrono::DateTime::from_timestamp_millis(r.updated_at) - .unwrap_or_default(), + created_at: chrono::DateTime::from_timestamp_millis(r.created_at).unwrap_or_default(), + updated_at: chrono::DateTime::from_timestamp_millis(r.updated_at).unwrap_or_default(), } } } @@ -55,39 +52,39 @@ impl ClickHouseProjectStore { } } -impl ProjectWriter for ClickHouseProjectStore { - async fn upsert_project(&self, project: &Project) -> Result<(), WriteError> { +impl WriteRepository for ClickHouseProjectStore { + async fn upsert(&self, project: &Project) -> Result<(), WriteError> { let mut insert = self .client .insert("projects") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .write(&ProjectRow::from(project)) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } } -impl ProjectReader for ClickHouseProjectStore { - async fn list_projects(&self) -> Result, ReadError> { +impl ReadRepository for ClickHouseProjectStore { + async fn list(&self) -> Result, ReadError> { let rows: Vec = self .client .query("SELECT ?fields FROM projects FINAL") .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows.into_iter().map(Project::from).collect()) } - async fn get_project(&self, id: &ProjectId) -> Result, ReadError> { + async fn get(&self, id: &ProjectId) -> Result, ReadError> { let uuid: uuid::Uuid = (*id).into(); let rows: Vec = self @@ -96,7 +93,7 @@ impl ProjectReader for ClickHouseProjectStore { .bind(uuid) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows.into_iter().next().map(Project::from)) } diff --git a/crates/store/src/clickhouse/run.rs b/crates/store/src/clickhouse/run.rs index f538c67..eb6cc5e 100644 --- a/crates/store/src/clickhouse/run.rs +++ b/crates/store/src/clickhouse/run.rs @@ -4,8 +4,7 @@ use serde::{Deserialize, Serialize}; use photon_core::domain::run::{Run, RunStatus}; use photon_core::types::id::{ExperimentId, ProjectId, RunId, UserId}; -use crate::ports::run::{RunReader, RunWriter}; -use crate::ports::{ReadError, WriteError}; +use crate::ports::{ReadError, ReadRepository, WriteError, WriteRepository}; #[derive(Row, Serialize, Deserialize)] struct RunRow { @@ -28,7 +27,7 @@ struct RunRow { impl From<&Run> for RunRow { fn from(r: &Run) -> Self { - let (status, status_reason) = match &r.status { + let (status, status_reason) = match r.status() { RunStatus::Created => ("created".to_string(), String::new()), RunStatus::Running => ("running".to_string(), String::new()), RunStatus::Finished => ("finished".to_string(), String::new()), @@ -36,17 +35,17 @@ impl From<&Run> for RunRow { }; Self { - id: r.id.into(), - project_id: r.project_id.into(), - experiment_id: r.experiment_id.map(Into::into).unwrap_or(uuid::Uuid::nil()), - has_experiment: r.experiment_id.is_some(), - user_id: r.user_id.into(), - name: r.name.clone(), + id: r.id().into(), + project_id: r.project_id().into(), + experiment_id: r.experiment_id().map_or(uuid::Uuid::nil(), Into::into), + has_experiment: r.experiment_id().is_some(), + user_id: r.user_id().into(), + name: r.name().to_owned(), status, status_reason, - tags: r.tags.clone(), - created_at: r.created_at.timestamp_millis(), - updated_at: r.updated_at.timestamp_millis(), + tags: r.tags().to_vec(), + created_at: r.created_at().timestamp_millis(), + updated_at: r.updated_at().timestamp_millis(), } } } @@ -62,23 +61,21 @@ impl From for Run { _ => RunStatus::Created, }; - Self { - id: RunId::from(r.id), - project_id: ProjectId::from(r.project_id), - experiment_id: if r.has_experiment { + Run::restore( + RunId::from(r.id), + ProjectId::from(r.project_id), + if r.has_experiment { Some(ExperimentId::from(r.experiment_id)) } else { None }, - user_id: UserId::from(r.user_id), - name: r.name, + UserId::from(r.user_id), + r.name, status, - tags: r.tags, - created_at: chrono::DateTime::from_timestamp_millis(r.created_at) - .unwrap_or_default(), - updated_at: chrono::DateTime::from_timestamp_millis(r.updated_at) - .unwrap_or_default(), - } + r.tags, + chrono::DateTime::from_timestamp_millis(r.created_at).unwrap_or_default(), + chrono::DateTime::from_timestamp_millis(r.updated_at).unwrap_or_default(), + ) } } @@ -93,39 +90,39 @@ impl ClickHouseRunStore { } } -impl RunWriter for ClickHouseRunStore { - async fn upsert_run(&self, run: &Run) -> Result<(), WriteError> { +impl WriteRepository for ClickHouseRunStore { + async fn upsert(&self, run: &Run) -> Result<(), WriteError> { let mut insert = self .client .insert("runs") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .write(&RunRow::from(run)) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } } -impl RunReader for ClickHouseRunStore { - async fn list_runs(&self) -> Result, ReadError> { +impl ReadRepository for ClickHouseRunStore { + async fn list(&self) -> Result, ReadError> { let rows: Vec = self .client .query("SELECT ?fields FROM runs FINAL") .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows.into_iter().map(Run::from).collect()) } - async fn get_run(&self, id: &RunId) -> Result, ReadError> { + async fn get(&self, id: &RunId) -> Result, ReadError> { let uuid: uuid::Uuid = (*id).into(); let rows: Vec = self @@ -134,7 +131,7 @@ impl RunReader for ClickHouseRunStore { .bind(uuid) .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows.into_iter().next().map(Run::from)) } diff --git a/crates/store/src/clickhouse/watermark.rs b/crates/store/src/clickhouse/watermark.rs index 710f180..9196ffe 100644 --- a/crates/store/src/clickhouse/watermark.rs +++ b/crates/store/src/clickhouse/watermark.rs @@ -37,7 +37,7 @@ impl WatermarkWriter for ClickHouseWatermarkStore { let mut insert = self .client .insert("watermarks") - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; for (run_id, seq) in entries { insert @@ -46,13 +46,13 @@ impl WatermarkWriter for ClickHouseWatermarkStore { sequence: (*seq).into(), }) .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; } insert .end() .await - .map_err(|e| WriteError::Unknown(e.into()))?; + .map_err(|e| WriteError::Store(Box::new(e)))?; Ok(()) } } @@ -64,7 +64,7 @@ impl WatermarkReader for ClickHouseWatermarkStore { .query("SELECT ?fields FROM watermarks FINAL") .fetch_all() .await - .map_err(|e| ReadError::Unknown(e.into()))?; + .map_err(|e| ReadError::Store(Box::new(e)))?; Ok(rows .into_iter() diff --git a/crates/store/src/memory/bucket.rs b/crates/store/src/memory/bucket.rs index bdea148..949a897 100644 --- a/crates/store/src/memory/bucket.rs +++ b/crates/store/src/memory/bucket.rs @@ -5,7 +5,7 @@ use dashmap::DashMap; use photon_core::types::bucket::{Bucket, BucketEntry}; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use crate::ports::bucket::{BucketReader, BucketWriter}; use crate::ports::{ReadError, WriteError}; @@ -52,7 +52,7 @@ impl BucketReader for InMemoryBucketStore { run_id: &RunId, key: &Metric, tier: usize, - step_range: Range, + step_range: Range, ) -> Result, ReadError> { let Some(buckets) = self.data.get(&(*run_id, key.clone(), tier)) else { return Ok(Vec::new()); diff --git a/crates/store/src/memory/compaction.rs b/crates/store/src/memory/compaction.rs index 2971ebc..26fca38 100644 --- a/crates/store/src/memory/compaction.rs +++ b/crates/store/src/memory/compaction.rs @@ -3,14 +3,14 @@ use std::sync::Arc; use dashmap::DashMap; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use crate::ports::compaction::CompactionCursor; use crate::ports::{ReadError, WriteError}; #[derive(Clone)] pub struct InMemoryCompactionCursor { - data: Arc>, + data: Arc>, } impl InMemoryCompactionCursor { @@ -33,7 +33,7 @@ impl CompactionCursor for InMemoryCompactionCursor { run_id: &RunId, key: &Metric, tier: usize, - ) -> Result, ReadError> { + ) -> Result, ReadError> { Ok(self .data .get(&(*run_id, key.clone(), tier)) @@ -45,9 +45,9 @@ impl CompactionCursor for InMemoryCompactionCursor { run_id: &RunId, key: &Metric, tier: usize, - offset: u64, + through: Step, ) -> Result<(), WriteError> { - self.data.insert((*run_id, key.clone(), tier), offset); + self.data.insert((*run_id, key.clone(), tier), through); Ok(()) } } diff --git a/crates/store/src/memory/experiment.rs b/crates/store/src/memory/experiment.rs index 068b72a..ae8d3b6 100644 --- a/crates/store/src/memory/experiment.rs +++ b/crates/store/src/memory/experiment.rs @@ -5,8 +5,7 @@ use dashmap::DashMap; use photon_core::domain::experiment::Experiment; use photon_core::types::id::ExperimentId; -use crate::ports::experiment::{ExperimentReader, ExperimentWriter}; -use crate::ports::{ReadError, WriteError}; +use crate::ports::{ReadError, ReadRepository, WriteError, WriteRepository}; #[derive(Clone)] pub struct InMemoryExperimentStore { @@ -27,8 +26,8 @@ impl Default for InMemoryExperimentStore { } } -impl ExperimentReader for InMemoryExperimentStore { - async fn list_experiments(&self) -> Result, ReadError> { +impl ReadRepository for InMemoryExperimentStore { + async fn list(&self) -> Result, ReadError> { Ok(self .data .iter() @@ -36,13 +35,13 @@ impl ExperimentReader for InMemoryExperimentStore { .collect()) } - async fn get_experiment(&self, id: &ExperimentId) -> Result, ReadError> { + async fn get(&self, id: &ExperimentId) -> Result, ReadError> { Ok(self.data.get(id).map(|entry| entry.value().clone())) } } -impl ExperimentWriter for InMemoryExperimentStore { - async fn upsert_experiment(&self, experiment: &Experiment) -> Result<(), WriteError> { +impl WriteRepository for InMemoryExperimentStore { + async fn upsert(&self, experiment: &Experiment) -> Result<(), WriteError> { self.data.insert(experiment.id, experiment.clone()); Ok(()) } diff --git a/crates/store/src/memory/metric.rs b/crates/store/src/memory/metric.rs index 6dd3336..db3f551 100644 --- a/crates/store/src/memory/metric.rs +++ b/crates/store/src/memory/metric.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use dashmap::DashMap; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricBatch}; +use photon_core::types::metric::{Metric, MetricBatch, Step}; use crate::ports::metric::{MetricReader, MetricWriter}; use crate::ports::{ReadError, WriteError}; @@ -35,7 +35,7 @@ impl MetricWriter for InMemoryMetricStore { for point in &batch.points { run.entry(batch.key(point).clone()) .or_default() - .insert(point.step, point.value); + .insert(point.step.as_u64(), point.value); } Ok(()) } @@ -46,8 +46,8 @@ impl MetricReader for InMemoryMetricStore { &self, run_id: &RunId, key: &Metric, - step_range: Range, - ) -> Result, ReadError> { + step_range: Range, + ) -> Result, ReadError> { let Some(run) = self.data.get(run_id) else { return Ok(Vec::new()); }; @@ -55,14 +55,18 @@ impl MetricReader for InMemoryMetricStore { return Ok(Vec::new()); }; - Ok(points.range(step_range).map(|(&s, &v)| (s, v)).collect()) + let raw_range = step_range.start.as_u64()..step_range.end.as_u64(); + Ok(points + .range(raw_range) + .map(|(&s, &v)| (Step::new(s), v)) + .collect()) } async fn count_points( &self, run_id: &RunId, key: &Metric, - step_range: Range, + step_range: Range, ) -> Result { let Some(run) = self.data.get(run_id) else { return Ok(0); @@ -71,7 +75,8 @@ impl MetricReader for InMemoryMetricStore { return Ok(0); }; - Ok(points.range(step_range).count()) + let raw_range = step_range.start.as_u64()..step_range.end.as_u64(); + Ok(points.range(raw_range).count()) } async fn list_metrics(&self, run_id: &RunId) -> Result, ReadError> { diff --git a/crates/store/src/memory/project.rs b/crates/store/src/memory/project.rs index 56f8a83..b450ad6 100644 --- a/crates/store/src/memory/project.rs +++ b/crates/store/src/memory/project.rs @@ -5,8 +5,7 @@ use dashmap::DashMap; use photon_core::domain::project::Project; use photon_core::types::id::ProjectId; -use crate::ports::project::{ProjectReader, ProjectWriter}; -use crate::ports::{ReadError, WriteError}; +use crate::ports::{ReadError, ReadRepository, WriteError, WriteRepository}; #[derive(Clone)] pub struct InMemoryProjectStore { @@ -27,8 +26,8 @@ impl Default for InMemoryProjectStore { } } -impl ProjectReader for InMemoryProjectStore { - async fn list_projects(&self) -> Result, ReadError> { +impl ReadRepository for InMemoryProjectStore { + async fn list(&self) -> Result, ReadError> { Ok(self .data .iter() @@ -36,13 +35,13 @@ impl ProjectReader for InMemoryProjectStore { .collect()) } - async fn get_project(&self, id: &ProjectId) -> Result, ReadError> { + async fn get(&self, id: &ProjectId) -> Result, ReadError> { Ok(self.data.get(id).map(|entry| entry.value().clone())) } } -impl ProjectWriter for InMemoryProjectStore { - async fn upsert_project(&self, project: &Project) -> Result<(), WriteError> { +impl WriteRepository for InMemoryProjectStore { + async fn upsert(&self, project: &Project) -> Result<(), WriteError> { self.data.insert(project.id, project.clone()); Ok(()) } diff --git a/crates/store/src/memory/run.rs b/crates/store/src/memory/run.rs index 7953e04..4c8e459 100644 --- a/crates/store/src/memory/run.rs +++ b/crates/store/src/memory/run.rs @@ -5,8 +5,7 @@ use dashmap::DashMap; use photon_core::domain::run::Run; use photon_core::types::id::RunId; -use crate::ports::run::{RunReader, RunWriter}; -use crate::ports::{ReadError, WriteError}; +use crate::ports::{ReadError, ReadRepository, WriteError, WriteRepository}; #[derive(Clone)] pub struct InMemoryRunStore { @@ -27,8 +26,8 @@ impl Default for InMemoryRunStore { } } -impl RunReader for InMemoryRunStore { - async fn list_runs(&self) -> Result, ReadError> { +impl ReadRepository for InMemoryRunStore { + async fn list(&self) -> Result, ReadError> { Ok(self .data .iter() @@ -36,14 +35,14 @@ impl RunReader for InMemoryRunStore { .collect()) } - async fn get_run(&self, run_id: &RunId) -> Result, ReadError> { + async fn get(&self, run_id: &RunId) -> Result, ReadError> { Ok(self.data.get(run_id).map(|entry| entry.value().clone())) } } -impl RunWriter for InMemoryRunStore { - async fn upsert_run(&self, run: &Run) -> Result<(), WriteError> { - self.data.insert(run.id, run.clone()); +impl WriteRepository for InMemoryRunStore { + async fn upsert(&self, run: &Run) -> Result<(), WriteError> { + self.data.insert(run.id(), run.clone()); Ok(()) } } diff --git a/crates/store/src/memory/watermark.rs b/crates/store/src/memory/watermark.rs index 1cf1a99..164ecf4 100644 --- a/crates/store/src/memory/watermark.rs +++ b/crates/store/src/memory/watermark.rs @@ -11,6 +11,12 @@ pub struct InMemoryWatermarkStore { inner: DashMap, } +impl Default for InMemoryWatermarkStore { + fn default() -> Self { + Self::new() + } +} + impl InMemoryWatermarkStore { pub fn new() -> Self { Self { diff --git a/crates/store/src/ports/bucket.rs b/crates/store/src/ports/bucket.rs index efdc4ff..2093c4b 100644 --- a/crates/store/src/ports/bucket.rs +++ b/crates/store/src/ports/bucket.rs @@ -3,7 +3,7 @@ use std::ops::Range; use photon_core::types::bucket::{Bucket, BucketEntry}; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use super::{ReadError, WriteError}; @@ -23,6 +23,6 @@ pub trait BucketReader: Send + Sync + Clone + 'static { run_id: &RunId, key: &Metric, tier: usize, - step_range: Range, + step_range: Range, ) -> impl Future, ReadError>> + Send; } diff --git a/crates/store/src/ports/compaction.rs b/crates/store/src/ports/compaction.rs index 35837d1..a0110e7 100644 --- a/crates/store/src/ports/compaction.rs +++ b/crates/store/src/ports/compaction.rs @@ -1,25 +1,24 @@ use std::future::Future; use photon_core::types::id::RunId; -use photon_core::types::metric::Metric; +use photon_core::types::metric::{Metric, Step}; use super::{ReadError, WriteError}; /// Tracks how far each tier has been compacted for a given run and metric. -/// The offset is a raw point count, not a step value. pub trait CompactionCursor: Send + Sync + Clone + 'static { fn get( &self, run_id: &RunId, key: &Metric, tier: usize, - ) -> impl Future, ReadError>> + Send; + ) -> impl Future, ReadError>> + Send; fn advance( &self, run_id: &RunId, key: &Metric, tier: usize, - offset: u64, + through: Step, ) -> impl Future> + Send; } diff --git a/crates/store/src/ports/experiment.rs b/crates/store/src/ports/experiment.rs deleted file mode 100644 index b6adcc9..0000000 --- a/crates/store/src/ports/experiment.rs +++ /dev/null @@ -1,24 +0,0 @@ -use std::future::Future; - -use photon_core::domain::experiment::Experiment; -use photon_core::types::id::ExperimentId; - -use super::{ReadError, WriteError}; - -/// Read access to experiment metadata. -pub trait ExperimentReader: Send + Sync + Clone + 'static { - fn list_experiments(&self) -> impl Future, ReadError>> + Send; - - fn get_experiment( - &self, - id: &ExperimentId, - ) -> impl Future, ReadError>> + Send; -} - -/// Write access to experiment metadata. -pub trait ExperimentWriter: Send + Sync + Clone + 'static { - fn upsert_experiment( - &self, - experiment: &Experiment, - ) -> impl Future> + Send; -} diff --git a/crates/store/src/ports/metric.rs b/crates/store/src/ports/metric.rs index 8b394b0..fad0168 100644 --- a/crates/store/src/ports/metric.rs +++ b/crates/store/src/ports/metric.rs @@ -2,7 +2,7 @@ use std::future::Future; use std::ops::Range; use photon_core::types::id::RunId; -use photon_core::types::metric::{Metric, MetricBatch}; +use photon_core::types::metric::{Metric, MetricBatch, Step}; use super::{ReadError, WriteError}; @@ -32,8 +32,8 @@ pub trait MetricReader: Send + Sync + Clone + 'static { &self, run_id: &RunId, key: &Metric, - step_range: Range, - ) -> impl Future, ReadError>> + Send; + step_range: Range, + ) -> impl Future, ReadError>> + Send; fn list_metrics( &self, @@ -44,6 +44,6 @@ pub trait MetricReader: Send + Sync + Clone + 'static { &self, run_id: &RunId, key: &Metric, - step_range: Range, + step_range: Range, ) -> impl Future> + Send; } diff --git a/crates/store/src/ports/mod.rs b/crates/store/src/ports/mod.rs index 99d25fa..a12c3a2 100644 --- a/crates/store/src/ports/mod.rs +++ b/crates/store/src/ports/mod.rs @@ -1,27 +1,29 @@ pub mod bucket; pub mod compaction; -pub mod experiment; pub mod metric; -pub mod project; -pub mod run; +pub mod repository; pub mod watermark; +pub use repository::{ReadRepository, WriteRepository}; + use photon_core::types::id::RunId; +#[non_exhaustive] #[derive(Debug, thiserror::Error)] pub enum WriteError { #[error("invalid batch for run {run_id}: {reason}")] InvalidBatch { run_id: RunId, reason: String }, - #[error(transparent)] - Unknown(#[from] anyhow::Error), + #[error("store error: {0}")] + Store(Box), } +#[non_exhaustive] #[derive(Debug, thiserror::Error)] pub enum ReadError { #[error("run {0} not found")] RunNotFound(RunId), - #[error(transparent)] - Unknown(#[from] anyhow::Error), + #[error("store error: {0}")] + Store(Box), } diff --git a/crates/store/src/ports/project.rs b/crates/store/src/ports/project.rs deleted file mode 100644 index 9b77dc3..0000000 --- a/crates/store/src/ports/project.rs +++ /dev/null @@ -1,24 +0,0 @@ -use std::future::Future; - -use photon_core::domain::project::Project; -use photon_core::types::id::ProjectId; - -use super::{ReadError, WriteError}; - -/// Read access to project metadata. -pub trait ProjectReader: Send + Sync + Clone + 'static { - fn list_projects(&self) -> impl Future, ReadError>> + Send; - - fn get_project( - &self, - id: &ProjectId, - ) -> impl Future, ReadError>> + Send; -} - -/// Write access to project metadata. -pub trait ProjectWriter: Send + Sync + Clone + 'static { - fn upsert_project( - &self, - project: &Project, - ) -> impl Future> + Send; -} diff --git a/crates/store/src/ports/repository.rs b/crates/store/src/ports/repository.rs new file mode 100644 index 0000000..cb270d7 --- /dev/null +++ b/crates/store/src/ports/repository.rs @@ -0,0 +1,17 @@ +use std::future::Future; + +use photon_core::domain::Entity; + +use super::{ReadError, WriteError}; + +/// Generic read access to entity metadata. +pub trait ReadRepository: Send + Sync + Clone + 'static { + fn list(&self) -> impl Future, ReadError>> + Send; + + fn get(&self, id: &T::Id) -> impl Future, ReadError>> + Send; +} + +/// Generic write access to entity metadata. +pub trait WriteRepository: Send + Sync + Clone + 'static { + fn upsert(&self, entity: &T) -> impl Future> + Send; +} diff --git a/crates/store/src/ports/run.rs b/crates/store/src/ports/run.rs deleted file mode 100644 index 6abe229..0000000 --- a/crates/store/src/ports/run.rs +++ /dev/null @@ -1,21 +0,0 @@ -use std::future::Future; - -use photon_core::domain::run::Run; -use photon_core::types::id::RunId; - -use super::{ReadError, WriteError}; - -/// Read access to run metadata. -pub trait RunReader: Send + Sync + Clone + 'static { - fn list_runs(&self) -> impl Future, ReadError>> + Send; - - fn get_run( - &self, - run_id: &RunId, - ) -> impl Future, ReadError>> + Send; -} - -/// Write access to run metadata. -pub trait RunWriter: Send + Sync + Clone + 'static { - fn upsert_run(&self, run: &Run) -> impl Future> + Send; -} diff --git a/crates/subscription/Cargo.toml b/crates/subscription/Cargo.toml index 44d28d6..84e1b57 100644 --- a/crates/subscription/Cargo.toml +++ b/crates/subscription/Cargo.toml @@ -13,3 +13,6 @@ photon-transport.workspace = true tokio.workspace = true tracing.workspace = true + +[lints] +workspace = true diff --git a/crates/subscription/src/handler.rs b/crates/subscription/src/handler.rs index eb13fc7..ae78f32 100644 --- a/crates/subscription/src/handler.rs +++ b/crates/subscription/src/handler.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use photon_core::types::id::RunId; use photon_core::types::subscription::{SubscriptionEvent, SubscriptionMessage}; -use photon_transport::ports::{Transport, TransportError}; +use photon_transport::ports::Transport; use tokio::sync::broadcast; pub async fn handle(transport: &T, mut events: broadcast::Receiver) @@ -21,7 +21,6 @@ where Ok(SubscriptionMessage::Unsubscribe(run_id)) => { subscribed_runs.remove(&run_id); } - Err(TransportError::StreamClosed(_)) => break, Err(_) => break, } } diff --git a/crates/subscription/src/hook.rs b/crates/subscription/src/hook.rs index 924b04f..6d30dc4 100644 --- a/crates/subscription/src/hook.rs +++ b/crates/subscription/src/hook.rs @@ -14,6 +14,12 @@ pub struct SubscriptionHook { seen_runs: std::sync::Arc>>, } +impl Default for SubscriptionHook { + fn default() -> Self { + Self::new() + } +} + impl SubscriptionHook { pub fn new() -> Self { Self { @@ -32,7 +38,7 @@ impl IngestHook for SubscriptionHook { let is_new = self .seen_runs .lock() - .unwrap_or_else(|e| e.into_inner()) + .unwrap_or_else(std::sync::PoisonError::into_inner) .insert(run_id); if is_new { diff --git a/crates/transport/Cargo.toml b/crates/transport/Cargo.toml index f0e2c4d..27099bc 100644 --- a/crates/transport/Cargo.toml +++ b/crates/transport/Cargo.toml @@ -15,9 +15,8 @@ bytes.workspace = true futures-util.workspace = true reqwest.workspace = true thiserror.workspace = true -anyhow.workspace = true -tracing.workspace = true async-trait.workspace = true +tracing.workspace = true [target.'cfg(not(target_arch = "wasm32"))'.dependencies] tokio.workspace = true @@ -27,3 +26,6 @@ axum = { workspace = true, features = ["ws"] } [target.'cfg(target_arch = "wasm32")'.dependencies] gloo-net.workspace = true wasm-bindgen-futures.workspace = true + +[lints] +workspace = true diff --git a/crates/transport/src/adapter/mod.rs b/crates/transport/src/adapter/mod.rs index b32c171..86c05f3 100644 --- a/crates/transport/src/adapter/mod.rs +++ b/crates/transport/src/adapter/mod.rs @@ -14,21 +14,16 @@ use self::websocket::WebSocketTransport; use crate::ports::{ByteTransport, TransportError}; /// Transport protocol selection. Call [`connect`](Self::connect) to create a connected transport. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Default)] pub enum TransportKind { #[cfg(not(target_arch = "wasm32"))] + #[default] Tcp, Http, WebSocket, } #[cfg(not(target_arch = "wasm32"))] -impl Default for TransportKind { - fn default() -> Self { - Self::Tcp - } -} - #[cfg(target_arch = "wasm32")] impl Default for TransportKind { fn default() -> Self { diff --git a/crates/transport/src/adapter/router.rs b/crates/transport/src/adapter/router.rs index 6148b74..d6bfde4 100644 --- a/crates/transport/src/adapter/router.rs +++ b/crates/transport/src/adapter/router.rs @@ -1,7 +1,6 @@ use std::future::Future; use std::sync::{Arc, Mutex}; -use async_trait::async_trait; use axum::extract::ws::{Message, WebSocket, WebSocketUpgrade}; use axum::http::StatusCode; use axum::response::IntoResponse; @@ -11,6 +10,8 @@ use tokio::net::TcpListener; use photon_protocol::codec::CodecKind; use super::websocket::WebSocketTransport; +use async_trait::async_trait; + use crate::codec::CodecTransport; use crate::ports::{ByteTransport, TransportError}; @@ -32,12 +33,12 @@ impl Router { F: Fn(CodecTransport) -> Fut + Clone + Send + Sync + 'static, Fut: Future + Send + 'static, { - let codec = self.codec.clone(); + let codec = self.codec; self.router = self.router.route( path, axum::routing::post(move |body: axum::body::Bytes| { let handler = handler.clone(); - let codec = codec.clone(); + let codec = codec; async move { let transport = BodyTransport::new(body.to_vec()); let response = transport.clone(); @@ -57,12 +58,12 @@ impl Router { F: Fn(CodecTransport) -> Fut + Clone + Send + Sync + 'static, Fut: Future + Send + 'static, { - let codec = self.codec.clone(); + let codec = self.codec; self.router = self.router.route( path, axum::routing::get(move |ws: WebSocketUpgrade| { let handler = handler.clone(); - let codec = codec.clone(); + let codec = codec; async move { ws.on_upgrade(move |socket| async move { let transport = ws_from_axum(socket); @@ -123,8 +124,7 @@ impl BodyTransport { } } -#[cfg_attr(not(target_arch = "wasm32"), async_trait)] -#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] +#[async_trait] impl ByteTransport for BodyTransport { async fn send_bytes(&self, bytes: &[u8]) -> Result<(), TransportError> { *self.response_body.lock().unwrap() = Some(bytes.to_vec()); diff --git a/crates/transport/src/adapter/websocket.rs b/crates/transport/src/adapter/websocket.rs index aacb916..6d8e873 100644 --- a/crates/transport/src/adapter/websocket.rs +++ b/crates/transport/src/adapter/websocket.rs @@ -1,8 +1,9 @@ use async_channel::{Receiver, Sender}; -use async_trait::async_trait; #[cfg(not(target_arch = "wasm32"))] use tokio::io::{AsyncRead, AsyncWrite}; +use async_trait::async_trait; + use crate::ports::{ByteTransport, TransportError}; const CHANNEL_CAPACITY: usize = 64; @@ -91,7 +92,7 @@ where while let Some(msg) = stream.next().await { match msg { Ok(Message::Binary(data)) => { - if in_tx.send(Ok(data.into())).await.is_err() { + if in_tx.send(Ok(data)).await.is_err() { break; } } @@ -107,7 +108,7 @@ where tokio::spawn(async move { while let Ok(data) = out_rx.recv().await { - if sink.send(Message::Binary(data.into())).await.is_err() { + if sink.send(Message::Binary(data)).await.is_err() { break; } } diff --git a/crates/transport/src/codec.rs b/crates/transport/src/codec.rs index 994ce5a..5f46ae2 100644 --- a/crates/transport/src/codec.rs +++ b/crates/transport/src/codec.rs @@ -1,6 +1,5 @@ use std::sync::Arc; -use async_trait::async_trait; use bytes::BytesMut; use photon_protocol::ports::codec::Codec; @@ -30,8 +29,6 @@ impl CodecTransport { } } -#[cfg_attr(not(target_arch = "wasm32"), async_trait)] -#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] impl Transport for CodecTransport where S: Send + Sync + 'static, diff --git a/crates/transport/src/lib.rs b/crates/transport/src/lib.rs index 44957f2..311fea0 100644 --- a/crates/transport/src/lib.rs +++ b/crates/transport/src/lib.rs @@ -24,7 +24,7 @@ use crate::adapter::tcp::TcpTransport; #[cfg(not(target_arch = "wasm32"))] use crate::codec::CodecTransport; -/// Accept TCP connections, wrap each in a CodecTransport, and dispatch to a handler. +/// Accept TCP connections, wrap each in a `CodecTransport`, and dispatch to a handler. #[cfg(not(target_arch = "wasm32"))] pub async fn serve(listener: TcpListener, codec: CodecKind, handler: F) where @@ -41,7 +41,6 @@ where }; let handler = handler.clone(); - let codec = codec.clone(); tokio::spawn(async move { tracing::trace!("accepted connection from {peer}"); diff --git a/crates/transport/src/ports.rs b/crates/transport/src/ports.rs index 023a40b..2a28495 100644 --- a/crates/transport/src/ports.rs +++ b/crates/transport/src/ports.rs @@ -1,5 +1,8 @@ +use std::future::Future; + use async_trait::async_trait; +#[non_exhaustive] #[derive(Debug, thiserror::Error)] pub enum TransportError { #[error("connection failed: {0}")] @@ -10,13 +13,11 @@ pub enum TransportError { #[error("stream closed: {0}")] StreamClosed(String), - - #[error(transparent)] - Unknown(#[from] anyhow::Error), } #[cfg(not(target_arch = "wasm32"))] impl TransportError { + #[allow(clippy::needless_pass_by_value)] pub(crate) fn from_io(e: std::io::Error) -> Self { use std::io::ErrorKind; match e.kind() { @@ -29,6 +30,7 @@ impl TransportError { } } +/// Low-level byte transport. #[cfg_attr(not(target_arch = "wasm32"), async_trait)] #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] pub trait ByteTransport: Send + Sync + 'static { @@ -48,9 +50,15 @@ impl ByteTransport for Box { } } -#[cfg_attr(not(target_arch = "wasm32"), async_trait)] -#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] +/// Typed transport over a codec. +#[cfg(not(target_arch = "wasm32"))] pub trait Transport: Clone + Send + Sync + 'static { - async fn send(&self, msg: &S) -> Result<(), TransportError>; - async fn recv(&self) -> Result; + fn send(&self, msg: &S) -> impl Future> + Send; + fn recv(&self) -> impl Future> + Send; +} + +#[cfg(target_arch = "wasm32")] +pub trait Transport: Clone + 'static { + fn send(&self, msg: &S) -> impl Future>; + fn recv(&self) -> impl Future>; } diff --git a/crates/uplink/Cargo.toml b/crates/uplink/Cargo.toml index 2325920..f4ee35e 100644 --- a/crates/uplink/Cargo.toml +++ b/crates/uplink/Cargo.toml @@ -13,7 +13,9 @@ photon-transport.workspace = true photon-wal.workspace = true thiserror.workspace = true -anyhow.workspace = true crossbeam-channel.workspace = true tokio.workspace = true tracing.workspace = true + +[lints] +workspace = true diff --git a/crates/uplink/src/domain/ack.rs b/crates/uplink/src/domain/ack.rs index 02207b3..929dcec 100644 --- a/crates/uplink/src/domain/ack.rs +++ b/crates/uplink/src/domain/ack.rs @@ -40,7 +40,7 @@ impl AckTracker { self.committed } - pub fn track(&mut self, ack: AckResult, stats: &mut UplinkStats) -> AckOutcome { + pub fn track(&mut self, ack: &AckResult, stats: &mut UplinkStats) -> AckOutcome { let seq = ack.sequence_number; match ack.status { diff --git a/crates/uplink/src/domain/error.rs b/crates/uplink/src/domain/error.rs index 07cc87a..63e904f 100644 --- a/crates/uplink/src/domain/error.rs +++ b/crates/uplink/src/domain/error.rs @@ -4,7 +4,7 @@ use photon_core::types::sequence::SequenceNumber; use photon_wal::ports::WalError; #[derive(Debug, thiserror::Error)] -pub enum TransportError { +pub enum UplinkTransportError { #[error("connection lost: {reason}")] ConnectionLost { reason: String }, @@ -16,9 +16,6 @@ pub enum TransportError { sequence: SequenceNumber, message: String, }, - - #[error(transparent)] - Unknown(#[from] anyhow::Error), } #[derive(Debug, thiserror::Error)] @@ -27,7 +24,7 @@ pub enum UplinkError { Wal(#[from] WalError), #[error("transport error")] - Transport(#[from] TransportError), + Transport(#[from] UplinkTransportError), #[error("shutdown timeout after {0:?}")] ShutdownTimeout(Duration), @@ -39,5 +36,5 @@ pub enum RecoveryError { Wal(#[from] WalError), #[error("transport error during recovery")] - Transport(#[from] TransportError), + Transport(#[from] UplinkTransportError), } diff --git a/crates/uplink/src/domain/ports.rs b/crates/uplink/src/domain/ports.rs index 9de013f..cf784e4 100644 --- a/crates/uplink/src/domain/ports.rs +++ b/crates/uplink/src/domain/ports.rs @@ -5,23 +5,23 @@ use photon_core::types::id::RunId; use photon_core::types::ingest::{IngestMessage, IngestResult}; use photon_core::types::sequence::SequenceNumber; -use super::error::TransportError; +use super::error::UplinkTransportError; pub trait IngestConnection: Clone + Send + Sync + 'static { fn send_batch( &self, batch: &WireBatch, - ) -> impl Future> + Send; + ) -> impl Future> + Send; fn send_message( &self, msg: IngestMessage, - ) -> impl Future> + Send; + ) -> impl Future> + Send; fn query_watermark( &self, run_id: &RunId, - ) -> impl Future> + Send; + ) -> impl Future> + Send; - fn recv(&self) -> impl Future> + Send; + fn recv(&self) -> impl Future> + Send; } diff --git a/crates/uplink/src/domain/service.rs b/crates/uplink/src/domain/service.rs index a3379f9..b204e07 100644 --- a/crates/uplink/src/domain/service.rs +++ b/crates/uplink/src/domain/service.rs @@ -114,7 +114,7 @@ where fn handle_ack(&mut self, ack: AckResult) -> Result<(), UplinkError> { let before = self.tracker.committed(); - let outcome = self.tracker.track(ack, &mut self.stats); + let outcome = self.tracker.track(&ack, &mut self.stats); if outcome.new_watermark.is_some() { let after = self.tracker.committed(); diff --git a/crates/uplink/src/inbound/adapter.rs b/crates/uplink/src/inbound/adapter.rs index 648041a..f2eb03e 100644 --- a/crates/uplink/src/inbound/adapter.rs +++ b/crates/uplink/src/inbound/adapter.rs @@ -4,16 +4,18 @@ use photon_core::types::ingest::{IngestMessage, IngestResult}; use photon_core::types::sequence::SequenceNumber; use photon_transport::ports::{Transport, TransportError as InfraTransportError}; -use crate::domain::error::TransportError; +use crate::domain::error::UplinkTransportError; use crate::domain::ports::IngestConnection; -impl From for TransportError { +impl From for UplinkTransportError { fn from(e: InfraTransportError) -> Self { match e { InfraTransportError::Connection(msg) | InfraTransportError::Request(msg) | InfraTransportError::StreamClosed(msg) => Self::ConnectionLost { reason: msg }, - InfraTransportError::Unknown(e) => Self::Unknown(e), + other => Self::ConnectionLost { + reason: other.to_string(), + }, } } } @@ -22,25 +24,28 @@ impl IngestConnection for T where T: Transport, { - async fn send_batch(&self, batch: &WireBatch) -> Result<(), TransportError> { + async fn send_batch(&self, batch: &WireBatch) -> Result<(), UplinkTransportError> { self.send(&IngestMessage::Batch(batch.clone())) .await .map_err(Into::into) } - async fn send_message(&self, msg: IngestMessage) -> Result<(), TransportError> { + async fn send_message(&self, msg: IngestMessage) -> Result<(), UplinkTransportError> { self.send(&msg).await.map_err(Into::into) } - async fn query_watermark(&self, run_id: &RunId) -> Result { + async fn query_watermark( + &self, + run_id: &RunId, + ) -> Result { self.send(&IngestMessage::QueryWatermark(*run_id)) .await - .map_err(TransportError::from)?; + .map_err(UplinkTransportError::from)?; match self.recv().await { Ok(IngestResult::Watermark(seq)) => Ok(seq), Ok(IngestResult::Error(e)) => { - tracing::warn!("watermark query rejected by server: {e}"); + tracing::warn!("watermark query rejected by server: {e:?}"); Ok(SequenceNumber::ZERO) } Ok(_) => Ok(SequenceNumber::ZERO), @@ -48,7 +53,7 @@ where } } - async fn recv(&self) -> Result { + async fn recv(&self) -> Result { Transport::recv(self).await.map_err(Into::into) } } diff --git a/crates/uplink/src/inbound/state.rs b/crates/uplink/src/inbound/state.rs index 3d7afdb..85ee4e4 100644 --- a/crates/uplink/src/inbound/state.rs +++ b/crates/uplink/src/inbound/state.rs @@ -22,7 +22,7 @@ enum Phase { Shutdown, } -pub struct ConnectionState { +pub(crate) struct ConnectionState { phase: Phase, in_flight: BTreeMap, draining_since: Option, @@ -33,7 +33,7 @@ pub struct ConnectionState { } impl ConnectionState { - pub fn new(config: &UplinkConfig) -> Self { + pub(crate) fn new(config: &UplinkConfig) -> Self { Self { phase: Phase::Connected, in_flight: BTreeMap::new(), @@ -45,19 +45,19 @@ impl ConnectionState { } } - pub fn is_shutdown(&self) -> bool { + pub(crate) fn is_shutdown(&self) -> bool { self.phase == Phase::Shutdown } - pub fn can_send(&self) -> bool { + pub(crate) fn can_send(&self) -> bool { self.phase == Phase::Connected && self.in_flight.len() < self.max_in_flight } - pub fn in_flight_empty(&self) -> bool { + pub(crate) fn in_flight_empty(&self) -> bool { self.in_flight.is_empty() } - pub fn record_sent(&mut self, seq: SequenceNumber) { + pub(crate) fn record_sent(&mut self, seq: SequenceNumber) { self.in_flight.insert( seq, InFlightEntry { @@ -66,11 +66,11 @@ impl ConnectionState { ); } - pub fn record_acked(&mut self, seq: SequenceNumber) { + pub(crate) fn record_acked(&mut self, seq: SequenceNumber) { self.in_flight.remove(&seq); } - pub fn enter_reconnecting(&mut self) { + pub(crate) fn enter_reconnecting(&mut self) { tracing::warn!( in_flight = self.in_flight.len(), "connection lost, entering reconnection" @@ -81,7 +81,7 @@ impl ConnectionState { }; } - pub fn reconnected(&mut self) { + pub(crate) fn reconnected(&mut self) { tracing::info!( "reconnected, replaying {} in-flight batches", self.in_flight.len() @@ -90,7 +90,7 @@ impl ConnectionState { self.phase = Phase::Connected; } - pub fn reconnect_due(&self) -> Option { + pub(crate) fn reconnect_due(&self) -> Option { match &self.phase { Phase::Reconnecting { attempt, next_at } if Instant::now() >= *next_at => { Some(*attempt) @@ -99,7 +99,7 @@ impl ConnectionState { } } - pub fn schedule_next_reconnect(&mut self, attempt: u32) { + pub(crate) fn schedule_next_reconnect(&mut self, attempt: u32) { let delay = backoff_delay(&self.retry, attempt); tracing::info!( attempt = attempt + 1, @@ -112,7 +112,7 @@ impl ConnectionState { }; } - pub fn check_timeouts(&mut self) -> Option { + pub(crate) fn check_timeouts(&mut self) -> Option { if self.in_flight.is_empty() { return None; } @@ -131,11 +131,11 @@ impl ConnectionState { } } - pub fn shutdown(&mut self) { + pub(crate) fn shutdown(&mut self) { self.phase = Phase::Shutdown; } - pub fn check_drain_timeout(&mut self) -> Option { + pub(crate) fn check_drain_timeout(&mut self) -> Option { let since = *self.draining_since.get_or_insert(Instant::now()); if since.elapsed() > self.shutdown_timeout { Some(since.elapsed()) @@ -144,17 +144,17 @@ impl ConnectionState { } } - pub fn reset_drain(&mut self) { + pub(crate) fn reset_drain(&mut self) { self.draining_since = None; } } -pub enum ReconnectResult { +pub(crate) enum ReconnectResult { Ok, Failed, } -pub async fn try_reconnect( +pub(crate) async fn try_reconnect( wal: &M, connection: &C, wal_cursor: WalOffset, diff --git a/crates/uplink/src/inbound/thread.rs b/crates/uplink/src/inbound/thread.rs index 093177c..a9a2e39 100644 --- a/crates/uplink/src/inbound/thread.rs +++ b/crates/uplink/src/inbound/thread.rs @@ -13,7 +13,7 @@ use tokio::sync::oneshot; use super::state::{ConnectionState, ReconnectResult, try_reconnect}; use crate::UplinkThreadError; use crate::domain::ack::UplinkStats; -use crate::domain::error::{TransportError, UplinkError}; +use crate::domain::error::{UplinkError, UplinkTransportError}; use crate::domain::ports::IngestConnection; use crate::domain::service::{Service, UplinkService}; @@ -32,7 +32,7 @@ where C: IngestConnection, M: Wal + Clone, { - let mut service = Service::new(connection.clone(), wal.clone(), run.id, start_sequence); + let mut service = Service::new(connection.clone(), wal.clone(), run.id(), start_sequence); service.recover().await?; let _ = connection @@ -76,36 +76,35 @@ where continue; } - if let Some(batch) = batch { - if conn.can_send() { - match service.send(&batch).await { - Ok(()) => conn.record_sent(batch.sequence_number), - Err(UplinkError::Transport(TransportError::ConnectionLost { .. })) => { - conn.enter_reconnecting(); - continue; - } - Err(e) => return Err(e.into()), + if let Some(batch) = batch + && conn.can_send() + { + match service.send(&batch).await { + Ok(()) => conn.record_sent(batch.sequence_number), + Err(UplinkError::Transport(UplinkTransportError::ConnectionLost { .. })) => { + conn.enter_reconnecting(); + continue; } + Err(e) => return Err(e.into()), } } match tokio::time::timeout(Duration::from_millis(1), connection.recv()).await { - Err(_) => {} Ok(Ok(IngestResult::Ack(ack))) => { conn.record_acked(ack.sequence_number); service.handle_ack(ack)?; } Ok(Ok(IngestResult::Error(e))) => { - tracing::warn!("ingest error from server: {e}"); + tracing::warn!("ingest error from server: {e:?}"); } - Ok(Ok(_)) => {} Ok(Err(e)) => match &e { - TransportError::ConnectionLost { .. } => { + UplinkTransportError::ConnectionLost { .. } => { conn.enter_reconnecting(); continue; } _ => return Err(UplinkError::from(e).into()), }, + _ => {} } if conn.check_timeouts().is_some() { diff --git a/crates/uplink/src/lib.rs b/crates/uplink/src/lib.rs index af02626..5263949 100644 --- a/crates/uplink/src/lib.rs +++ b/crates/uplink/src/lib.rs @@ -2,7 +2,7 @@ pub mod domain; pub mod inbound; pub use domain::ack::UplinkStats; -pub use domain::error::{RecoveryError, TransportError, UplinkError}; +pub use domain::error::{RecoveryError, UplinkError, UplinkTransportError}; pub use domain::service::UplinkService; pub use inbound::thread::run_uplink; @@ -12,11 +12,9 @@ pub enum UplinkThreadError { #[error("failed to create uplink runtime")] Runtime(#[source] std::io::Error), #[error("uplink connection failed")] - Connection(#[from] TransportError), + Connection(#[from] UplinkTransportError), #[error("WAL recovery failed")] Recovery(#[from] RecoveryError), #[error("uplink run loop failed")] Uplink(#[from] UplinkError), - #[error("transport error")] - Transport(TransportError), } diff --git a/crates/wal/Cargo.toml b/crates/wal/Cargo.toml index ff0d4ff..44bb9f8 100644 --- a/crates/wal/Cargo.toml +++ b/crates/wal/Cargo.toml @@ -11,11 +11,12 @@ publish = true photon-core.workspace = true thiserror.workspace = true -anyhow.workspace = true bytes.workspace = true serde.workspace = true serde_json.workspace = true crc32fast.workspace = true tracing.workspace = true uuid.workspace = true -dyn-clone.workspace = true + +[lints] +workspace = true diff --git a/crates/wal/src/disk/mod.rs b/crates/wal/src/disk/mod.rs index 5a0e4e7..7bb21c4 100644 --- a/crates/wal/src/disk/mod.rs +++ b/crates/wal/src/disk/mod.rs @@ -76,11 +76,12 @@ pub fn open_disk_wal( let next_offset = consumed.advance(to_replay); - let bytes_used_val = active.bytes_used() + sealed.iter().map(|s| s.bytes_used()).sum::(); + let bytes_used_val = + active.bytes_used() + sealed.iter().map(segment::Segment::bytes_used).sum::(); let next_segment = sealed .iter() - .map(|s| s.index()) + .map(segment::Segment::index) .chain(std::iter::once(active.index())) .max() .unwrap_or(SegmentIndex::ZERO) @@ -154,9 +155,8 @@ impl DiskWalAppender { } fn enforce_budget(&mut self) -> Result<(), WalError> { - let budget = match self.config.wal.max_total_size { - Some(b) => b, - None => return Ok(()), + let Some(budget) = self.config.wal.max_total_size else { + return Ok(()); }; let used = self.bytes_used.load(Ordering::Relaxed); @@ -238,7 +238,7 @@ impl DiskWalManager { cursor: u64::from(cursor), consumed: u64::from(consumed), }) - .map_err(|e| WalError::Unknown(e.into()))?; + .map_err(|e| WalError::Serialization(Box::new(e)))?; let tmp = self.dir.join(".wal.meta.tmp"); fs::write(&tmp, json.as_bytes())?; @@ -294,7 +294,7 @@ impl Wal for DiskWalManager { } /// Record the consumer's position, then delete fully consumed sealed segments. - fn truncate_through(&mut self, offset: WalOffset) -> Result<(), WalError> { + fn truncate_through(&self, offset: WalOffset) -> Result<(), WalError> { *self.consumed.lock().unwrap() = offset; let mut cursor = self.cursor.lock().unwrap(); @@ -334,7 +334,7 @@ impl Wal for DiskWalManager { }) } - fn delete_all(&mut self) -> Result<(), WalError> { + fn delete_all(&self) -> Result<(), WalError> { fs::remove_dir_all(&self.dir)?; Ok(()) } diff --git a/crates/wal/src/disk/segment.rs b/crates/wal/src/disk/segment.rs index 480fd6b..d37b74b 100644 --- a/crates/wal/src/disk/segment.rs +++ b/crates/wal/src/disk/segment.rs @@ -25,18 +25,18 @@ const OFF_CREATED_AT: usize = 36; const OFF_POINT_COUNT: usize = 44; const OFF_UNCOMPRESSED: usize = 48; -pub trait SegmentPhase {} +pub(crate) trait SegmentPhase {} -pub struct Active; +pub(crate) struct Active; impl SegmentPhase for Active {} -pub struct Sealed; +pub(crate) struct Sealed; impl SegmentPhase for Sealed {} -pub struct Acked; +pub(crate) struct Acked; impl SegmentPhase for Acked {} -pub struct Segment { +pub(crate) struct Segment { path: PathBuf, index: SegmentIndex, file: File, @@ -47,7 +47,7 @@ pub struct Segment { } impl Segment { - pub fn create(dir: &Path, index: SegmentIndex, capacity: u64) -> Result { + pub(crate) fn create(dir: &Path, index: SegmentIndex, capacity: u64) -> Result { let path = segment_path(dir, index); let file = OpenOptions::new() @@ -68,7 +68,7 @@ impl Segment { } /// Reopen an existing segment for crash recovery. - pub fn open_for_recovery( + pub(crate) fn open_for_recovery( dir: &Path, index: SegmentIndex, capacity: u64, @@ -90,7 +90,11 @@ impl Segment { Ok(seg) } - pub fn append(&mut self, batch: &WireBatch, rotation_threshold: f64) -> Result { + pub(crate) fn append( + &mut self, + batch: &WireBatch, + rotation_threshold: f64, + ) -> Result { let payload = &batch.compressed_payload; let total = RECORD_OVERHEAD as u64 + payload.len() as u64; @@ -135,20 +139,21 @@ impl Segment { Ok(self.write_offset as f64 / self.capacity as f64 > rotation_threshold) } - pub fn flush(&self) -> Result<(), WalError> { + pub(crate) fn flush(&self) -> Result<(), WalError> { self.file.sync_data().map_err(Into::into) } /// No explicit sync as the OS flushes page cache - pub fn flush_async(&self) -> Result<(), WalError> { + #[allow(clippy::unused_self, clippy::unnecessary_wraps)] + pub(crate) fn flush_async(&self) -> Result<(), WalError> { Ok(()) } - pub fn seal(self) -> Segment { + pub(crate) fn seal(self) -> Segment { self.transition() } - /// Walk from offset 0, validate each record's CRC, set write_offset + /// Walk from offset 0, validate each record's CRC, set `write_offset` /// after the last valid one. Truncate any trailing partial record. fn scan_valid_extent(&mut self) -> Result<(), WalError> { self.file.seek(SeekFrom::Start(0))?; @@ -205,13 +210,13 @@ impl Segment { } impl Segment { - pub fn mark_acked(self) -> Segment { + pub(crate) fn mark_acked(self) -> Segment { self.transition() } } impl Segment { - pub fn delete(self) -> io::Result<()> { + pub(crate) fn delete(self) -> io::Result<()> { let path = self.path.clone(); drop(self.file); fs::remove_file(&path) @@ -219,15 +224,15 @@ impl Segment { } impl Segment { - pub fn index(&self) -> SegmentIndex { + pub(crate) fn index(&self) -> SegmentIndex { self.index } - pub fn bytes_used(&self) -> u64 { + pub(crate) fn bytes_used(&self) -> u64 { self.write_offset } /// Count records without reading payloads into memory. - pub fn record_count(&self) -> Result { + pub(crate) fn record_count(&self) -> Result { let mut reader = BufReader::new(&self.file); reader.seek(SeekFrom::Start(0))?; @@ -260,7 +265,7 @@ impl Segment { } /// Read all valid records from the segment. - pub fn read_records(&self) -> Result, WalError> { + pub(crate) fn read_records(&self) -> Result, WalError> { let mut reader = BufReader::new(&self.file); reader.seek(SeekFrom::Start(0))?; diff --git a/crates/wal/src/lib.rs b/crates/wal/src/lib.rs index 4313c10..98e4a90 100644 --- a/crates/wal/src/lib.rs +++ b/crates/wal/src/lib.rs @@ -9,6 +9,7 @@ pub use self::memory::{InMemoryWalAppender, InMemoryWalManager, open_in_memory_w pub use self::ports::{Wal, WalAppender, WalError}; use std::path::PathBuf; +use std::sync::Arc; /// WAL backend selection. Call [`open`](Self::open) to create appender and manager. #[derive(Clone, Copy, Debug, Default)] @@ -23,15 +24,15 @@ impl WalKind { self, dir: impl Into, config: DiskWalConfig, - ) -> Result<(Box, Box), WalError> { + ) -> Result<(Box, Arc), WalError> { match self { Self::Disk => { let (a, m) = open_disk_wal(dir, config)?; - Ok((Box::new(a), Box::new(m))) + Ok((Box::new(a), Arc::new(m))) } Self::Memory => { let (a, m) = open_in_memory_wal(); - Ok((Box::new(a), Box::new(m))) + Ok((Box::new(a), Arc::new(m))) } } } diff --git a/crates/wal/src/memory.rs b/crates/wal/src/memory.rs index b1eff39..4a49f29 100644 --- a/crates/wal/src/memory.rs +++ b/crates/wal/src/memory.rs @@ -15,7 +15,7 @@ pub struct InMemoryWalAppender { #[derive(Clone)] pub struct InMemoryWalManager { batches: Arc>>, - cursor: WalOffset, + cursor: Arc>, } pub fn open_in_memory_wal() -> (InMemoryWalAppender, InMemoryWalManager) { @@ -26,7 +26,7 @@ pub fn open_in_memory_wal() -> (InMemoryWalAppender, InMemoryWalManager) { }, InMemoryWalManager { batches, - cursor: WalOffset::ZERO, + cursor: Arc::new(Mutex::new(WalOffset::ZERO)), }, ) } @@ -43,15 +43,16 @@ impl Wal for InMemoryWalManager { Ok(()) } - fn truncate_through(&mut self, offset: WalOffset) -> Result<(), WalError> { - let cursor_val = u64::from(self.cursor); + fn truncate_through(&self, offset: WalOffset) -> Result<(), WalError> { + let mut cursor = self.cursor.lock().unwrap(); + let cursor_val = u64::from(*cursor); let new_val = u64::from(offset); let to_remove = (new_val - cursor_val) as usize; let mut batches = self.batches.lock().unwrap(); let n = to_remove.min(batches.len()); batches.drain(..n); - self.cursor = offset; + *cursor = offset; Ok(()) } @@ -60,7 +61,8 @@ impl Wal for InMemoryWalManager { } fn read_from(&self, offset: WalOffset) -> Result, WalError> { - let cursor_val = u64::from(self.cursor); + let cursor = self.cursor.lock().unwrap(); + let cursor_val = u64::from(*cursor); let offset_val = u64::from(offset); let skip = (offset_val - cursor_val) as usize; @@ -69,15 +71,16 @@ impl Wal for InMemoryWalManager { } fn read_meta(&self) -> Result { + let cursor = *self.cursor.lock().unwrap(); Ok(WalMeta { - cursor: self.cursor, - consumed: self.cursor, + cursor, + consumed: cursor, }) } - fn delete_all(&mut self) -> Result<(), WalError> { + fn delete_all(&self) -> Result<(), WalError> { self.batches.lock().unwrap().clear(); - self.cursor = WalOffset::ZERO; + *self.cursor.lock().unwrap() = WalOffset::ZERO; Ok(()) } } diff --git a/crates/wal/src/ports.rs b/crates/wal/src/ports.rs index 61824b8..4b503a2 100644 --- a/crates/wal/src/ports.rs +++ b/crates/wal/src/ports.rs @@ -1,4 +1,3 @@ -use dyn_clone::DynClone; use photon_core::types::batch::WireBatch; use photon_core::types::config::WalMeta; use photon_core::types::wal::{SegmentIndex, WalOffset}; @@ -6,10 +5,10 @@ use photon_core::types::wal::{SegmentIndex, WalOffset}; /// WAL lifecycle and read/truncate operations. /// Split from [`WalAppender`] so the append path has exclusive `&mut self` access /// without contention from the reader/truncator on another thread. -pub trait Wal: DynClone + Send + 'static { +pub trait Wal: Send + Sync + 'static { fn close(&self) -> Result<(), WalError>; - fn truncate_through(&mut self, offset: WalOffset) -> Result<(), WalError>; + fn truncate_through(&self, offset: WalOffset) -> Result<(), WalError>; fn sync(&self) -> Result<(), WalError>; @@ -17,21 +16,25 @@ pub trait Wal: DynClone + Send + 'static { fn read_meta(&self) -> Result; - fn delete_all(&mut self) -> Result<(), WalError>; + fn delete_all(&self) -> Result<(), WalError>; } -dyn_clone::clone_trait_object!(Wal); - pub trait WalAppender: Send + 'static { fn append(&mut self, batch: &WireBatch) -> Result<(), WalError>; } -impl Wal for Box { +impl WalAppender for Box { + fn append(&mut self, batch: &WireBatch) -> Result<(), WalError> { + (**self).append(batch) + } +} + +impl Wal for std::sync::Arc { fn close(&self) -> Result<(), WalError> { (**self).close() } - fn truncate_through(&mut self, offset: WalOffset) -> Result<(), WalError> { + fn truncate_through(&self, offset: WalOffset) -> Result<(), WalError> { (**self).truncate_through(offset) } @@ -47,17 +50,12 @@ impl Wal for Box { (**self).read_meta() } - fn delete_all(&mut self) -> Result<(), WalError> { + fn delete_all(&self) -> Result<(), WalError> { (**self).delete_all() } } -impl WalAppender for Box { - fn append(&mut self, batch: &WireBatch) -> Result<(), WalError> { - (**self).append(batch) - } -} - +#[non_exhaustive] #[derive(Debug, thiserror::Error)] pub enum WalError { #[error("corrupt segment {index} at byte offset {offset}")] @@ -69,6 +67,6 @@ pub enum WalError { #[error(transparent)] Io(#[from] std::io::Error), - #[error(transparent)] - Unknown(#[from] anyhow::Error), + #[error("serialization error: {0}")] + Serialization(Box), }