From a07da76d102789b1567138e5ac452dea2a2b8284 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sat, 1 Aug 2026 20:14:04 -0700 Subject: [PATCH 01/11] db: add count_pipelines_needing_compilation Counts stopped pipelines whose program status is pending, compiling_sql, sql_compiled or compiling_rust: exactly the union of the four compiler worker queries across shards and platform versions, so the count is nonzero if and only if some compiler worker would act. The enterprise runner uses it to derive compiler autoscaling demand. Signed-off-by: Gerd Zellweger --- .../src/db/operations/pipeline.rs | 24 ++ crates/pipeline-manager/src/db/storage.rs | 15 + .../src/db/storage_postgres.rs | 15 + crates/pipeline-manager/src/db/test.rs | 264 ++++++++++++++++++ 4 files changed, 318 insertions(+) diff --git a/crates/pipeline-manager/src/db/operations/pipeline.rs b/crates/pipeline-manager/src/db/operations/pipeline.rs index 89d370b61bd..37212396476 100644 --- a/crates/pipeline-manager/src/db/operations/pipeline.rs +++ b/crates/pipeline-manager/src/db/operations/pipeline.rs @@ -2024,6 +2024,30 @@ pub(crate) async fn get_next_rust_compilation( } } +/// Counts pipelines with outstanding compilation work. +/// +/// The predicate must remain the union of the predicates of the four worker +/// queries above (the two `_needing_*_compilation_clear` lists and the two +/// `get_next_*_compilation` pickers) across all shards and platform versions: +/// the count is greater than zero if and only if some compiler worker would +/// act. It drives compiler autoscaling in the enterprise runner. +pub(crate) async fn count_pipelines_needing_compilation( + txn: &Transaction<'_>, +) -> Result { + let stmt = txn + .prepare_cached( + "SELECT COUNT(*) + FROM pipeline AS p + WHERE p.deployment_resources_status = 'stopped' + AND p.program_status IN ('pending', 'compiling_sql', 'sql_compiled', 'compiling_rust') + ", + ) + .await?; + let row = txn.query_one(&stmt, &[]).await?; + let count: i64 = row.get(0); + Ok(count as u64) +} + /// Retrieves the list of successfully compiled pipeline programs (pipeline identifier, program version, /// program binary source checksum, program binary integrity checksum, program info integrity checksum) AND pipeline programs that /// are currently being compiled (pipeline identifier, program version) across all tenants. diff --git a/crates/pipeline-manager/src/db/storage.rs b/crates/pipeline-manager/src/db/storage.rs index 2930734a0ee..2f3233862bd 100644 --- a/crates/pipeline-manager/src/db/storage.rs +++ b/crates/pipeline-manager/src/db/storage.rs @@ -654,6 +654,21 @@ pub(crate) trait Storage { total_workers: usize, ) -> Result, DBError>; + /// Counts pipelines with outstanding compilation work: stopped pipelines + /// whose program status is `Pending`, `CompilingSql`, `SqlCompiled`, or + /// `CompilingRust`. + /// + /// Invariant: the predicate is exactly the union of the predicates of the + /// four worker queries + /// (`list_pipelines_across_all_tenants_needing_sql_compilation_clear`, + /// `list_pipelines_across_all_tenants_needing_rust_compilation_clear`, + /// `get_next_sql_compilation` and `get_next_rust_compilation`) across all + /// shards and platform versions, so the count is greater than zero if and + /// only if some compiler worker would act. If you change one of those four + /// queries you must keep this count in sync. It drives compiler + /// autoscaling in the enterprise runner. + async fn count_pipelines_needing_compilation(&self) -> Result; + /// Retrieves the list of fully compiled pipeline programs (pipeline identifier, program version, /// program binary source checksum, program binary integrity checksum) AND pipeline programs that /// are currently being compiled (pipeline identifier, program version) across all tenants. diff --git a/crates/pipeline-manager/src/db/storage_postgres.rs b/crates/pipeline-manager/src/db/storage_postgres.rs index 5f355abafa7..39cc6d15a22 100644 --- a/crates/pipeline-manager/src/db/storage_postgres.rs +++ b/crates/pipeline-manager/src/db/storage_postgres.rs @@ -1542,6 +1542,14 @@ impl Storage for StoragePostgres { Ok(next_pipeline_program) } + async fn count_pipelines_needing_compilation(&self) -> Result { + let mut client = self.pool.get().await?; + let txn = client.transaction().await?; + let count = operations::pipeline::count_pipelines_needing_compilation(&txn).await?; + txn.commit().await?; + Ok(count) + } + async fn list_pipeline_programs_across_all_tenants( &self, ) -> Result< @@ -1824,6 +1832,13 @@ impl Storage for StoragePostgres { } impl StoragePostgres { + /// Public mirror of the `Storage::count_pipelines_needing_compilation` + /// trait method: the enterprise runner derives compiler autoscaling demand + /// from it but cannot name the crate-private `Storage` trait. + pub async fn count_pipelines_needing_compilation(&self) -> Result { + ::count_pipelines_needing_compilation(self).await + } + pub async fn connect( db_config: &DatabaseConfig, #[cfg(feature = "postgresql_embedded")] pg_embed_config: PgEmbedConfig, diff --git a/crates/pipeline-manager/src/db/test.rs b/crates/pipeline-manager/src/db/test.rs index ca852a05c07..1724bfcfe8c 100644 --- a/crates/pipeline-manager/src/db/test.rs +++ b/crates/pipeline-manager/src/db/test.rs @@ -2390,6 +2390,245 @@ async fn pipeline_program_compilation() { ); } +/// Checks that `count_pipelines_needing_compilation` counts exactly the stopped +/// pipelines whose program status is pending, compiling_sql, sql_compiled or +/// compiling_rust. +#[tokio::test] +async fn count_pipelines_needing_compilation() { + async fn new_test_pipeline( + db: &StoragePostgres, + tenant_id: TenantId, + name: &str, + ) -> ExtendedPipelineDescr { + db.new_pipeline( + tenant_id, + Uuid::now_v7(), + "v0", + PipelineDescr { + name: name.to_string(), + description: "".to_string(), + tags: vec![], + runtime_config: json!({}), + program_code: "".to_string(), + udf_rust: "".to_string(), + udf_toml: "".to_string(), + program_config: json!({}), + }, + ) + .await + .unwrap() + } + + async fn advance_program_status( + db: &StoragePostgres, + tenant_id: TenantId, + pipeline_id: PipelineId, + statuses: &[ProgramStatus], + program_info: &serde_json::Value, + ) { + let sql_compilation = SqlCompilationInfo { + exit_code: 0, + messages: vec![], + }; + let rust_compilation = RustCompilationInfo { + exit_code: 0, + stdout: "".to_string(), + stderr: "".to_string(), + }; + for status in statuses { + match status { + ProgramStatus::Pending => db + .transit_program_status_to_pending(tenant_id, pipeline_id, Version(1)) + .await + .unwrap(), + ProgramStatus::CompilingSql => db + .transit_program_status_to_compiling_sql(tenant_id, pipeline_id, Version(1)) + .await + .unwrap(), + ProgramStatus::SqlCompiled => db + .transit_program_status_to_sql_compiled( + tenant_id, + pipeline_id, + Version(1), + &sql_compilation, + program_info, + ) + .await + .unwrap(), + ProgramStatus::CompilingRust => db + .transit_program_status_to_compiling_rust(tenant_id, pipeline_id, Version(1)) + .await + .unwrap(), + ProgramStatus::Success => db + .transit_program_status_to_success( + tenant_id, + pipeline_id, + Version(1), + &rust_compilation, + "abc", + "123", + "456", + ) + .await + .unwrap(), + ProgramStatus::SqlError => db + .transit_program_status_to_sql_error( + tenant_id, + pipeline_id, + Version(1), + &sql_compilation, + ) + .await + .unwrap(), + ProgramStatus::RustError => db + .transit_program_status_to_rust_error( + tenant_id, + pipeline_id, + Version(1), + &rust_compilation, + ) + .await + .unwrap(), + ProgramStatus::SystemError => db + .transit_program_status_to_system_error( + tenant_id, + pipeline_id, + Version(1), + "system error", + ) + .await + .unwrap(), + } + } + } + + let handle = test_setup().await; + let tenant_id = TenantRecord::default().id; + let program_info = serde_json::to_value(ProgramInfo { + schema: serde_json::to_value(ProgramSchema { + inputs: vec![], + outputs: vec![], + }) + .unwrap(), + main_rust: "".to_string(), + udf_stubs: "".to_string(), + input_connectors: BTreeMap::new(), + output_connectors: BTreeMap::new(), + dataflow: None, + }) + .unwrap(); + + assert_eq!( + handle + .db + .count_pipelines_needing_compilation() + .await + .unwrap(), + 0 + ); + + // One counted pipeline per status with outstanding compilation work + let p_pending = new_test_pipeline(&handle.db, tenant_id, "counted-pending").await; + for (name, statuses) in [ + ("counted-compiling-sql", vec![ProgramStatus::CompilingSql]), + ( + "counted-sql-compiled", + vec![ProgramStatus::CompilingSql, ProgramStatus::SqlCompiled], + ), + ( + "counted-compiling-rust", + vec![ + ProgramStatus::CompilingSql, + ProgramStatus::SqlCompiled, + ProgramStatus::CompilingRust, + ], + ), + ] { + let pipeline = new_test_pipeline(&handle.db, tenant_id, name).await; + advance_program_status(&handle.db, tenant_id, pipeline.id, &statuses, &program_info).await; + } + assert_eq!( + handle + .db + .count_pipelines_needing_compilation() + .await + .unwrap(), + 4 + ); + + // Terminal statuses are counted while in flight, no longer once reached + for (name, statuses) in [ + ( + "done-success", + vec![ + ProgramStatus::CompilingSql, + ProgramStatus::SqlCompiled, + ProgramStatus::CompilingRust, + ProgramStatus::Success, + ], + ), + ( + "done-sql-error", + vec![ProgramStatus::CompilingSql, ProgramStatus::SqlError], + ), + ( + "done-rust-error", + vec![ + ProgramStatus::CompilingSql, + ProgramStatus::SqlCompiled, + ProgramStatus::CompilingRust, + ProgramStatus::RustError, + ], + ), + ("done-system-error", vec![ProgramStatus::SystemError]), + ] { + let pipeline = new_test_pipeline(&handle.db, tenant_id, name).await; + assert_eq!( + handle + .db + .count_pipelines_needing_compilation() + .await + .unwrap(), + 5 + ); + advance_program_status(&handle.db, tenant_id, pipeline.id, &statuses, &program_info).await; + assert_eq!( + handle + .db + .count_pipelines_needing_compilation() + .await + .unwrap(), + 4 + ); + } + + // A non-stopped deployment is not counted. Non-stopped resources with an + // unfinished program is unreachable through the Storage API (provisioning + // requires a compiled program), so force the state to pin the predicate. + for (resources_status, expected_count) in [("provisioning", 3), ("stopped", 4)] { + handle + .db + .pool + .get() + .await + .unwrap() + .execute( + "UPDATE pipeline SET deployment_resources_status = $1 WHERE id = $2", + &[&resources_status, &p_pending.id.0], + ) + .await + .unwrap(); + assert_eq!( + handle + .db + .count_pipelines_needing_compilation() + .await + .unwrap(), + expected_count + ); + } +} + /// Tests the following sequence of events: /// - Pipeline is compiled /// - User calls /start @@ -4380,6 +4619,7 @@ enum StorageAction { GetNextSqlCompilation(#[proptest(strategy = "limited_platform_version()")] String), ClearOngoingRustCompilation(#[proptest(strategy = "limited_platform_version()")] String), GetNextRustCompilation(#[proptest(strategy = "limited_platform_version()")] String), + CountPipelinesNeedingCompilation, ListPipelineProgramsAcrossAllTenants, ListClusterMonitorEvents, GetClusterMonitorEventShort(ClusterMonitorEventId), @@ -5174,6 +5414,11 @@ fn db_impl_behaves_like_model() { let impl_response = handle.db.get_next_rust_compilation(&platform_version, 0, 1).await; check_response_optional_pipeline_with_tenant_id(i, model_response, impl_response); } + StorageAction::CountPipelinesNeedingCompilation => { + let model_response = model.count_pipelines_needing_compilation().await; + let impl_response = handle.db.count_pipelines_needing_compilation().await; + check_responses(i, model_response, impl_response); + } StorageAction::ListPipelineProgramsAcrossAllTenants => { let model_response = model.list_pipeline_programs_across_all_tenants().await; let impl_response = handle.db.list_pipeline_programs_across_all_tenants().await; @@ -7641,6 +7886,25 @@ impl Storage for Mutex { Ok(Some((chosen.0, chosen.1))) } + async fn count_pipelines_needing_compilation(&self) -> Result { + Ok(self + .lock() + .await + .pipelines + .values() + .filter(|p| { + p.deployment_resources_status == ResourcesStatus::Stopped + && matches!( + p.program_status, + ProgramStatus::Pending + | ProgramStatus::CompilingSql + | ProgramStatus::SqlCompiled + | ProgramStatus::CompilingRust + ) + }) + .count() as u64) + } + async fn list_pipeline_programs_across_all_tenants( &self, ) -> Result< From 49c0f47248c168e58cb8f78e16038c20731d1206 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sat, 1 Aug 2026 20:14:04 -0700 Subject: [PATCH 02/11] compiler: artifact server mode, atomic uploads, retryable upload failures artifact_server_main runs the compiler HTTP surface plus a janitor without the SQL and Rust compile tasks. It backs deployments where compiler workers scale to zero and a small always-on pod stores and serves binaries (enterprise compiler autoscaling). The janitor garbage-collects pipeline binaries of deleted or recompiled pipelines, ephemeral validation directories orphaned by crashed validations, and stale SQL compiler jars. Binary and program info uploads stream to a temp file and rename onto the final path after checksum verification and fsync, so an interrupted upload can no longer leave a truncated file under a valid name. Cleanup removes orphaned temp files after an hour. Upload failures no longer park programs in SystemError: transport errors, 5xx, 408 and 429 leave the row in CompilingRust so the regular reset retries the compile and upload once the endpoint recovers; other 4xx responses are permanent rejections and still surface as SystemError. Signed-off-by: Gerd Zellweger --- crates/pipeline-manager/src/compiler/main.rs | 374 ++++++++++++++++-- .../src/compiler/rust_compiler.rs | 306 ++++++++++---- .../src/compiler/sql_compiler.rs | 35 +- 3 files changed, 597 insertions(+), 118 deletions(-) diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index d3a666c59f3..7ebcd901e54 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -2,11 +2,12 @@ use crate::api::error::ApiError; use crate::common_error::CommonError; use crate::compiler::error::CompilerError; use crate::compiler::rust_compiler::{ - perform_rust_compilation, rust_compiler_task, RustCompilationError, RustCompilationResult, + cleanup_pipeline_binaries, perform_rust_compilation, rust_compiler_task, RustCompilationError, + RustCompilationResult, CLEANUP_INTERVAL, }; use crate::compiler::sql_compiler::{ - ephemeral_compilation_dir, perform_sql_compilation, sql_compiler_task, validate_program, - ProgramValidationRequest, SqlCompilationError, SqlCompilationOutput, + ephemeral_compilation_dir, jar_cache_dir, perform_sql_compilation, sql_compiler_task, + validate_program, ProgramValidationRequest, SqlCompilationError, SqlCompilationOutput, }; use crate::compiler::util::{ pipeline_binary_filename, program_info_filename, validate_is_sha256_checksum, @@ -20,14 +21,18 @@ use crate::db::types::tenant::TenantId; use crate::db::types::version::Version; use crate::error::ManagerError; use actix_files::NamedFile; +use actix_web::error::PayloadError; use actix_web::{get, post, web, HttpRequest, HttpResponse, HttpServer, Responder}; -use futures_util::StreamExt; +use futures_util::{Stream, StreamExt}; use std::net::TcpListener; use std::path::Path; use std::str::FromStr; use std::sync::Arc; +use std::time::Duration; +use tokio::task::JoinHandle; +use tokio::time::sleep; use tokio::{fs, io::AsyncWriteExt, spawn, sync::Mutex}; -use tracing::{error, info}; +use tracing::{error, info, warn}; use uuid::Uuid; /// Decodes the URL encoded parameter value as a string. @@ -496,15 +501,82 @@ async fn validate_program_endpoint( Ok(HttpResponse::Ok().json(response)) } +/// Streams the payload to a temp file next to `target_file_path`, verifies the +/// sha256 checksum, and only then renames the temp file onto the final path. +/// The final path therefore never holds a partially written or corrupt file; +/// on any error the temp file is removed. async fn save_file( target_file_path: &Path, - mut payload: web::Payload, + payload: impl Stream> + Unpin, expected_integrity_checksum: &str, ) -> Result { - // Stream the binary directly to disk with integrity checksum validation - let mut file = fs::File::create(&target_file_path).await.map_err(|e| { + let target_file_name = target_file_path + .file_name() + .map(|name| name.to_string_lossy().to_string()) + .unwrap_or_default(); + let temp_file_path = + target_file_path.with_file_name(format!("{target_file_name}.tmp-{}", Uuid::now_v7())); + + let write_result = + stream_to_file_and_verify(&temp_file_path, payload, expected_integrity_checksum).await; + + match write_result { + Ok(total_size) => match fs::rename(&temp_file_path, target_file_path).await { + Ok(()) => { + // Persist the rename: without a directory fsync a crash can + // lose the entry while the database already says Success. + fsync_parent_dir(target_file_path).await?; + Ok(total_size) + } + Err(e) => { + remove_temp_upload_file(&temp_file_path).await; + Err(ManagerError::from(CommonError::io_error( + format!( + "renaming '{}' to '{}'", + temp_file_path.display(), + target_file_path.display() + ), + e, + ))) + } + }, + Err(e) => { + remove_temp_upload_file(&temp_file_path).await; + Err(e) + } + } +} + +/// Fsyncs the directory containing `path` so that a rename into it is durable. +async fn fsync_parent_dir(path: &Path) -> Result<(), ManagerError> { + let Some(parent_dir) = path.parent() else { + return Ok(()); + }; + let parent_dir_handle = fs::File::open(parent_dir).await.map_err(|e| { ManagerError::from(CommonError::io_error( - format!("creating file '{}'", target_file_path.display()), + format!("opening directory '{}'", parent_dir.display()), + e, + )) + })?; + parent_dir_handle.sync_all().await.map_err(|e| { + ManagerError::from(CommonError::io_error( + format!("syncing directory '{}'", parent_dir.display()), + e, + )) + }) +} + +/// Streams the payload to `file_path` and validates the sha256 checksum after +/// flushing. Returns the total size in bytes. The caller removes the file on +/// any error. +async fn stream_to_file_and_verify( + file_path: &Path, + mut payload: impl Stream> + Unpin, + expected_integrity_checksum: &str, +) -> Result { + let mut file = fs::File::create(&file_path).await.map_err(|e| { + ManagerError::from(CommonError::io_error( + format!("creating file '{}'", file_path.display()), e, )) })?; @@ -527,16 +599,23 @@ async fn save_file( // Write chunk to file file.write_all(&chunk).await.map_err(|e| { ManagerError::from(CommonError::io_error( - format!("writing to file '{}'", target_file_path.display()), + format!("writing to file '{}'", file_path.display()), e, )) })?; } - // Flush and close file + // Flush and persist to disk before the rename makes the file visible + // under its final name. file.flush().await.map_err(|e| { ManagerError::from(CommonError::io_error( - format!("flushing file '{}'", target_file_path.display()), + format!("flushing file '{}'", file_path.display()), + e, + )) + })?; + file.sync_all().await.map_err(|e| { + ManagerError::from(CommonError::io_error( + format!("syncing file '{}'", file_path.display()), e, )) })?; @@ -545,8 +624,6 @@ async fn save_file( // Validate integrity checksum let actual_integrity_checksum = hex::encode(hasher.finish()); if actual_integrity_checksum != expected_integrity_checksum { - // Remove the invalid file - let _ = fs::remove_file(&target_file_path).await; return Err(ManagerError::from(ApiError::InvalidChecksumParam { value: format!( "Expected integrity checksum '{}', but calculated '{}'", @@ -559,6 +636,19 @@ async fn save_file( Ok(total_size) } +/// Removes a temp upload file. Failure only leaves an orphan file behind, so +/// it is logged rather than propagated. +async fn remove_temp_upload_file(temp_file_path: &Path) { + if let Err(e) = fs::remove_file(temp_file_path).await { + if e.kind() != std::io::ErrorKind::NotFound { + warn!( + "Unable to remove temp upload file '{}': {e}", + temp_file_path.display() + ); + } + } +} + /// Health check which returns success if it is able to reach the database. #[get("/healthz")] async fn healthz(probe: web::Data>>) -> Result { @@ -742,6 +832,54 @@ pub async fn compiler_main( )); // Spawn HTTP server thread + let http_server = spawn_compiler_http_server(&common_config, &config, &db).await; + + // All threads should run indefinitely + let error = tokio::select! { + _ = sql_task => "Compiler SQL task ended prematurely", + _ = rust_task => "Compiler Rust task ended prematurely", + _ = http_server => "Compiler HTTP(S) server task ended prematurely", + }; + error!("{error}"); + error!("Returning compiler thread"); + Err(ManagerError::from(CompilerError::TaskFailed { + error: error.to_string(), + })) +} + +/// Runs the artifact-store variant of the compiler server: the full HTTP +/// surface plus a janitor, but no SQL or Rust compilation tasks. Serves +/// deployments where compiler workers are ephemeral and this process is the +/// durable binary store (see enterprise compiler autoscaling). +pub async fn artifact_server_main( + common_config: CommonConfig, + config: CompilerConfig, + db: Arc>, +) -> Result<(), ManagerError> { + create_working_directory_if_not_exists(&config).await?; + + // Spawn janitor and HTTP server threads + let janitor_task = spawn(artifact_server_janitor_task(config.clone(), db.clone())); + let http_server = spawn_compiler_http_server(&common_config, &config, &db).await; + + // Both threads should run indefinitely + let error = tokio::select! { + _ = janitor_task => "Artifact server janitor task ended prematurely", + _ = http_server => "Artifact server HTTP(S) server task ended prematurely", + }; + error!("{error}"); + Err(ManagerError::from(CompilerError::TaskFailed { + error: error.to_string(), + })) +} + +/// Spawns the compiler HTTP(S) server serving artifacts, uploads, program +/// validation, and health checks. Panics if the listener cannot be bound. +async fn spawn_compiler_http_server( + common_config: &CommonConfig, + config: &CompilerConfig, + db: &Arc>, +) -> JoinHandle> { let config = web::Data::new(config.clone()); let common_config_data = web::Data::new(common_config.clone()); let probe = web::Data::new(DbProbe::new(db.clone()).await); @@ -793,25 +931,116 @@ pub async fn compiler_main( common_config.compiler_port, common_config.http_workers, ); + http_server +} - // All threads should run indefinitely - let error = tokio::select! { - _ = sql_task => "Compiler SQL task ended prematurely", - _ = rust_task => "Compiler Rust task ended prematurely", - _ = http_server => "Compiler HTTP(S) server task ended prematurely", - }; - error!("{error}"); - error!("Returning compiler thread"); - Err(ManagerError::from(CompilerError::TaskFailed { - error: error.to_string(), - })) +/// Age above which an ephemeral validation directory is an orphan of a crashed +/// validation; live validations finish within seconds. +const ORPHANED_EPHEMERAL_DIR_MAX_AGE: Duration = Duration::from_secs(3600); + +/// Age above which the janitor removes a cached SQL compiler jar. +const STALE_JAR_MAX_AGE: Duration = Duration::from_secs(30 * 24 * 3600); + +/// Janitor of the artifact server: garbage-collects pipeline binaries of +/// deleted or recompiled pipelines, ephemeral validation directories orphaned +/// by crashed validations, and stale SQL compiler jars. Errors within a pass +/// are logged and the loop continues. +async fn artifact_server_janitor_task(config: CompilerConfig, db: Arc>) { + loop { + if let Err(e) = cleanup_pipeline_binaries(&config, db.clone()).await { + error!("Artifact server janitor: pipeline binaries cleanup failed: {e}"); + } + if let Err(e) = remove_stale_entries( + &ephemeral_compilation_dir(&config), + ORPHANED_EPHEMERAL_DIR_MAX_AGE, + StaleEntryKind::Directories, + ) + .await + { + error!("Artifact server janitor: ephemeral validation directory cleanup failed: {e}"); + } + if let Err(e) = remove_stale_entries( + &jar_cache_dir(&config), + STALE_JAR_MAX_AGE, + StaleEntryKind::Files, + ) + .await + { + error!("Artifact server janitor: SQL compiler jar cache cleanup failed: {e}"); + } + sleep(CLEANUP_INTERVAL).await; + } +} + +/// Which kind of directory entry [`remove_stale_entries`] removes. +#[derive(Clone, Copy)] +enum StaleEntryKind { + Directories, + Files, +} + +/// Removes the entries of `dir` of the given kind whose modification time is +/// older than `max_age`. A failure to remove one entry is logged and does not +/// stop the sweep. +async fn remove_stale_entries( + dir: &Path, + max_age: Duration, + kind: StaleEntryKind, +) -> Result<(), std::io::Error> { + if !dir.is_dir() { + return Ok(()); + } + let mut entries = fs::read_dir(dir).await?; + while let Some(entry) = entries.next_entry().await? { + // Entries can vanish mid-sweep when racing a finishing validation; + // one unstat-able entry must not abort the whole sweep. + let metadata = match entry.metadata().await { + Ok(metadata) => metadata, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, + Err(e) => { + warn!( + "Unable to stat entry '{}' during stale entry sweep: {e}", + entry.path().display() + ); + continue; + } + }; + let matches_kind = match kind { + StaleEntryKind::Directories => metadata.is_dir(), + StaleEntryKind::Files => metadata.is_file(), + }; + if !matches_kind { + continue; + } + // A modification time that is unavailable or in the future never + // counts as stale. + let Ok(modified_time) = metadata.modified() else { + continue; + }; + if !modified_time.elapsed().is_ok_and(|age| age >= max_age) { + continue; + } + let removal_result = match kind { + StaleEntryKind::Directories => fs::remove_dir_all(entry.path()).await, + StaleEntryKind::Files => fs::remove_file(entry.path()).await, + }; + match removal_result { + Ok(()) => info!("Removed stale entry '{}'", entry.path().display()), + Err(e) => warn!( + "Unable to remove stale entry '{}': {e}", + entry.path().display() + ), + } + } + Ok(()) } #[cfg(test)] mod test { use crate::api::error::ApiError; use crate::compiler::main::{ - create_working_directory_if_not_exists, decode_url_encoded_parameter, upload_binary, + create_working_directory_if_not_exists, decode_url_encoded_parameter, remove_stale_entries, + save_file, upload_binary, StaleEntryKind, }; use crate::compiler::util::pipeline_binary_filename; use crate::config::CompilerConfig; @@ -819,8 +1048,10 @@ mod test { use crate::db::types::program::CompilationProfile; use crate::db::types::version::Version; use crate::error::ManagerError; + use actix_web::error::PayloadError; use actix_web::{test as actix_test, web, App}; use openssl::sha::sha256; + use std::time::Duration; use tokio::fs; use uuid::Uuid; @@ -961,6 +1192,14 @@ mod test { "Checksum should match for test case: {}", test_case.name ); + + // No temp file may be left behind + let dir_names = + list_file_names(expected_path.parent().expect("path must have a parent")).await; + assert!( + dir_names.iter().all(|name| !name.contains(".tmp-")), + "No temp file may remain, found: {dir_names:?}" + ); } } @@ -1004,6 +1243,79 @@ mod test { !expected_path.exists(), "Binary file should not exist after checksum failure" ); + + // Neither the final file nor a temp file may remain + let dir_names = + list_file_names(expected_path.parent().expect("path must have a parent")).await; + assert!( + dir_names.is_empty(), + "No file may remain after checksum failure, found: {dir_names:?}" + ); + } + + /// An upload whose payload errors mid-stream leaves no file under the + /// final name and no temp file behind. + #[tokio::test] + async fn test_interrupted_upload_leaves_no_file() { + let tempdir = tempfile::tempdir().unwrap(); + let pipeline_binaries_dir = tempdir.path().join("pipeline-binaries"); + fs::create_dir_all(&pipeline_binaries_dir).await.unwrap(); + let target_file_path = pipeline_binaries_dir.join("pipeline_example_binary"); + + let data = b"partial data"; + let interrupted_payload = futures_util::stream::iter(vec![ + Ok(web::Bytes::from_static(data)), + Err(PayloadError::Incomplete(None)), + ]); + let result = save_file( + &target_file_path, + interrupted_payload, + &hex::encode(sha256(data)), + ) + .await; + assert!(result.is_err(), "Interrupted upload should fail"); + + let dir_names = list_file_names(&pipeline_binaries_dir).await; + assert!( + dir_names.is_empty(), + "No file may remain after an interrupted upload, found: {dir_names:?}" + ); + } + + /// Stale-entry removal deletes only entries older than the maximum age and + /// only entries of the requested kind. + #[tokio::test] + async fn test_stale_entry_removal() { + let tempdir = tempfile::tempdir().unwrap(); + let dir = tempdir.path(); + let subdir = dir.join("subdir"); + fs::create_dir(&subdir).await.unwrap(); + let file = dir.join("file.jar"); + fs::write(&file, b"jar").await.unwrap(); + + // Entries younger than the maximum age are kept + let long_max_age = Duration::from_secs(3600); + remove_stale_entries(dir, long_max_age, StaleEntryKind::Directories) + .await + .unwrap(); + remove_stale_entries(dir, long_max_age, StaleEntryKind::Files) + .await + .unwrap(); + assert!(subdir.is_dir()); + assert!(file.is_file()); + + // Older entries are removed, but only those of the requested kind + tokio::time::sleep(Duration::from_millis(100)).await; + let short_max_age = Duration::from_millis(50); + remove_stale_entries(dir, short_max_age, StaleEntryKind::Directories) + .await + .unwrap(); + assert!(!subdir.exists()); + assert!(file.is_file()); + remove_stale_entries(dir, short_max_age, StaleEntryKind::Files) + .await + .unwrap(); + assert!(!file.exists()); } #[tokio::test] @@ -1101,6 +1413,16 @@ mod test { (0..size).map(|i| (i % 256) as u8).collect() } + /// Lists the file names in a directory + async fn list_file_names(dir: &std::path::Path) -> Vec { + let mut file_names = vec![]; + let mut entries = fs::read_dir(dir).await.unwrap(); + while let Some(entry) = entries.next_entry().await.unwrap() { + file_names.push(entry.file_name().to_string_lossy().to_string()); + } + file_names + } + /// Builds upload URL from parameters fn build_upload_url( pipeline_id: &PipelineId, diff --git a/crates/pipeline-manager/src/compiler/rust_compiler.rs b/crates/pipeline-manager/src/compiler/rust_compiler.rs index ea6de2232d6..553127f4167 100644 --- a/crates/pipeline-manager/src/compiler/rust_compiler.rs +++ b/crates/pipeline-manager/src/compiler/rust_compiler.rs @@ -56,7 +56,11 @@ const POLL_ERROR_INTERVAL: Duration = Duration::from_secs(30); const COMPILATION_CHECK_INTERVAL: Duration = Duration::from_millis(250); /// The frequency at which Rust cleanup is performed. -const CLEANUP_INTERVAL: Duration = Duration::from_secs(120); +pub(crate) const CLEANUP_INTERVAL: Duration = Duration::from_secs(120); + +/// Age above which a `.tmp-` upload file is an orphan of a crashed upload +/// rather than an upload in flight, and is removed by the binaries cleanup. +const STALE_TEMP_UPLOAD_MAX_AGE: Duration = Duration::from_secs(3600); /// Minimum time between when the Rust cleanup has detected a pipeline to be deleted until /// its compilation artifacts are actually cleaned up. It is a minimum, as cleanup both @@ -338,19 +342,13 @@ async fn attempt_end_to_end_rust_compilation( ); } RustCompilationError::FileUploadError(upload_error) => { - db.lock() - .await - .transit_program_status_to_system_error( - tenant_id, - pipeline.id, - pipeline.program_version, - &upload_error, - ) - .await?; + // No status is written: the row stays `CompilingRust` and the next + // iteration's reset returns it to `SqlCompiled`, so the compile and + // upload are retried once the upload endpoint is reachable again. error!( pipeline_id = %pipeline.id, pipeline = %pipeline.name, - "Rust compilation failed due to binary upload error (program version: {}): {upload_error}", + "Rust compilation failed due to binary upload error; it will be retried (program version: {}): {upload_error}", pipeline.program_version ); } @@ -607,6 +605,11 @@ async fn upload_binary_to_endpoint_with_retries( return Ok(result); } Err(e) => { + // A permanent rejection cannot be fixed by retrying; surface + // it immediately so a terminal status is written. + if matches!(e, RustCompilationError::SystemError(_)) { + return Err(e); + } if attempts > max_retries { error!( pipeline_id = %metadata.pipeline_id, @@ -685,6 +688,11 @@ async fn upload_program_info_to_endpoint_with_retries( return Ok(result); } Err(e) => { + // A permanent rejection cannot be fixed by retrying; surface + // it immediately so a terminal status is written. + if matches!(e, RustCompilationError::SystemError(_)) { + return Err(e); + } if attempts > max_retries { error!( pipeline_id = %metadata.pipeline_id, @@ -715,6 +723,14 @@ async fn upload_program_info_to_endpoint_with_retries( } } +/// Returns true when the upload response status indicates a permanent +/// rejection (4xx other than 408 and 429) that no retry or recompile can fix. +fn is_permanent_upload_rejection(status: reqwest::StatusCode) -> bool { + status.is_client_error() + && status != reqwest::StatusCode::REQUEST_TIMEOUT + && status != reqwest::StatusCode::TOO_MANY_REQUESTS +} + /// Uploads the compiled binary to an HTTP endpoint (single attempt) using streaming. async fn upload_binary_to_endpoint( common_config: &CommonConfig, @@ -794,10 +810,14 @@ async fn upload_binary_to_endpoint( .text() .await .unwrap_or_else(|_| "Unknown error".to_string()); - return Err(RustCompilationError::FileUploadError(format!( - "Binary upload failed with status {}: {}", - status, error_text - ))); + let message = format!("Binary upload failed with status {status}: {error_text}"); + // A 4xx other than 408/429 is a permanent rejection: retrying or + // recompiling cannot succeed, so surface a terminal SystemError. + return Err(if is_permanent_upload_rejection(status) { + RustCompilationError::SystemError(message) + } else { + RustCompilationError::FileUploadError(message) + }); } // Get response body for any location information @@ -862,10 +882,14 @@ async fn upload_program_info_to_endpoint( .text() .await .unwrap_or_else(|_| "Unknown error".to_string()); - return Err(RustCompilationError::FileUploadError(format!( - "Program info upload failed with status {}: {}", - status, error_text - ))); + let message = format!("Program info upload failed with status {status}: {error_text}"); + // A 4xx other than 408/429 is a permanent rejection: retrying or + // recompiling cannot succeed, so surface a terminal SystemError. + return Err(if is_permanent_upload_rejection(status) { + RustCompilationError::SystemError(message) + } else { + RustCompilationError::FileUploadError(message) + }); } // Get response body for any location information @@ -1753,7 +1777,7 @@ async fn call_compiler( } /// Rust compilation cleanup possible error outcomes. -enum RustCompilationCleanupError { +pub(crate) enum RustCompilationCleanupError { /// Database error occurred (e.g., lost connectivity). Database(DBError), /// Utility function problem occurred (e.g., I/O error) @@ -1776,6 +1800,18 @@ impl From for RustCompilationCleanupError { } } +impl std::fmt::Display for RustCompilationCleanupError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RustCompilationCleanupError::Database(e) => write!(f, "database error occurred: {e}"), + RustCompilationCleanupError::Utility(e) => write!(f, "utility error occurred: {e}"), + RustCompilationCleanupError::TargetCleared => { + write!(f, "target directory was cleared") + } + } + } +} + /// Makes the cleanup decision based on the provided name of the file or directory. fn decide_cleanup( name: &str, @@ -1817,27 +1853,74 @@ fn decide_cleanup( } } -/// Cleans up the Rust compilation working directory by removing binaries and compilation artifacts. -/// If the working directory `target` directory is about to become full (due to old dependencies), -/// it will be fully cleared. Otherwise, it will only clear compilation artifacts of pipeline -/// programs that no longer exist. -async fn cleanup_rust_compilation( +/// Decides whether a file in the pipeline-binaries directory is kept, +/// removed, or ignored by the binaries cleanup. +fn decide_pipeline_binary_cleanup( + filename: &str, + metadata: Option, + valid_pipeline_binary_filenames: &[String], + valid_program_info_filenames: &[String], +) -> CleanupDecision { + // Temp upload files share the binary name prefixes, so they must + // be decided before the prefix matches below would keep them. + if filename.contains(".tmp-") { + let is_stale_orphan = metadata + .and_then(|metadata| metadata.modified().ok()) + .and_then(|modified_time| modified_time.elapsed().ok()) + .is_some_and(|age| age >= STALE_TEMP_UPLOAD_MAX_AGE); + return if is_stale_orphan { + CleanupDecision::Remove + } else { + // Keep rather than Ignore: an upload may still be writing this + // file, and Ignore would warn on every cleanup pass. + CleanupDecision::Keep { + motivation: filename.to_string(), + } + }; + } + if filename.starts_with("pipeline_") { + if valid_pipeline_binary_filenames + .iter() + .any(|f| filename.starts_with(f)) + { + CleanupDecision::Keep { + motivation: filename.to_string(), + } + } else { + CleanupDecision::Remove + } + } else if filename.starts_with("program_info_") { + if valid_program_info_filenames + .iter() + .any(|f| filename.starts_with(f)) + { + CleanupDecision::Keep { + motivation: filename.to_string(), + } + } else { + CleanupDecision::Remove + } + } else { + CleanupDecision::Ignore + } +} + +/// Removes pipeline binaries and program info files that no longer correspond +/// to an existing pipeline program; only the latest version of each program is +/// retained. A row still `CompilingRust` has no checksums yet, so its files +/// are matched on a checksum-less prefix and retained. +pub(crate) async fn cleanup_pipeline_binaries( config: &CompilerConfig, db: Arc>, ) -> Result<(), RustCompilationCleanupError> { - trace!("Performing Rust cleanup..."); - - // Rust compilation directory - let rust_compilation_dir = config.working_dir().join("rust-compilation"); - if !rust_compilation_dir.exists() { + let pipeline_binaries_dir = config + .working_dir() + .join("rust-compilation") + .join("pipeline-binaries"); + if !pipeline_binaries_dir.is_dir() { return Ok(()); } - /////////////////////////////// - // PHASE 1: PIPELINE BINARIES - // Only the latest version binaries of successfully compiled pipeline - // programs are retained. Older version binaries are deleted. - // Retrieve existing pipeline programs // (pipeline_id, program_version, program_binary_source_checksum, program_binary_integrity_checksum, program_info_integrity_checksum) let existing_pipeline_programs = db @@ -1848,7 +1931,6 @@ async fn cleanup_rust_compilation( // Clean up pipeline binaries // These are not subject to the retention period. - let pipeline_binaries_dir = rust_compilation_dir.join("pipeline-binaries"); let valid_pipeline_binary_filenames: Vec = existing_pipeline_programs .iter() .map( @@ -1905,45 +1987,45 @@ async fn cleanup_rust_compilation( ) .collect(); - if pipeline_binaries_dir.is_dir() { - cleanup_specific_files( - "Rust compilation pipeline binaries", - &pipeline_binaries_dir, - Arc::new( - move |filename: &str, _metadata: Option| { - if filename.starts_with("pipeline_") { - if valid_pipeline_binary_filenames - .iter() - .any(|f| filename.starts_with(f)) - { - CleanupDecision::Keep { - motivation: filename.to_string(), - } - } else { - CleanupDecision::Remove - } - } else if filename.starts_with("program_info_") { - if valid_program_info_filenames - .iter() - .any(|f| filename.starts_with(f)) - { - CleanupDecision::Keep { - motivation: filename.to_string(), - } - } else { - CleanupDecision::Remove - } - } else { - CleanupDecision::Ignore - } - }, - ), - true, - false, - ) - .await?; + cleanup_specific_files( + "Rust compilation pipeline binaries", + &pipeline_binaries_dir, + Arc::new(move |filename: &str, metadata: Option| { + decide_pipeline_binary_cleanup( + filename, + metadata, + &valid_pipeline_binary_filenames, + &valid_program_info_filenames, + ) + }), + true, + true, + ) + .await?; + + Ok(()) +} + +/// Cleans up the Rust compilation working directory by removing binaries and compilation artifacts. +/// If the working directory `target` directory is about to become full (due to old dependencies), +/// it will be fully cleared. Otherwise, it will only clear compilation artifacts of pipeline +/// programs that no longer exist. +async fn cleanup_rust_compilation( + config: &CompilerConfig, + db: Arc>, +) -> Result<(), RustCompilationCleanupError> { + trace!("Performing Rust cleanup..."); + + // Rust compilation directory + let rust_compilation_dir = config.working_dir().join("rust-compilation"); + if !rust_compilation_dir.exists() { + return Ok(()); } + /////////////////////////////// + // PHASE 1: PIPELINE BINARIES + cleanup_pipeline_binaries(config, db.clone()).await?; + /////////////////////////////////// // PHASE 2: COMPILATION ARTIFACTS // Remove the artifacts used during compilation itself, notably @@ -2298,7 +2380,10 @@ async fn cleanup_rust_compilation( mod test { use crate::auth::TenantRecord; use crate::compiler::rust_compiler::prepare_workspace; - use crate::compiler::rust_compiler::{calculate_source_checksum, decide_cleanup}; + use crate::compiler::rust_compiler::{ + calculate_source_checksum, decide_cleanup, decide_pipeline_binary_cleanup, + STALE_TEMP_UPLOAD_MAX_AGE, + }; use crate::compiler::test::{list_content_as_sorted_names, CompilerTest}; use crate::compiler::util::{ crate_name_pipeline_globals, crate_name_pipeline_main, read_file_content, CleanupDecision, @@ -2544,4 +2629,77 @@ mod test { ); } } + + /// Checks the pipeline binaries cleanup decision: stale `.tmp-` uploads are + /// removed, fresh or unstat-able ones are kept, and prefix matching decides the rest. + #[test] + fn pipeline_binary_cleanup_decision() { + let valid_binaries = vec!["pipeline_a_v1_".to_string()]; + let valid_program_infos = vec!["program_info_a_v1_".to_string()]; + let decide = |filename: &str, metadata: Option| { + decide_pipeline_binary_cleanup( + filename, + metadata, + &valid_binaries, + &valid_program_infos, + ) + }; + + // Metadata with an mtime older than the staleness threshold + let temp_dir = tempfile::tempdir().unwrap(); + let stale_file_path = temp_dir.path().join("stale"); + let stale_file = std::fs::File::create(&stale_file_path).unwrap(); + let stale_mtime = std::time::SystemTime::now() + - (STALE_TEMP_UPLOAD_MAX_AGE + std::time::Duration::from_secs(60)); + stale_file + .set_times(std::fs::FileTimes::new().set_modified(stale_mtime)) + .unwrap(); + let stale_metadata = std::fs::metadata(&stale_file_path).unwrap(); + + // Metadata with a current mtime + let fresh_file_path = temp_dir.path().join("fresh"); + std::fs::File::create(&fresh_file_path).unwrap(); + let fresh_metadata = std::fs::metadata(&fresh_file_path).unwrap(); + + // Stale temp upload is removed + assert_eq!( + decide("pipeline_a_v1_x.tmp-123", Some(stale_metadata)), + CleanupDecision::Remove + ); + // Fresh temp upload is kept (upload may still be in flight) + assert_eq!( + decide("pipeline_a_v1_x.tmp-123", Some(fresh_metadata)), + CleanupDecision::Keep { + motivation: "pipeline_a_v1_x.tmp-123".to_string() + } + ); + // Missing metadata never removes a temp upload + assert_eq!( + decide("pipeline_a_v1_x.tmp-123", None), + CleanupDecision::Keep { + motivation: "pipeline_a_v1_x.tmp-123".to_string() + } + ); + // Binary with a valid prefix is kept, unknown is removed + assert_eq!( + decide("pipeline_a_v1_abc", None), + CleanupDecision::Keep { + motivation: "pipeline_a_v1_abc".to_string() + } + ); + assert_eq!(decide("pipeline_b_v1_abc", None), CleanupDecision::Remove); + // Program info with a valid prefix is kept, unknown is removed + assert_eq!( + decide("program_info_a_v1_abc", None), + CleanupDecision::Keep { + motivation: "program_info_a_v1_abc".to_string() + } + ); + assert_eq!( + decide("program_info_b_v1_abc", None), + CleanupDecision::Remove + ); + // Unrelated file is ignored + assert_eq!(decide("unrelated.txt", None), CleanupDecision::Ignore); + } } diff --git a/crates/pipeline-manager/src/compiler/sql_compiler.rs b/crates/pipeline-manager/src/compiler/sql_compiler.rs index e07dc02e54b..26a6c720de3 100644 --- a/crates/pipeline-manager/src/compiler/sql_compiler.rs +++ b/crates/pipeline-manager/src/compiler/sql_compiler.rs @@ -334,6 +334,15 @@ impl From for SqlCompilationError { } } +/// Directory in which downloaded SQL compiler jars for non-platform runtime +/// versions are cached. +pub(crate) fn jar_cache_dir(config: &CompilerConfig) -> PathBuf { + config + .working_dir() + .join("sql-compilation") + .join("jar-cache") +} + /// Determines the path to the SQL compiler executable based on the runtime selector. fn determine_sql_compiler_path( config: &CompilerConfig, @@ -341,16 +350,12 @@ fn determine_sql_compiler_path( ) -> PathBuf { match runtime_selector { RuntimeSelector::Platform(_) => PathBuf::from(&config.sql_compiler_path), - RuntimeSelector::Sha(sha) => config - .working_dir() - .join("sql-compilation") - .join("jar-cache") - .join(format!("sql2dbsp-jar-with-dependencies-{sha}.jar")), - RuntimeSelector::Version(version) => config - .working_dir() - .join("sql-compilation") - .join("jar-cache") - .join(format!("sql2dbsp-jar-with-dependencies-{version}.jar")), + RuntimeSelector::Sha(sha) => { + jar_cache_dir(config).join(format!("sql2dbsp-jar-with-dependencies-{sha}.jar")) + } + RuntimeSelector::Version(version) => { + jar_cache_dir(config).join(format!("sql2dbsp-jar-with-dependencies-{version}.jar")) + } } } @@ -363,10 +368,7 @@ async fn fetch_sql_compiler( "This code-path is only enabled in unstable mode" ); - let jar_cache_dir = config - .working_dir() - .join("sql-compilation") - .join("jar-cache"); + let jar_cache_dir = jar_cache_dir(config); fs::create_dir_all(&jar_cache_dir).await.map_err(|e| SqlCompilationError::SystemError(format!( "Unable initialize JAR cache directory '{}': {}. If possible, fall-back to platform version by removing `runtime_version` in the program config.", jar_cache_dir.display(), @@ -1032,10 +1034,7 @@ pub(crate) async fn cleanup_sql_compilation( } // (3) Clean up JAR cache to make sure it does not grow unboundedly - let jar_cache_dir = config - .working_dir() - .join("sql-compilation") - .join("jar-cache"); + let jar_cache_dir = jar_cache_dir(config); if jar_cache_dir.is_dir() { cleanup_specific_files( "SQL JAR cache", From d0dad3ef477aa9af6e99261452b73904bf46da04 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sat, 1 Aug 2026 20:14:04 -0700 Subject: [PATCH 03/11] docs: compiler autoscaling section for enterprise parallel compilation Covers the 0-or-N model, the always-on artifact server, configuration defaults, cold start latency expectations, the enable and disable procedures for existing installations including binary store seeding, and troubleshooting. Signed-off-by: Gerd Zellweger --- .../enterprise/parallel-compilation.md | 115 +++++++++++++++++- 1 file changed, 112 insertions(+), 3 deletions(-) diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index c1b09e6cdc6..b1303fd7f3d 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -17,7 +17,7 @@ Feldera deploys the compiler server as a Kubernetes StatefulSet with **N** repli To further accelerate builds, Feldera optionally supports [sccache](https://github.com/mozilla/sccache) with an S3-compatible backend. This allows workers to share compiled operator artifacts instead of rebuilding identical code. :::info -Autoscaling based on workload is not yet supported. You must set the number of compiler server replicas at install time or scale them manually later. +Workload-based autoscaling is available as an experimental feature; see [Autoscaling (experimental)](#autoscaling-experimental). Without it, you set the number of compiler server replicas at install time or scale them manually later. ::: --- @@ -131,6 +131,104 @@ parallelCompilation: ``` +--- + +## Autoscaling (experimental) + +Compiler autoscaling scales the compiler server StatefulSet between 0 and N replicas so that idle deployments stop paying for compiler nodes. The kubernetes-runner drives the scaling: + +- Every `pollIntervalSeconds` the runner counts pipelines that need compilation. A pipeline counts when its deployment resources are stopped and its program status is `Pending`, `CompilingSql`, `SqlCompiled`, or `CompilingRust`. +- When the count is greater than zero, the runner scales the StatefulSet to N. N is `parallelCompilation.replicas` when parallel compilation is enabled, otherwise 1. +- After `idleTimeoutSeconds` without pending compilation work, the runner scales the StatefulSet to zero. + +A small always-on artifact server Deployment (`-compiler-artifact-server`) owns the binary store and serves compiled pipeline binaries and SQL program validation. The `-compiler-server-0` service routes to it, so the api-server, the runner, and pipeline pods keep working while the compiler workers are scaled to zero. All compiler workers, including worker 0, upload their binaries to the artifact server. + +### Configuration Defaults + +```yaml +compilerAutoscaling: + # Experimental: scale the compiler server StatefulSet to zero when idle. + enabled: false + # Seconds without pending compilation work before scaling to zero. + idleTimeoutSeconds: 1800 + # Seconds between checks for pending compilation work. + pollIntervalSeconds: 10 + artifactServer: + httpWorkers: 2 + pvcSize: 20Gi + # The artifact server also serves SQL program validation (a JVM); the CPU + # limit lets the JVM size its heap. + resources: + requests: + cpu: "1" + memory: 2000Mi + limits: + cpu: "1" + memory: 2000Mi + # Seed the artifact store once from the compiler-server-0 PVC of an + # existing installation. Keep false on fresh installs. + seedFromCompilerServer0: false +``` + +### Latency Expectations + +A compilation submitted while the compilers are scaled to zero waits for the full cold start: the autoscaler poll (up to `pollIntervalSeconds`), node provisioning if the cluster must add compiler nodes, image pull, and compiler server startup including precompiled dependency extraction. The pipeline shows `Pending` during this time; that is expected, not a stall. Compilations submitted while workers are already up behave exactly as without autoscaling. + +Starting or restarting a pipeline whose binary is already compiled does not wake the compilers; the artifact server serves the stored binary directly. + +### Enabling on an Existing Installation + +Enabling changes the StatefulSet `podManagementPolicy` (an immutable field) and moves the binary store to the artifact server PVC, so it is a short maintenance event rather than a pure values change: + +1. Upgrade to images that support autoscaling, with `compilerAutoscaling.enabled` still `false`. Older images do not understand the new flags and would crash loop. +2. Run the enabling upgrade with `compilerAutoscaling.enabled=true` and `compilerAutoscaling.artifactServer.seedFromCompilerServer0=true`. While the seed setting is true the chart omits the compiler StatefulSet, so this upgrade deletes the existing one and frees its ReadWriteOnce `compiler-storage--compiler-server-0` volume; the artifact server then copies the compiled binaries from that volume into the artifact store, so existing pipelines keep their binaries. No compilation runs during this window. +3. Wait until the artifact server pod is `Running` (the copy happens in its init container). +4. Run a follow-up upgrade with `seedFromCompilerServer0` back to `false`. This recreates the compiler StatefulSet (fresh, with the required `podManagementPolicy`) and detaches the old volume from the artifact server. + +Fresh installations skip this procedure: set `compilerAutoscaling.enabled=true` at install time. + +### Disabling + +1. Copy binaries compiled while autoscaling was enabled back to the compiler-server-0 volume. Without this step, a running pipeline whose pod restarts after disabling cannot fetch its binary until the program is recompiled. Scale the artifact server down so its ReadWriteOnce volume is free: + + ```bash + kubectl scale deployment -compiler-artifact-server -n --replicas=0 + ``` + + then run a one-shot pod that mounts both the `-compiler-artifact-server` and the `compiler-storage--compiler-server-0` PVCs and copies `rust-compilation/pipeline-binaries` across (the mirror image of the seed init container). +2. Delete the compiler StatefulSet (disabling reverts `podManagementPolicy`, which is immutable): + + ```bash + kubectl delete statefulset -compiler-server -n + ``` + +3. Run `helm upgrade` with `compilerAutoscaling.enabled=false`. Helm recreates the StatefulSet and restores the configured replica count. The artifact store PVC carries `helm.sh/resource-policy: keep`, so it survives disabling and is re-adopted if you re-enable later. +4. Verify with `kubectl get statefulset -compiler-server -n ` that the replica count matches your configuration. + +If the runner starts with autoscaling disabled and finds the compiler StatefulSet scaled to zero (for example after a disable that raced the old autoscaler), it patches the StatefulSet to 1 replica so compilation never stays wedged; the next `helm upgrade` restores the configured count. + +:::warning +Avoid `helm upgrade --force` while autoscaling is enabled: it replaces the StatefulSet, which resets `spec.replicas` and restarts compiler pods; the autoscaler restores the correct count within one poll interval, but in-flight compilations restart. Do not `helm rollback` across the enable boundary either: the rollback tries to revert the immutable `podManagementPolicy` field and fails on the StatefulSet, leaving the release half rolled back. To return to a pre-autoscaling revision, follow the Disabling procedure instead. +::: + +### Autoscaling Troubleshooting + +- **`/cluster_healthz` reports `scaled_to_zero`:** + +While the compilers are parked at zero, the health endpoint reports the compiler section as healthy with a `"scaled_to_zero": true` marker. This is the intended idle state, not a failure. + +- **`/cluster_healthz` reports the compiler not ready during scale-up:** + +During a 0 to N cold start the compiler section reports not ready together with a note that autoscaling is active; this resolves once the compiler pods pass their startup probes. Only a not-ready state that persists well beyond the expected cold start indicates a real problem (for example unschedulable pods or exhausted quota). + +- **`SCALING DETECTED` restarts during transitions:** + +Compiler pods that observe a replica change exit with `SCALING DETECTED` and restart with the new worker count. During 0 to N and N to 0 transitions these restarts are expected and bounded; the pods converge as soon as the transition completes. + +- **Compilers never scale down:** + +A compilation that never reaches a terminal status keeps demand pending and keeps the workers up. The typical cause is a compile that is OOM-killed on every attempt: the pipeline cycles between `SqlCompiled` and `CompilingRust` forever. Give the compiler pods more memory or remove the offending pipeline. + --- ## Troubleshooting & FAQs @@ -143,9 +241,9 @@ Ensure your cluster nodes have enough resources to run the desired number of com If a pipeline is assigned to a worker pod that is not yet running or is unhealthy, it will not be compiled until that pod is available and running. Make sure to validate all pods are running. -- **SystemError: Failed to upload binary:** +- **Failed to upload binary:** -If the pipeline gets this status, that means pod N failed to upload its binary to `<_>-compiler-server-0`. +If a worker pod cannot upload its binary to `<_>-compiler-server-0` due to a transient failure (network error, HTTP 5xx), the compilation stays in a compiling status and is retried once the upload target is reachable again. A permanent rejection (an HTTP 4xx response, for example a proxy body-size limit) surfaces as `SystemError` instead of retrying forever. You can check if `<_>-compiler-server-0` is healthy or not by `/cluster_healthz` endpoint. Make sure to adjust binary upload related configuration as per your needs, e.g. if your _upgrade_ takes a while, we should configure retries and backoff interval to sane values such that pods get time to come up to receive the binary. @@ -179,4 +277,15 @@ Comman causes can be misconfigured S3 bucket / endpoint / credentials. | `parallelCompilation.sccache.s3.serverSideEncryption` | Enable server-side encryption with s3 managed key (SSE-S3) | `false` | | `parallelCompilation.sccache.s3.endpoint` | Custom endpoint (e.g. MinIO) | `minio.mydomain.com:9000` | +**Autoscaling (Experimental)** +| Key | Description | Default/Example | +|-----|-------------|-----------------| +| `compilerAutoscaling.enabled` | Scale the compiler server StatefulSet to zero when idle | `false` (ex: `true`) | +| `compilerAutoscaling.idleTimeoutSeconds` | Seconds without pending compilation work before scaling to zero | `1800` | +| `compilerAutoscaling.pollIntervalSeconds` | Seconds between checks for pending compilation work | `10` | +| `compilerAutoscaling.artifactServer.httpWorkers` | HTTP worker threads of the artifact server | `2` | +| `compilerAutoscaling.artifactServer.pvcSize` | Size of the artifact store volume | `20Gi` | +| `compilerAutoscaling.artifactServer.resources` | Artifact server pod resources | `1` CPU, `2000Mi` memory | +| `compilerAutoscaling.artifactServer.seedFromCompilerServer0` | Seed the artifact store from the compiler-server-0 PVC during the enabling upgrade | `false` (ex: `true`) | + --- From 4760e19c8924f560a8d41e0b885ae6e3ffdfae1f Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 2 Aug 2026 11:07:25 -0700 Subject: [PATCH 04/11] compiler: upload failures park in SystemError; healthz reports storage pressure Persistent binary upload failures (a full binary store, a long-dead endpoint) need an operator, so after the in-cycle retry budget the program parks in SystemError naming the cause instead of silently recompiling forever, which pinned the autoscaled fleet at N with no user-visible error when the artifact store filled up in a dev-cluster incident. Permanent 4xx rejections keep failing immediately; transient blips are still absorbed by the exponential retry backoff. The compiler /healthz gains a deep variant (?deep=1) that fails at 95% usage of the working-directory filesystem; the cluster monitor polls it so /v0/cluster_healthz warns before uploads start failing. Kubernetes probes keep the shallow variant: a full disk must not restart a pod that still serves compiled binaries. Signed-off-by: Gerd Zellweger --- .../pipeline-manager/src/cluster_monitor.rs | 4 +- crates/pipeline-manager/src/compiler/main.rs | 59 ++++++++++++++++++- .../src/compiler/rust_compiler.rs | 17 ++++-- .../enterprise/parallel-compilation.md | 6 +- 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/crates/pipeline-manager/src/cluster_monitor.rs b/crates/pipeline-manager/src/cluster_monitor.rs index 0fbc3f57e43..bfdf15421f0 100644 --- a/crates/pipeline-manager/src/cluster_monitor.rs +++ b/crates/pipeline-manager/src/cluster_monitor.rs @@ -72,8 +72,10 @@ pub async fn cluster_monitor( "{protocol}://{}:{}/healthz", common_config.api_host, common_config.api_port ); + // The deep variant additionally fails on storage pressure of the compiler + // working directory, which would make binary uploads fail with ENOSPC. let compiler_url = format!( - "{protocol}://{}:{}/healthz", + "{protocol}://{}:{}/healthz?deep=1", common_config.compiler_host, common_config.compiler_port ); let runner_url = format!( diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index 7ebcd901e54..c45d9fd02f3 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -10,7 +10,7 @@ use crate::compiler::sql_compiler::{ validate_program, ProgramValidationRequest, SqlCompilationError, SqlCompilationOutput, }; use crate::compiler::util::{ - pipeline_binary_filename, program_info_filename, validate_is_sha256_checksum, + pipeline_binary_filename, program_info_filename, validate_is_sha256_checksum, DiskSpace, }; use crate::config::{CommonConfig, CompilerConfig}; use crate::db::probe::DbProbe; @@ -649,9 +649,47 @@ async fn remove_temp_upload_file(temp_file_path: &Path) { } } +/// Fraction of the working-directory filesystem above which the deep health +/// check reports storage pressure: binary uploads are about to fail with +/// ENOSPC and an operator must grow the volume or reclaim space. +const STORAGE_PRESSURE_THRESHOLD: f64 = 0.95; + +/// Message for the deep health check when the working-directory filesystem is +/// above `threshold`, `None` when it is not. +fn storage_pressure_message(disk_space: &DiskSpace, threshold: f64) -> Option { + if disk_space.used_fraction < threshold { + return None; + } + Some(format!( + "unhealthy: compiler working directory filesystem is {:.1}% full ({} of {} bytes used); \ + binary uploads will fail until an operator grows the volume or storage is reclaimed", + disk_space.used_fraction * 100.0, + disk_space.used_byte, + disk_space.total_byte + )) +} + /// Health check which returns success if it is able to reach the database. +/// With a `deep` query parameter it additionally fails on storage pressure of +/// the working-directory filesystem. Kubernetes probes use the shallow +/// variant: a full disk must not restart the pod, which still serves already +/// compiled binaries; the cluster monitor uses the deep variant so +/// /v0/cluster_healthz surfaces the condition to operators. #[get("/healthz")] -async fn healthz(probe: web::Data>>) -> Result { +async fn healthz( + probe: web::Data>>, + config: web::Data, + req: HttpRequest, +) -> Result { + if req.query_string().contains("deep") { + if let Some(disk_space) = DiskSpace::new_from_path(&config.working_dir()) { + if let Some(message) = storage_pressure_message(&disk_space, STORAGE_PRESSURE_THRESHOLD) + { + return Ok(HttpResponse::ServiceUnavailable() + .json(serde_json::json!({ "status": message }))); + } + } + } Ok(probe.lock().await.as_http_response()) } @@ -1282,6 +1320,23 @@ mod test { ); } + /// The deep health check reports storage pressure at or above the + /// threshold and stays silent below it. + #[test] + fn storage_pressure_threshold() { + let disk_space = |used_fraction: f64| crate::compiler::util::DiskSpace { + total_byte: 100, + used_byte: (used_fraction * 100.0) as u64, + used_fraction, + available_byte: 100 - (used_fraction * 100.0) as u64, + available_fraction: 1.0 - used_fraction, + }; + assert!(super::storage_pressure_message(&disk_space(0.5), 0.95).is_none()); + assert!(super::storage_pressure_message(&disk_space(0.95), 0.95).is_some()); + let message = super::storage_pressure_message(&disk_space(1.0), 0.95).unwrap(); + assert!(message.contains("100.0% full")); + } + /// Stale-entry removal deletes only entries older than the maximum age and /// only entries of the requested kind. #[tokio::test] diff --git a/crates/pipeline-manager/src/compiler/rust_compiler.rs b/crates/pipeline-manager/src/compiler/rust_compiler.rs index 553127f4167..cb7c80fd591 100644 --- a/crates/pipeline-manager/src/compiler/rust_compiler.rs +++ b/crates/pipeline-manager/src/compiler/rust_compiler.rs @@ -342,13 +342,22 @@ async fn attempt_end_to_end_rust_compilation( ); } RustCompilationError::FileUploadError(upload_error) => { - // No status is written: the row stays `CompilingRust` and the next - // iteration's reset returns it to `SqlCompiled`, so the compile and - // upload are retried once the upload endpoint is reachable again. + // The in-cycle retry budget already absorbed transient blips, so a + // failure here (endpoint down for long, disk full) needs an operator; + // a terminal status tells the user why their compile failed. + db.lock() + .await + .transit_program_status_to_system_error( + tenant_id, + pipeline.id, + pipeline.program_version, + &upload_error, + ) + .await?; error!( pipeline_id = %pipeline.id, pipeline = %pipeline.name, - "Rust compilation failed due to binary upload error; it will be retried (program version: {}): {upload_error}", + "Rust compilation failed due to binary upload error (program version: {}): {upload_error}", pipeline.program_version ); } diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index b1303fd7f3d..6393a33be21 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -225,6 +225,10 @@ During a 0 to N cold start the compiler section reports not ready together with Compiler pods that observe a replica change exit with `SCALING DETECTED` and restart with the new worker count. During 0 to N and N to 0 transitions these restarts are expected and bounded; the pods converge as soon as the transition completes. +- **`/cluster_healthz` reports the compiler unhealthy with a storage message:** + +The compiler health check fails once the binary store filesystem is 95% full, before uploads start failing with `No space left on device`. Grow the artifact server PVC (the storage class must support volume expansion) or delete unused pipelines so the garbage collector reclaims their binaries. Budget roughly 200 to 300 MB per optimized program version when sizing `compilerAutoscaling.artifactServer.pvcSize`. + - **Compilers never scale down:** A compilation that never reaches a terminal status keeps demand pending and keeps the workers up. The typical cause is a compile that is OOM-killed on every attempt: the pipeline cycles between `SqlCompiled` and `CompilingRust` forever. Give the compiler pods more memory or remove the offending pipeline. @@ -243,7 +247,7 @@ If a pipeline is assigned to a worker pod that is not yet running or is unhealth - **Failed to upload binary:** -If a worker pod cannot upload its binary to `<_>-compiler-server-0` due to a transient failure (network error, HTTP 5xx), the compilation stays in a compiling status and is retried once the upload target is reachable again. A permanent rejection (an HTTP 4xx response, for example a proxy body-size limit) surfaces as `SystemError` instead of retrying forever. +Transient upload failures (network errors, HTTP 5xx) are retried with exponential backoff inside the compilation attempt; with the default retry settings this absorbs roughly half an hour of outage, for example a restart of the upload target during an upgrade. A permanent rejection (an HTTP 4xx response, for example a proxy body-size limit) or a failure that outlives the retry budget surfaces as `SystemError` with the underlying cause, such as `No space left on device` when the binary store volume is full. After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. You can check if `<_>-compiler-server-0` is healthy or not by `/cluster_healthz` endpoint. Make sure to adjust binary upload related configuration as per your needs, e.g. if your _upgrade_ takes a while, we should configure retries and backoff interval to sane values such that pods get time to come up to receive the binary. From 574f08f8ec375a7567a5672d215574f4c276cec5 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 2 Aug 2026 11:32:12 -0700 Subject: [PATCH 05/11] compiler: fail uploads fast on a full binary store (507) The artifact server returns 507 Insufficient Storage when a binary or program info write fails with ENOSPC, and workers classify 507 as a permanent rejection: the compilation parks in SystemError on the first attempt instead of retrying for half an hour against a volume that only an operator can grow. Other 5xx responses keep the retry budget for genuinely transient failures. Signed-off-by: Gerd Zellweger --- crates/pipeline-manager/src/compiler/main.rs | 55 ++++++++++++++++++- .../src/compiler/rust_compiler.rs | 30 +++++++++- .../enterprise/parallel-compilation.md | 2 +- 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index c45d9fd02f3..d75900a6d82 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -362,7 +362,14 @@ async fn upload_binary( &expected_integrity_checksum, )); - let total_size = save_file(&target_file_path, payload, &expected_integrity_checksum).await?; + let total_size = match save_file(&target_file_path, payload, &expected_integrity_checksum).await + { + Ok(total_size) => total_size, + Err(error) if is_out_of_storage_error(&error) => { + return Ok(insufficient_storage_response(&error)); + } + Err(error) => return Err(error), + }; info!( pipeline_id = %pipeline_id, @@ -457,7 +464,14 @@ async fn upload_program_info( &expected_integrity_checksum, )); - let total_size = save_file(&target_file_path, payload, &expected_integrity_checksum).await?; + let total_size = match save_file(&target_file_path, payload, &expected_integrity_checksum).await + { + Ok(total_size) => total_size, + Err(error) if is_out_of_storage_error(&error) => { + return Ok(insufficient_storage_response(&error)); + } + Err(error) => return Err(error), + }; info!( pipeline_id = %pipeline_id, @@ -636,6 +650,28 @@ async fn stream_to_file_and_verify( Ok(total_size) } +/// Whether the error is an out-of-storage I/O error (ENOSPC). +fn is_out_of_storage_error(error: &ManagerError) -> bool { + let ManagerError::CommonError { + common_error: CommonError::IoError { io_error, .. }, + } = error + else { + return false; + }; + // ENOSPC is errno 28 on Linux and macOS. + io_error.raw_os_error() == Some(28) +} + +/// 507 Insufficient Storage response for an upload that failed with ENOSPC. +/// The distinct status lets workers fail the compilation fast with a +/// user-visible error instead of burning their retry budget on a volume that +/// only an operator can grow. +fn insufficient_storage_response(error: &ManagerError) -> HttpResponse { + HttpResponse::InsufficientStorage().json(serde_json::json!({ + "message": format!("Insufficient storage on the binary store: {error}"), + })) +} + /// Removes a temp upload file. Failure only leaves an orphan file behind, so /// it is logged rather than propagated. async fn remove_temp_upload_file(temp_file_path: &Path) { @@ -1320,6 +1356,21 @@ mod test { ); } + /// ENOSPC I/O errors are recognized as out-of-storage; other errors are not. + #[test] + fn out_of_storage_error_detection() { + let enospc = ManagerError::from(crate::common_error::CommonError::io_error( + "writing".to_string(), + std::io::Error::from_raw_os_error(28), + )); + assert!(super::is_out_of_storage_error(&enospc)); + let other_io = ManagerError::from(crate::common_error::CommonError::io_error( + "writing".to_string(), + std::io::Error::new(std::io::ErrorKind::PermissionDenied, "denied"), + )); + assert!(!super::is_out_of_storage_error(&other_io)); + } + /// The deep health check reports storage pressure at or above the /// threshold and stays silent below it. #[test] diff --git a/crates/pipeline-manager/src/compiler/rust_compiler.rs b/crates/pipeline-manager/src/compiler/rust_compiler.rs index cb7c80fd591..4eca09fd58a 100644 --- a/crates/pipeline-manager/src/compiler/rust_compiler.rs +++ b/crates/pipeline-manager/src/compiler/rust_compiler.rs @@ -733,8 +733,13 @@ async fn upload_program_info_to_endpoint_with_retries( } /// Returns true when the upload response status indicates a permanent -/// rejection (4xx other than 408 and 429) that no retry or recompile can fix. +/// rejection that no retry can fix: a 4xx other than 408 and 429, or 507 +/// Insufficient Storage (the binary store is full and only an operator can +/// grow it, so retrying just delays the user-visible error). fn is_permanent_upload_rejection(status: reqwest::StatusCode) -> bool { + if status == reqwest::StatusCode::INSUFFICIENT_STORAGE { + return true; + } status.is_client_error() && status != reqwest::StatusCode::REQUEST_TIMEOUT && status != reqwest::StatusCode::TOO_MANY_REQUESTS @@ -2391,7 +2396,7 @@ mod test { use crate::compiler::rust_compiler::prepare_workspace; use crate::compiler::rust_compiler::{ calculate_source_checksum, decide_cleanup, decide_pipeline_binary_cleanup, - STALE_TEMP_UPLOAD_MAX_AGE, + is_permanent_upload_rejection, STALE_TEMP_UPLOAD_MAX_AGE, }; use crate::compiler::test::{list_content_as_sorted_names, CompilerTest}; use crate::compiler::util::{ @@ -2641,6 +2646,27 @@ mod test { /// Checks the pipeline binaries cleanup decision: stale `.tmp-` uploads are /// removed, fresh or unstat-able ones are kept, and prefix matching decides the rest. + /// 4xx (except 408 and 429) and 507 fail an upload permanently; transient + /// statuses stay retryable. + #[test] + fn permanent_upload_rejection_classification() { + use reqwest::StatusCode; + assert!(is_permanent_upload_rejection(StatusCode::BAD_REQUEST)); + assert!(is_permanent_upload_rejection(StatusCode::NOT_FOUND)); + assert!(is_permanent_upload_rejection(StatusCode::PAYLOAD_TOO_LARGE)); + assert!(is_permanent_upload_rejection( + StatusCode::INSUFFICIENT_STORAGE + )); + assert!(!is_permanent_upload_rejection(StatusCode::REQUEST_TIMEOUT)); + assert!(!is_permanent_upload_rejection( + StatusCode::TOO_MANY_REQUESTS + )); + assert!(!is_permanent_upload_rejection( + StatusCode::INTERNAL_SERVER_ERROR + )); + assert!(!is_permanent_upload_rejection(StatusCode::BAD_GATEWAY)); + } + #[test] fn pipeline_binary_cleanup_decision() { let valid_binaries = vec!["pipeline_a_v1_".to_string()]; diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index 6393a33be21..3546a7d42e9 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -247,7 +247,7 @@ If a pipeline is assigned to a worker pod that is not yet running or is unhealth - **Failed to upload binary:** -Transient upload failures (network errors, HTTP 5xx) are retried with exponential backoff inside the compilation attempt; with the default retry settings this absorbs roughly half an hour of outage, for example a restart of the upload target during an upgrade. A permanent rejection (an HTTP 4xx response, for example a proxy body-size limit) or a failure that outlives the retry budget surfaces as `SystemError` with the underlying cause, such as `No space left on device` when the binary store volume is full. After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. +Transient upload failures (network errors, HTTP 5xx) are retried with exponential backoff inside the compilation attempt; with the default retry settings this absorbs roughly half an hour of outage, for example a restart of the upload target during an upgrade. Permanent failures surface as `SystemError` with the underlying cause and skip the retry budget entirely: an HTTP 4xx rejection (for example a proxy body-size limit), or HTTP 507 when the binary store volume is full (`Insufficient storage on the binary store`). A retryable failure that outlives the retry budget also ends in `SystemError`. After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. You can check if `<_>-compiler-server-0` is healthy or not by `/cluster_healthz` endpoint. Make sure to adjust binary upload related configuration as per your needs, e.g. if your _upgrade_ takes a while, we should configure retries and backoff interval to sane values such that pods get time to come up to receive the binary. From 17046545ee8c14bf9f10c3e3ab58e6fbc1926a87 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 2 Aug 2026 18:52:25 -0700 Subject: [PATCH 06/11] compiler: apply craft review feedback Typed deep-healthz query instead of substring matching, StorageFull kind instead of errno 28, one shared jar-cache retention constant, the demand-count invariant referenced from the four worker queries it mirrors, a shared upload-failure classifier, and docs corrections (50Gi artifact store, safe disabling order, upload failure classes). Signed-off-by: Gerd Zellweger --- .../pipeline-manager/src/cluster_monitor.rs | 2 +- crates/pipeline-manager/src/compiler/main.rs | 42 ++++++++++++------- .../src/compiler/rust_compiler.rs | 37 ++++++++-------- .../src/compiler/sql_compiler.rs | 12 +++--- .../src/db/operations/pipeline.rs | 4 ++ crates/pipeline-manager/src/db/storage.rs | 14 ++----- .../enterprise/parallel-compilation.md | 35 +++++++++------- 7 files changed, 80 insertions(+), 66 deletions(-) diff --git a/crates/pipeline-manager/src/cluster_monitor.rs b/crates/pipeline-manager/src/cluster_monitor.rs index bfdf15421f0..c81c93eab07 100644 --- a/crates/pipeline-manager/src/cluster_monitor.rs +++ b/crates/pipeline-manager/src/cluster_monitor.rs @@ -75,7 +75,7 @@ pub async fn cluster_monitor( // The deep variant additionally fails on storage pressure of the compiler // working directory, which would make binary uploads fail with ENOSPC. let compiler_url = format!( - "{protocol}://{}:{}/healthz?deep=1", + "{protocol}://{}:{}/healthz?deep=true", common_config.compiler_host, common_config.compiler_port ); let runner_url = format!( diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index d75900a6d82..654175337a9 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -8,6 +8,7 @@ use crate::compiler::rust_compiler::{ use crate::compiler::sql_compiler::{ ephemeral_compilation_dir, jar_cache_dir, perform_sql_compilation, sql_compiler_task, validate_program, ProgramValidationRequest, SqlCompilationError, SqlCompilationOutput, + JAR_CACHE_RETENTION, }; use crate::compiler::util::{ pipeline_binary_filename, program_info_filename, validate_is_sha256_checksum, DiskSpace, @@ -526,8 +527,8 @@ async fn save_file( ) -> Result { let target_file_name = target_file_path .file_name() - .map(|name| name.to_string_lossy().to_string()) - .unwrap_or_default(); + .expect("target_file_path must name a file") + .to_string_lossy(); let temp_file_path = target_file_path.with_file_name(format!("{target_file_name}.tmp-{}", Uuid::now_v7())); @@ -658,8 +659,7 @@ fn is_out_of_storage_error(error: &ManagerError) -> bool { else { return false; }; - // ENOSPC is errno 28 on Linux and macOS. - io_error.raw_os_error() == Some(28) + io_error.kind() == std::io::ErrorKind::StorageFull } /// 507 Insufficient Storage response for an upload that failed with ENOSPC. @@ -691,9 +691,12 @@ async fn remove_temp_upload_file(temp_file_path: &Path) { const STORAGE_PRESSURE_THRESHOLD: f64 = 0.95; /// Message for the deep health check when the working-directory filesystem is -/// above `threshold`, `None` when it is not. -fn storage_pressure_message(disk_space: &DiskSpace, threshold: f64) -> Option { - if disk_space.used_fraction < threshold { +/// at or above `used_fraction_threshold`, `None` when it is below. +fn storage_pressure_message( + disk_space: &DiskSpace, + used_fraction_threshold: f64, +) -> Option { + if disk_space.used_fraction < used_fraction_threshold { return None; } Some(format!( @@ -705,8 +708,15 @@ fn storage_pressure_message(disk_space: &DiskSpace, threshold: f64) -> Option Option>>, config: web::Data, - req: HttpRequest, + query: web::Query, ) -> Result { - if req.query_string().contains("deep") { + if query.deep { if let Some(disk_space) = DiskSpace::new_from_path(&config.working_dir()) { if let Some(message) = storage_pressure_message(&disk_space, STORAGE_PRESSURE_THRESHOLD) { @@ -1012,9 +1022,6 @@ async fn spawn_compiler_http_server( /// validation; live validations finish within seconds. const ORPHANED_EPHEMERAL_DIR_MAX_AGE: Duration = Duration::from_secs(3600); -/// Age above which the janitor removes a cached SQL compiler jar. -const STALE_JAR_MAX_AGE: Duration = Duration::from_secs(30 * 24 * 3600); - /// Janitor of the artifact server: garbage-collects pipeline binaries of /// deleted or recompiled pipelines, ephemeral validation directories orphaned /// by crashed validations, and stale SQL compiler jars. Errors within a pass @@ -1033,9 +1040,11 @@ async fn artifact_server_janitor_task(config: CompilerConfig, db: Arc { - // A permanent rejection cannot be fixed by retrying; surface - // it immediately so a terminal status is written. + // Permanent rejections skip the retry budget. if matches!(e, RustCompilationError::SystemError(_)) { return Err(e); } @@ -697,8 +696,7 @@ async fn upload_program_info_to_endpoint_with_retries( return Ok(result); } Err(e) => { - // A permanent rejection cannot be fixed by retrying; surface - // it immediately so a terminal status is written. + // Permanent rejections skip the retry budget. if matches!(e, RustCompilationError::SystemError(_)) { return Err(e); } @@ -745,6 +743,17 @@ fn is_permanent_upload_rejection(status: reqwest::StatusCode) -> bool { && status != reqwest::StatusCode::TOO_MANY_REQUESTS } +/// Classifies a non-2xx upload response: a permanent rejection becomes a +/// terminal `SystemError` because retrying or recompiling cannot succeed; +/// anything else stays a retryable `FileUploadError`. +fn upload_failure_error(status: reqwest::StatusCode, message: String) -> RustCompilationError { + if is_permanent_upload_rejection(status) { + RustCompilationError::SystemError(message) + } else { + RustCompilationError::FileUploadError(message) + } +} + /// Uploads the compiled binary to an HTTP endpoint (single attempt) using streaming. async fn upload_binary_to_endpoint( common_config: &CommonConfig, @@ -825,13 +834,7 @@ async fn upload_binary_to_endpoint( .await .unwrap_or_else(|_| "Unknown error".to_string()); let message = format!("Binary upload failed with status {status}: {error_text}"); - // A 4xx other than 408/429 is a permanent rejection: retrying or - // recompiling cannot succeed, so surface a terminal SystemError. - return Err(if is_permanent_upload_rejection(status) { - RustCompilationError::SystemError(message) - } else { - RustCompilationError::FileUploadError(message) - }); + return Err(upload_failure_error(status, message)); } // Get response body for any location information @@ -897,13 +900,7 @@ async fn upload_program_info_to_endpoint( .await .unwrap_or_else(|_| "Unknown error".to_string()); let message = format!("Program info upload failed with status {status}: {error_text}"); - // A 4xx other than 408/429 is a permanent rejection: retrying or - // recompiling cannot succeed, so surface a terminal SystemError. - return Err(if is_permanent_upload_rejection(status) { - RustCompilationError::SystemError(message) - } else { - RustCompilationError::FileUploadError(message) - }); + return Err(upload_failure_error(status, message)); } // Get response body for any location information @@ -2644,8 +2641,6 @@ mod test { } } - /// Checks the pipeline binaries cleanup decision: stale `.tmp-` uploads are - /// removed, fresh or unstat-able ones are kept, and prefix matching decides the rest. /// 4xx (except 408 and 429) and 507 fail an upload permanently; transient /// statuses stay retryable. #[test] @@ -2667,6 +2662,8 @@ mod test { assert!(!is_permanent_upload_rejection(StatusCode::BAD_GATEWAY)); } + /// Stale `.tmp-` uploads are removed, fresh or unstat-able ones are kept, + /// and prefix matching decides the rest. #[test] fn pipeline_binary_cleanup_decision() { let valid_binaries = vec!["pipeline_a_v1_".to_string()]; diff --git a/crates/pipeline-manager/src/compiler/sql_compiler.rs b/crates/pipeline-manager/src/compiler/sql_compiler.rs index 26a6c720de3..42042c128ce 100644 --- a/crates/pipeline-manager/src/compiler/sql_compiler.rs +++ b/crates/pipeline-manager/src/compiler/sql_compiler.rs @@ -334,6 +334,9 @@ impl From for SqlCompilationError { } } +/// How long a cached SQL compiler jar is retained after its last use. +pub(crate) const JAR_CACHE_RETENTION: Duration = Duration::from_secs(7 * 24 * 3600); + /// Directory in which downloaded SQL compiler jars for non-platform runtime /// versions are cached. pub(crate) fn jar_cache_dir(config: &CompilerConfig) -> PathBuf { @@ -1040,16 +1043,15 @@ pub(crate) async fn cleanup_sql_compilation( "SQL JAR cache", &jar_cache_dir, Arc::new(move |_jar_name: &str, metadata: Option| { - // Get rid of JAR files that have not been accessed within the last week - const MAX_AGE: Duration = Duration::from_secs(7*24*60*60); + // Get rid of JAR files that have not been accessed within the retention window if let Some(metadata) = metadata { match metadata.accessed() { Ok(atime) => { if let Ok(elapsed) = atime.elapsed() { - if elapsed < MAX_AGE { - trace!("Keeping {_jar_name} because it was accessed within the last 7 days ({elapsed:?} ago)"); + if elapsed < JAR_CACHE_RETENTION { + trace!("Keeping {_jar_name} because it was accessed within the retention window ({elapsed:?} ago)"); CleanupDecision::Keep { - motivation: "Accessed within the last week".to_string(), + motivation: "Accessed within the retention window".to_string(), } } else { CleanupDecision::Remove diff --git a/crates/pipeline-manager/src/db/operations/pipeline.rs b/crates/pipeline-manager/src/db/operations/pipeline.rs index 37212396476..d741dc06272 100644 --- a/crates/pipeline-manager/src/db/operations/pipeline.rs +++ b/crates/pipeline-manager/src/db/operations/pipeline.rs @@ -1859,6 +1859,7 @@ pub(crate) async fn list_pipelines_across_all_tenants_needing_sql_compilation_cl worker_id: usize, total_workers: usize, ) -> Result, DBError> { + // Predicate changes must be mirrored in count_pipelines_needing_compilation. let stmt = txn .prepare_cached(&format!( "SELECT p.tenant_id, {PIPELINE_COLUMNS_MONITORING} @@ -1908,6 +1909,7 @@ pub(crate) async fn list_pipelines_across_all_tenants_needing_rust_compilation_c worker_id: usize, total_workers: usize, ) -> Result, DBError> { + // Predicate changes must be mirrored in count_pipelines_needing_compilation. let stmt = txn .prepare_cached(&format!( "SELECT p.tenant_id, {PIPELINE_COLUMNS_MONITORING} @@ -1952,6 +1954,7 @@ pub(crate) async fn get_next_sql_compilation( worker_id: usize, total_workers: usize, ) -> Result, DBError> { + // Predicate changes must be mirrored in count_pipelines_needing_compilation. let stmt = txn .prepare_cached(&format!( "SELECT p.tenant_id, {PIPELINE_COLUMNS_ALL} @@ -1992,6 +1995,7 @@ pub(crate) async fn get_next_rust_compilation( worker_id: usize, total_workers: usize, ) -> Result, DBError> { + // Predicate changes must be mirrored in count_pipelines_needing_compilation. let stmt = txn .prepare_cached(&format!( "SELECT p.tenant_id, {PIPELINE_COLUMNS_ALL} diff --git a/crates/pipeline-manager/src/db/storage.rs b/crates/pipeline-manager/src/db/storage.rs index 2f3233862bd..6269f369c48 100644 --- a/crates/pipeline-manager/src/db/storage.rs +++ b/crates/pipeline-manager/src/db/storage.rs @@ -656,17 +656,9 @@ pub(crate) trait Storage { /// Counts pipelines with outstanding compilation work: stopped pipelines /// whose program status is `Pending`, `CompilingSql`, `SqlCompiled`, or - /// `CompilingRust`. - /// - /// Invariant: the predicate is exactly the union of the predicates of the - /// four worker queries - /// (`list_pipelines_across_all_tenants_needing_sql_compilation_clear`, - /// `list_pipelines_across_all_tenants_needing_rust_compilation_clear`, - /// `get_next_sql_compilation` and `get_next_rust_compilation`) across all - /// shards and platform versions, so the count is greater than zero if and - /// only if some compiler worker would act. If you change one of those four - /// queries you must keep this count in sync. It drives compiler - /// autoscaling in the enterprise runner. + /// `CompilingRust`. The predicate must stay the union of the four worker + /// queries; the full invariant is documented at + /// `operations::pipeline::count_pipelines_needing_compilation`. async fn count_pipelines_needing_compilation(&self) -> Result; /// Retrieves the list of fully compiled pipeline programs (pipeline identifier, program version, diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index 3546a7d42e9..e6385e63a78 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -155,7 +155,8 @@ compilerAutoscaling: pollIntervalSeconds: 10 artifactServer: httpWorkers: 2 - pvcSize: 20Gi + # Budget roughly 200 to 300 MB per optimized program version. + pvcSize: 50Gi # The artifact server also serves SQL program validation (a JVM); the CPU # limit lets the JVM size its heap. resources: @@ -172,9 +173,9 @@ compilerAutoscaling: ### Latency Expectations -A compilation submitted while the compilers are scaled to zero waits for the full cold start: the autoscaler poll (up to `pollIntervalSeconds`), node provisioning if the cluster must add compiler nodes, image pull, and compiler server startup including precompiled dependency extraction. The pipeline shows `Pending` during this time; that is expected, not a stall. Compilations submitted while workers are already up behave exactly as without autoscaling. +A compilation submitted while the compiler workers are scaled to zero waits for the full cold start: the autoscaler poll (up to `pollIntervalSeconds`), node provisioning if the cluster must add compiler nodes, image pull, and compiler server startup including precompiled dependency extraction. The pipeline shows `Pending` during this time; that is expected, not a stall. Compilations submitted while workers are already up behave exactly as without autoscaling. -Starting or restarting a pipeline whose binary is already compiled does not wake the compilers; the artifact server serves the stored binary directly. +Starting or restarting a pipeline whose binary is already compiled does not wake the compiler workers; the artifact server serves the stored binary directly. ### Enabling on an Existing Installation @@ -189,21 +190,21 @@ Fresh installations skip this procedure: set `compilerAutoscaling.enabled=true` ### Disabling -1. Copy binaries compiled while autoscaling was enabled back to the compiler-server-0 volume. Without this step, a running pipeline whose pod restarts after disabling cannot fetch its binary until the program is recompiled. Scale the artifact server down so its ReadWriteOnce volume is free: +1. Delete the compiler StatefulSet (disabling reverts `podManagementPolicy`, which is immutable): ```bash - kubectl scale deployment -compiler-artifact-server -n --replicas=0 + kubectl delete statefulset -compiler-server -n ``` - then run a one-shot pod that mounts both the `-compiler-artifact-server` and the `compiler-storage--compiler-server-0` PVCs and copies `rust-compilation/pipeline-binaries` across (the mirror image of the seed init container). -2. Delete the compiler StatefulSet (disabling reverts `podManagementPolicy`, which is immutable): +2. Scale the artifact server down so its ReadWriteOnce volume is free: ```bash - kubectl delete statefulset -compiler-server -n + kubectl scale deployment -compiler-artifact-server -n --replicas=0 ``` -3. Run `helm upgrade` with `compilerAutoscaling.enabled=false`. Helm recreates the StatefulSet and restores the configured replica count. The artifact store PVC carries `helm.sh/resource-policy: keep`, so it survives disabling and is re-adopted if you re-enable later. -4. Verify with `kubectl get statefulset -compiler-server -n ` that the replica count matches your configuration. +3. Copy binaries compiled while autoscaling was enabled back to the compiler-server-0 volume: run a one-shot pod that mounts both the `-compiler-artifact-server` and the `compiler-storage--compiler-server-0` PVCs and copies `rust-compilation/pipeline-binaries` across (the mirror image of the seed init container). Without this step, a running pipeline whose pod restarts after disabling cannot fetch its binary until the program is recompiled. +4. Run `helm upgrade` with `compilerAutoscaling.enabled=false`. Helm recreates the StatefulSet and restores the configured replica count. The artifact store PVC carries `helm.sh/resource-policy: keep`, so it survives disabling and is re-adopted if you re-enable later. +5. Verify with `kubectl get statefulset -compiler-server -n ` that the replica count matches your configuration. If the runner starts with autoscaling disabled and finds the compiler StatefulSet scaled to zero (for example after a disable that raced the old autoscaler), it patches the StatefulSet to 1 replica so compilation never stays wedged; the next `helm upgrade` restores the configured count. @@ -215,7 +216,7 @@ Avoid `helm upgrade --force` while autoscaling is enabled: it replaces the State - **`/cluster_healthz` reports `scaled_to_zero`:** -While the compilers are parked at zero, the health endpoint reports the compiler section as healthy with a `"scaled_to_zero": true` marker. This is the intended idle state, not a failure. +While the compiler workers are parked at zero, the health endpoint reports the compiler section as healthy with a `"scaled_to_zero": true` marker. This is the intended idle state, not a failure. - **`/cluster_healthz` reports the compiler not ready during scale-up:** @@ -247,9 +248,13 @@ If a pipeline is assigned to a worker pod that is not yet running or is unhealth - **Failed to upload binary:** -Transient upload failures (network errors, HTTP 5xx) are retried with exponential backoff inside the compilation attempt; with the default retry settings this absorbs roughly half an hour of outage, for example a restart of the upload target during an upgrade. Permanent failures surface as `SystemError` with the underlying cause and skip the retry budget entirely: an HTTP 4xx rejection (for example a proxy body-size limit), or HTTP 507 when the binary store volume is full (`Insufficient storage on the binary store`). A retryable failure that outlives the retry budget also ends in `SystemError`. After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. +Upload failures fall into three classes: + + - Transient (network errors, HTTP 5xx): retried with exponential backoff inside the compilation attempt; the default retry settings absorb roughly half an hour of outage, for example a restart of the upload target during an upgrade. + - Permanent: surface immediately as `SystemError` with the underlying cause and skip the retry budget: an HTTP 4xx rejection (for example a proxy body-size limit), or HTTP 507 when the binary store volume is full (`Insufficient storage on the binary store`). + - Retry budget exhausted: a retryable failure that outlives the retry budget also ends in `SystemError`. -You can check if `<_>-compiler-server-0` is healthy or not by `/cluster_healthz` endpoint. Make sure to adjust binary upload related configuration as per your needs, e.g. if your _upgrade_ takes a while, we should configure retries and backoff interval to sane values such that pods get time to come up to receive the binary. +After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. Check `-compiler-server-0` health via the `/cluster_healthz` endpoint; if your upgrades take long, raise the upload retry settings so the upload target has time to come back up before the retry budget runs out. - **error: process didn't exit successfully: `sccache .. rustc -vV`:** @@ -281,14 +286,14 @@ Comman causes can be misconfigured S3 bucket / endpoint / credentials. | `parallelCompilation.sccache.s3.serverSideEncryption` | Enable server-side encryption with s3 managed key (SSE-S3) | `false` | | `parallelCompilation.sccache.s3.endpoint` | Custom endpoint (e.g. MinIO) | `minio.mydomain.com:9000` | -**Autoscaling (Experimental)** +**Autoscaling (experimental)** | Key | Description | Default/Example | |-----|-------------|-----------------| | `compilerAutoscaling.enabled` | Scale the compiler server StatefulSet to zero when idle | `false` (ex: `true`) | | `compilerAutoscaling.idleTimeoutSeconds` | Seconds without pending compilation work before scaling to zero | `1800` | | `compilerAutoscaling.pollIntervalSeconds` | Seconds between checks for pending compilation work | `10` | | `compilerAutoscaling.artifactServer.httpWorkers` | HTTP worker threads of the artifact server | `2` | -| `compilerAutoscaling.artifactServer.pvcSize` | Size of the artifact store volume | `20Gi` | +| `compilerAutoscaling.artifactServer.pvcSize` | Size of the artifact store volume | `50Gi` | | `compilerAutoscaling.artifactServer.resources` | Artifact server pod resources | `1` CPU, `2000Mi` memory | | `compilerAutoscaling.artifactServer.seedFromCompilerServer0` | Seed the artifact store from the compiler-server-0 PVC during the enabling upgrade | `false` (ex: `true`) | From 902bfc973bbb29a5afa1dffdf4792192424950df Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 2 Aug 2026 19:44:48 -0700 Subject: [PATCH 07/11] docs: single-upgrade enable and disable via the policy hook The pre-upgrade hook recreates the compiler StatefulSet on upgrades that flip the immutable podManagementPolicy, so toggling compilerAutoscaling no longer needs manual kubectl steps. Documents the no-seed enable path and restructures disabling into with and without binary preservation. Signed-off-by: Gerd Zellweger --- .../enterprise/parallel-compilation.md | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index e6385e63a78..2036eba82aa 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -179,32 +179,34 @@ Starting or restarting a pipeline whose binary is already compiled does not wake ### Enabling on an Existing Installation -Enabling changes the StatefulSet `podManagementPolicy` (an immutable field) and moves the binary store to the artifact server PVC, so it is a short maintenance event rather than a pure values change: +Toggling the feature changes the StatefulSet `podManagementPolicy`, an immutable field. A pre-upgrade hook (`compilerAutoscaling.kubectlImage`) handles that automatically: on the one upgrade that flips the policy it deletes the compiler StatefulSet, and the same upgrade recreates it with the new policy while the retained per-replica volumes re-attach. The hook renders only on such transition upgrades; steady-state upgrades and installations that never toggle the feature run no hook at all. The hook image must be reachable from your cluster (mirror it for air-gapped installations). 1. Upgrade to images that support autoscaling, with `compilerAutoscaling.enabled` still `false`. Older images do not understand the new flags and would crash loop. -2. Run the enabling upgrade with `compilerAutoscaling.enabled=true` and `compilerAutoscaling.artifactServer.seedFromCompilerServer0=true`. While the seed setting is true the chart omits the compiler StatefulSet, so this upgrade deletes the existing one and frees its ReadWriteOnce `compiler-storage--compiler-server-0` volume; the artifact server then copies the compiled binaries from that volume into the artifact store, so existing pipelines keep their binaries. No compilation runs during this window. +2. Run the enabling upgrade with `compilerAutoscaling.enabled=true` and `compilerAutoscaling.artifactServer.seedFromCompilerServer0=true`. While the seed setting is true the chart omits the compiler StatefulSet and frees its ReadWriteOnce `compiler-storage--compiler-server-0` volume; the artifact server then copies the compiled binaries from that volume into the artifact store, so existing pipelines keep their binaries. No compilation runs during this window. 3. Wait until the artifact server pod is `Running` (the copy happens in its init container). -4. Run a follow-up upgrade with `seedFromCompilerServer0` back to `false`. This recreates the compiler StatefulSet (fresh, with the required `podManagementPolicy`) and detaches the old volume from the artifact server. +4. Run a follow-up upgrade with `seedFromCompilerServer0` back to `false`. This recreates the compiler StatefulSet and detaches the old volume from the artifact server. -Fresh installations skip this procedure: set `compilerAutoscaling.enabled=true` at install time. +To enable without preserving existing binaries, skip the seeding: a single upgrade with `compilerAutoscaling.enabled=true` suffices. The artifact store then starts empty: stopped pipelines recompile on their next start, but a running pipeline whose pod restarts cannot fetch its binary until its program is recompiled (stop and start it once). Stop running pipelines first if that is not acceptable. + +Fresh installations need no procedure: set `compilerAutoscaling.enabled=true` at install time. ### Disabling -1. Delete the compiler StatefulSet (disabling reverts `podManagementPolicy`, which is immutable): +Without preserving binaries compiled while autoscaling was enabled, disabling is a single upgrade; stopped pipelines recompile on their next start: - ```bash - kubectl delete statefulset -compiler-server -n - ``` +1. Run `helm upgrade` with `compilerAutoscaling.enabled=false`. The pre-upgrade hook recreates the StatefulSet and helm restores the configured replica count. The artifact store PVC carries `helm.sh/resource-policy: keep`, so it survives disabling and is re-adopted if you re-enable later. +2. Verify with `kubectl get statefulset -compiler-server -n ` that the replica count matches your configuration. + +To preserve the binaries (required if pipelines are running and must survive pod restarts without a recompile), copy them back before the upgrade: -2. Scale the artifact server down so its ReadWriteOnce volume is free: +1. Wait until the compiler workers are parked at zero replicas (or scale the StatefulSet to zero), then scale the artifact server down so both ReadWriteOnce volumes are free: ```bash kubectl scale deployment -compiler-artifact-server -n --replicas=0 ``` -3. Copy binaries compiled while autoscaling was enabled back to the compiler-server-0 volume: run a one-shot pod that mounts both the `-compiler-artifact-server` and the `compiler-storage--compiler-server-0` PVCs and copies `rust-compilation/pipeline-binaries` across (the mirror image of the seed init container). Without this step, a running pipeline whose pod restarts after disabling cannot fetch its binary until the program is recompiled. -4. Run `helm upgrade` with `compilerAutoscaling.enabled=false`. Helm recreates the StatefulSet and restores the configured replica count. The artifact store PVC carries `helm.sh/resource-policy: keep`, so it survives disabling and is re-adopted if you re-enable later. -5. Verify with `kubectl get statefulset -compiler-server -n ` that the replica count matches your configuration. +2. Run a one-shot pod that mounts both the `-compiler-artifact-server` and the `compiler-storage--compiler-server-0` PVCs and copies `rust-compilation/pipeline-binaries` across (the mirror image of the seed init container). +3. Run `helm upgrade` with `compilerAutoscaling.enabled=false` and verify the replica count as above. If the runner starts with autoscaling disabled and finds the compiler StatefulSet scaled to zero (for example after a disable that raced the old autoscaler), it patches the StatefulSet to 1 replica so compilation never stays wedged; the next `helm upgrade` restores the configured count. From c1f690bef048e0e85c4511ca49fc0944930401a3 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 2 Aug 2026 22:01:40 -0700 Subject: [PATCH 08/11] compiler: reuse the shared cleanup helpers in the artifact server janitor The janitor's age sweeps duplicated the directory-walking machinery in util.rs and the jar staleness decision in cleanup_sql_compilation. cleanup_specific_directories gains the same metadata support as the files variant, the jar decision moves into decide_stale_jar shared by the worker cleanup and the janitor (unifying on access time), and the bespoke remove_stale_entries is gone. Also simplifies the autoscaling docs' demand description. Signed-off-by: Gerd Zellweger --- crates/pipeline-manager/src/compiler/main.rs | 201 ++++++++---------- .../src/compiler/rust_compiler.rs | 11 +- .../src/compiler/sql_compiler.rs | 95 ++++++--- crates/pipeline-manager/src/compiler/util.rs | 13 +- .../enterprise/parallel-compilation.md | 4 +- 5 files changed, 168 insertions(+), 156 deletions(-) diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index 654175337a9..0a9f9594bd7 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -6,12 +6,13 @@ use crate::compiler::rust_compiler::{ RustCompilationResult, CLEANUP_INTERVAL, }; use crate::compiler::sql_compiler::{ - ephemeral_compilation_dir, jar_cache_dir, perform_sql_compilation, sql_compiler_task, - validate_program, ProgramValidationRequest, SqlCompilationError, SqlCompilationOutput, - JAR_CACHE_RETENTION, + decide_stale_jar, ephemeral_compilation_dir, jar_cache_dir, perform_sql_compilation, + sql_compiler_task, validate_program, ProgramValidationRequest, SqlCompilationError, + SqlCompilationOutput, }; use crate::compiler::util::{ - pipeline_binary_filename, program_info_filename, validate_is_sha256_checksum, DiskSpace, + cleanup_specific_directories, cleanup_specific_files, pipeline_binary_filename, + program_info_filename, validate_is_sha256_checksum, CleanupDecision, DiskSpace, }; use crate::config::{CommonConfig, CompilerConfig}; use crate::db::probe::DbProbe; @@ -1022,6 +1023,28 @@ async fn spawn_compiler_http_server( /// validation; live validations finish within seconds. const ORPHANED_EPHEMERAL_DIR_MAX_AGE: Duration = Duration::from_secs(3600); +/// Removes an ephemeral validation directory whose modification time exceeds +/// [`ORPHANED_EPHEMERAL_DIR_MAX_AGE`]. Missing or future modification times +/// never remove. +fn decide_orphaned_ephemeral_dir( + _dir_name: &str, + metadata: Option, +) -> CleanupDecision { + let Some(modified_time) = metadata.and_then(|metadata| metadata.modified().ok()) else { + return CleanupDecision::Ignore; + }; + if modified_time + .elapsed() + .is_ok_and(|age| age >= ORPHANED_EPHEMERAL_DIR_MAX_AGE) + { + CleanupDecision::Remove + } else { + CleanupDecision::Keep { + motivation: "Validation may still be in progress".to_string(), + } + } +} + /// Janitor of the artifact server: garbage-collects pipeline binaries of /// deleted or recompiled pipelines, ephemeral validation directories orphaned /// by crashed validations, and stale SQL compiler jars. Errors within a pass @@ -1031,101 +1054,49 @@ async fn artifact_server_janitor_task(config: CompilerConfig, db: Arc Result<(), std::io::Error> { - if !dir.is_dir() { - return Ok(()); - } - let mut entries = fs::read_dir(dir).await?; - while let Some(entry) = entries.next_entry().await? { - // Entries can vanish mid-sweep when racing a finishing validation; - // one unstat-able entry must not abort the whole sweep. - let metadata = match entry.metadata().await { - Ok(metadata) => metadata, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, - Err(e) => { - warn!( - "Unable to stat entry '{}' during stale entry sweep: {e}", - entry.path().display() + let ephemeral_dir = ephemeral_compilation_dir(&config); + if ephemeral_dir.is_dir() { + if let Err(e) = cleanup_specific_directories( + "Ephemeral validation directories", + &ephemeral_dir, + Arc::new(decide_orphaned_ephemeral_dir), + false, + true, + ) + .await + { + error!( + "Artifact server janitor: ephemeral validation directory cleanup failed: {e}" ); - continue; } - }; - let matches_kind = match kind { - StaleEntryKind::Directories => metadata.is_dir(), - StaleEntryKind::Files => metadata.is_file(), - }; - if !matches_kind { - continue; } - // A modification time that is unavailable or in the future never - // counts as stale. - let Ok(modified_time) = metadata.modified() else { - continue; - }; - if !modified_time.elapsed().is_ok_and(|age| age >= max_age) { - continue; - } - let removal_result = match kind { - StaleEntryKind::Directories => fs::remove_dir_all(entry.path()).await, - StaleEntryKind::Files => fs::remove_file(entry.path()).await, - }; - match removal_result { - Ok(()) => info!("Removed stale entry '{}'", entry.path().display()), - Err(e) => warn!( - "Unable to remove stale entry '{}': {e}", - entry.path().display() - ), + let jar_cache_dir = jar_cache_dir(&config); + if jar_cache_dir.is_dir() { + if let Err(e) = cleanup_specific_files( + "SQL JAR cache", + &jar_cache_dir, + Arc::new(decide_stale_jar), + true, + true, + ) + .await + { + error!("Artifact server janitor: SQL compiler jar cache cleanup failed: {e}"); + } } + sleep(CLEANUP_INTERVAL).await; } - Ok(()) } #[cfg(test)] mod test { use crate::api::error::ApiError; use crate::compiler::main::{ - create_working_directory_if_not_exists, decode_url_encoded_parameter, remove_stale_entries, - save_file, upload_binary, StaleEntryKind, + create_working_directory_if_not_exists, decide_orphaned_ephemeral_dir, + decode_url_encoded_parameter, save_file, upload_binary, }; use crate::compiler::util::pipeline_binary_filename; + use crate::compiler::util::CleanupDecision; use crate::config::CompilerConfig; use crate::db::types::pipeline::PipelineId; use crate::db::types::program::CompilationProfile; @@ -1402,40 +1373,34 @@ mod test { assert!(message.contains("100.0% full")); } - /// Stale-entry removal deletes only entries older than the maximum age and - /// only entries of the requested kind. - #[tokio::test] - async fn test_stale_entry_removal() { + /// An old ephemeral validation directory is removed, a fresh one is kept, + /// and missing metadata never removes. + #[test] + fn orphaned_ephemeral_dir_decision() { let tempdir = tempfile::tempdir().unwrap(); - let dir = tempdir.path(); - let subdir = dir.join("subdir"); - fs::create_dir(&subdir).await.unwrap(); - let file = dir.join("file.jar"); - fs::write(&file, b"jar").await.unwrap(); - - // Entries younger than the maximum age are kept - let long_max_age = Duration::from_secs(3600); - remove_stale_entries(dir, long_max_age, StaleEntryKind::Directories) - .await - .unwrap(); - remove_stale_entries(dir, long_max_age, StaleEntryKind::Files) - .await - .unwrap(); - assert!(subdir.is_dir()); - assert!(file.is_file()); - - // Older entries are removed, but only those of the requested kind - tokio::time::sleep(Duration::from_millis(100)).await; - let short_max_age = Duration::from_millis(50); - remove_stale_entries(dir, short_max_age, StaleEntryKind::Directories) - .await - .unwrap(); - assert!(!subdir.exists()); - assert!(file.is_file()); - remove_stale_entries(dir, short_max_age, StaleEntryKind::Files) - .await + let dir_path = tempdir.path(); + let recent = std::fs::metadata(dir_path).unwrap(); + assert!(matches!( + decide_orphaned_ephemeral_dir("d", Some(recent)), + CleanupDecision::Keep { .. } + )); + let old_time = std::time::SystemTime::now() - Duration::from_secs(2 * 3600); + let times = std::fs::FileTimes::new() + .set_accessed(old_time) + .set_modified(old_time); + std::fs::File::open(dir_path) + .unwrap() + .set_times(times) .unwrap(); - assert!(!file.exists()); + let old = std::fs::metadata(dir_path).unwrap(); + assert_eq!( + decide_orphaned_ephemeral_dir("d", Some(old)), + CleanupDecision::Remove + ); + assert_eq!( + decide_orphaned_ephemeral_dir("d", None), + CleanupDecision::Ignore + ); } #[tokio::test] diff --git a/crates/pipeline-manager/src/compiler/rust_compiler.rs b/crates/pipeline-manager/src/compiler/rust_compiler.rs index 8efd637be5a..ef4871b65e4 100644 --- a/crates/pipeline-manager/src/compiler/rust_compiler.rs +++ b/crates/pipeline-manager/src/compiler/rust_compiler.rs @@ -2162,8 +2162,11 @@ async fn cleanup_rust_compilation( &mut cleanup_specific_directories( "Rust compilation crates", &crates_dir, - Arc::new(move |name: &str| decide_cleanup(name, None, &deletion_clone)), + Arc::new(move |name: &str, _metadata: Option| { + decide_cleanup(name, None, &deletion_clone) + }), true, + false, ) .await?, ); @@ -2308,10 +2311,11 @@ async fn cleanup_rust_compilation( &mut cleanup_specific_directories( &format!("Rust compilation target/{target_profile_folder}/.fingerprint"), &fingerprint_dir, - Arc::new(move |name: &str| { + Arc::new(move |name: &str, _metadata: Option| { decide_cleanup(name, Some((true, '-')), &deletion_clone) }), false, + false, ) .await?, ); @@ -2325,10 +2329,11 @@ async fn cleanup_rust_compilation( &mut cleanup_specific_directories( &format!("Rust compilation target/{target_profile_folder}/incremental"), &incremental_dir, - Arc::new(move |name: &str| { + Arc::new(move |name: &str, _metadata: Option| { decide_cleanup(name, Some((true, '-')), &deletion_clone) }), false, + false, ) .await?, ); diff --git a/crates/pipeline-manager/src/compiler/sql_compiler.rs b/crates/pipeline-manager/src/compiler/sql_compiler.rs index 42042c128ce..8453ce0bbfe 100644 --- a/crates/pipeline-manager/src/compiler/sql_compiler.rs +++ b/crates/pipeline-manager/src/compiler/sql_compiler.rs @@ -337,6 +337,35 @@ impl From for SqlCompilationError { /// How long a cached SQL compiler jar is retained after its last use. pub(crate) const JAR_CACHE_RETENTION: Duration = Duration::from_secs(7 * 24 * 3600); +/// Removes a cached SQL compiler jar that was not accessed within +/// [`JAR_CACHE_RETENTION`]; jars still read by compilations or validations +/// stay cached. Missing metadata or access times never remove. +pub(crate) fn decide_stale_jar(jar_name: &str, metadata: Option) -> CleanupDecision { + let Some(metadata) = metadata else { + debug!("Failed to get metadata for JAR file"); + return CleanupDecision::Ignore; + }; + let atime = match metadata.accessed() { + Ok(atime) => atime, + Err(e) => { + debug!("Failed to get access time for JAR file: {:?}", e); + return CleanupDecision::Ignore; + } + }; + let Ok(elapsed) = atime.elapsed() else { + warn!("Unable to determine access time for JAR file, your system clock may be set incorrectly."); + return CleanupDecision::Ignore; + }; + if elapsed < JAR_CACHE_RETENTION { + trace!("Keeping {jar_name} because it was accessed within the retention window ({elapsed:?} ago)"); + CleanupDecision::Keep { + motivation: "Accessed within the retention window".to_string(), + } + } else { + CleanupDecision::Remove + } +} + /// Directory in which downloaded SQL compiler jars for non-platform runtime /// versions are cached. pub(crate) fn jar_cache_dir(config: &CompilerConfig) -> PathBuf { @@ -1008,7 +1037,7 @@ pub(crate) async fn cleanup_sql_compilation( cleanup_specific_directories( "SQL compilation directories", &sql_pipelines_dir, - Arc::new(move |dirname: &str| { + Arc::new(move |dirname: &str, _metadata: Option| { let spl: Vec<&str> = dirname.splitn(2, '-').collect(); if spl.len() == 2 && spl[0] == "pipeline" { if let Ok(uuid) = Uuid::parse_str(spl[1]) { @@ -1032,6 +1061,7 @@ pub(crate) async fn cleanup_sql_compilation( } }), true, + false, ) .await?; } @@ -1042,35 +1072,7 @@ pub(crate) async fn cleanup_sql_compilation( cleanup_specific_files( "SQL JAR cache", &jar_cache_dir, - Arc::new(move |_jar_name: &str, metadata: Option| { - // Get rid of JAR files that have not been accessed within the retention window - if let Some(metadata) = metadata { - match metadata.accessed() { - Ok(atime) => { - if let Ok(elapsed) = atime.elapsed() { - if elapsed < JAR_CACHE_RETENTION { - trace!("Keeping {_jar_name} because it was accessed within the retention window ({elapsed:?} ago)"); - CleanupDecision::Keep { - motivation: "Accessed within the retention window".to_string(), - } - } else { - CleanupDecision::Remove - } - } else { - warn!("Unable to determine access time for JAR file, your system clock may be set incorrectly."); - CleanupDecision::Ignore - } - } - Err(e) => { - debug!("Failed to get access time for JAR file: {:?}", e); - CleanupDecision::Ignore - } - } - } else { - debug!("Failed to get metadata for JAR file"); - CleanupDecision::Ignore - } - }), + Arc::new(decide_stale_jar), true, true, ) @@ -1083,6 +1085,39 @@ pub(crate) async fn cleanup_sql_compilation( #[cfg(test)] mod test { use crate::auth::TenantRecord; + + /// A jar unaccessed past the retention window is removed, a recently + /// accessed one is kept, and missing metadata never removes. + #[test] + fn stale_jar_decision() { + use crate::compiler::sql_compiler::{decide_stale_jar, JAR_CACHE_RETENTION}; + use crate::compiler::util::CleanupDecision; + let tempdir = tempfile::tempdir().unwrap(); + let jar_path = tempdir.path().join("a.jar"); + std::fs::write(&jar_path, b"jar").unwrap(); + let recent = std::fs::metadata(&jar_path).unwrap(); + assert!(matches!( + decide_stale_jar("a.jar", Some(recent)), + CleanupDecision::Keep { .. } + )); + let old_time = std::time::SystemTime::now() - JAR_CACHE_RETENTION - JAR_CACHE_RETENTION; + let times = std::fs::FileTimes::new() + .set_accessed(old_time) + .set_modified(old_time); + std::fs::File::options() + .write(true) + .open(&jar_path) + .unwrap() + .set_times(times) + .unwrap(); + let old = std::fs::metadata(&jar_path).unwrap(); + assert_eq!( + decide_stale_jar("a.jar", Some(old)), + CleanupDecision::Remove + ); + assert_eq!(decide_stale_jar("a.jar", None), CleanupDecision::Ignore); + } + use crate::compiler::test::{list_content_as_sorted_names, CompilerTest}; use crate::compiler::util::{create_new_file, recreate_dir}; use crate::db::types::program::ProgramStatus; diff --git a/crates/pipeline-manager/src/compiler/util.rs b/crates/pipeline-manager/src/compiler/util.rs index 92baafbd7b8..7898edd095d 100644 --- a/crates/pipeline-manager/src/compiler/util.rs +++ b/crates/pipeline-manager/src/compiler/util.rs @@ -530,14 +530,20 @@ pub async fn cleanup_specific_files( pub async fn cleanup_specific_directories( cleanup_name: &str, dir: &Path, - decide: Arc CleanupDecision + Send + Sync>, + decide: DecisionFn, warn_ignore: bool, + add_metadata: bool, ) -> Result, UtilError> { let content = DirectoryContent::new(dir).await?.content; let mut keep_motivations = vec![]; for (path, name, is_file) in content { if !is_file { - match decide(&name) { + let metadata = if add_metadata { + fs::metadata(&path).await.ok() + } else { + None + }; + match decide(&name, metadata) { CleanupDecision::Keep { motivation } => { // If it should be kept, nothing needs to happen to the directory keep_motivations.push(motivation); @@ -1135,7 +1141,7 @@ mod test { cleanup_specific_directories( "", &dir_path, - Arc::new(|name: &str| { + Arc::new(|name: &str, _metadata: Option| { if name.starts_with("dir-") { CleanupDecision::Remove } else { @@ -1145,6 +1151,7 @@ mod test { } }), true, + false, ) .await .unwrap(); diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index 2036eba82aa..60d1d1eee99 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -137,8 +137,8 @@ parallelCompilation: Compiler autoscaling scales the compiler server StatefulSet between 0 and N replicas so that idle deployments stop paying for compiler nodes. The kubernetes-runner drives the scaling: -- Every `pollIntervalSeconds` the runner counts pipelines that need compilation. A pipeline counts when its deployment resources are stopped and its program status is `Pending`, `CompilingSql`, `SqlCompiled`, or `CompilingRust`. -- When the count is greater than zero, the runner scales the StatefulSet to N. N is `parallelCompilation.replicas` when parallel compilation is enabled, otherwise 1. +- Every `pollIntervalSeconds` the runner checks whether there are outstanding compilation requests. +- When there are, the runner scales the StatefulSet to N. N is `parallelCompilation.replicas` when parallel compilation is enabled, otherwise 1. - After `idleTimeoutSeconds` without pending compilation work, the runner scales the StatefulSet to zero. A small always-on artifact server Deployment (`-compiler-artifact-server`) owns the binary store and serves compiled pipeline binaries and SQL program validation. The `-compiler-server-0` service routes to it, so the api-server, the runner, and pipeline pods keep working while the compiler workers are scaled to zero. All compiler workers, including worker 0, upload their binaries to the artifact server. From e4168861d55a861d427da917c68fe09ec7ef97a8 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 2 Aug 2026 22:16:48 -0700 Subject: [PATCH 09/11] docs: tighten the autoscaling section Adds the typical 1 to 3 minute cold-start latency, concrete air-gap mirroring for the hook kubectl image, and an explicit binary-restore pod manifest with source and target paths; drops reassurance filler, the unwedge and rollback caveats, and the retry-tuning aside. Signed-off-by: Gerd Zellweger --- .../enterprise/parallel-compilation.md | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index 60d1d1eee99..6ecea10f2c7 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -155,7 +155,7 @@ compilerAutoscaling: pollIntervalSeconds: 10 artifactServer: httpWorkers: 2 - # Budget roughly 200 to 300 MB per optimized program version. + # Budget at least 200 to 300 MB per optimized program version. pvcSize: 50Gi # The artifact server also serves SQL program validation (a JVM); the CPU # limit lets the JVM size its heap. @@ -173,20 +173,20 @@ compilerAutoscaling: ### Latency Expectations -A compilation submitted while the compiler workers are scaled to zero waits for the full cold start: the autoscaler poll (up to `pollIntervalSeconds`), node provisioning if the cluster must add compiler nodes, image pull, and compiler server startup including precompiled dependency extraction. The pipeline shows `Pending` during this time; that is expected, not a stall. Compilations submitted while workers are already up behave exactly as without autoscaling. +A compilation submitted while the compiler workers are scaled to zero waits for the full cold start: the autoscaler poll (up to `pollIntervalSeconds`), node provisioning if the cluster must add compiler nodes, image pull, and compiler server startup including precompiled dependency extraction. The pipeline shows `Pending` during this time; the cold start usually adds 1 to 3 minutes of latency to the compilation, more if node provisioning is needed. Compilations submitted while workers are already up behave exactly as without autoscaling. Starting or restarting a pipeline whose binary is already compiled does not wake the compiler workers; the artifact server serves the stored binary directly. ### Enabling on an Existing Installation -Toggling the feature changes the StatefulSet `podManagementPolicy`, an immutable field. A pre-upgrade hook (`compilerAutoscaling.kubectlImage`) handles that automatically: on the one upgrade that flips the policy it deletes the compiler StatefulSet, and the same upgrade recreates it with the new policy while the retained per-replica volumes re-attach. The hook renders only on such transition upgrades; steady-state upgrades and installations that never toggle the feature run no hook at all. The hook image must be reachable from your cluster (mirror it for air-gapped installations). +Toggling the feature changes the StatefulSet `podManagementPolicy`, an immutable field. A pre-upgrade hook (`compilerAutoscaling.kubectlImage`) handles that automatically: on the one upgrade that flips the policy it deletes the compiler StatefulSet, and the same upgrade recreates it with the new policy while the retained per-replica volumes re-attach. The hook renders only on such transition upgrades; steady-state upgrades and installations that never toggle the feature run no hook at all. The hook runs `kubectl` from the image configured in `compilerAutoscaling.kubectlImage`, which must be pullable from your cluster. For air-gapped installations, copy the image into your registry (for example `crane copy docker.io/rancher/kubectl:v1.33.13 registry.example.com/kubectl:v1.33.13`) and set `compilerAutoscaling.kubectlImage` to the mirrored reference. -1. Upgrade to images that support autoscaling, with `compilerAutoscaling.enabled` still `false`. Older images do not understand the new flags and would crash loop. -2. Run the enabling upgrade with `compilerAutoscaling.enabled=true` and `compilerAutoscaling.artifactServer.seedFromCompilerServer0=true`. While the seed setting is true the chart omits the compiler StatefulSet and frees its ReadWriteOnce `compiler-storage--compiler-server-0` volume; the artifact server then copies the compiled binaries from that volume into the artifact store, so existing pipelines keep their binaries. No compilation runs during this window. +1. Upgrade to images that support autoscaling, with `compilerAutoscaling.enabled` still `false`. +2. Run the enabling upgrade with `compilerAutoscaling.enabled=true` and `compilerAutoscaling.artifactServer.seedFromCompilerServer0=true`. While the seed setting is true the chart omits the compiler StatefulSet and frees its ReadWriteOnce `compiler-storage--compiler-server-0` volume; the artifact server then copies the compiled binaries from that volume into the artifact store, so existing pipelines keep their binaries. 3. Wait until the artifact server pod is `Running` (the copy happens in its init container). 4. Run a follow-up upgrade with `seedFromCompilerServer0` back to `false`. This recreates the compiler StatefulSet and detaches the old volume from the artifact server. -To enable without preserving existing binaries, skip the seeding: a single upgrade with `compilerAutoscaling.enabled=true` suffices. The artifact store then starts empty: stopped pipelines recompile on their next start, but a running pipeline whose pod restarts cannot fetch its binary until its program is recompiled (stop and start it once). Stop running pipelines first if that is not acceptable. +Setting `seedFromCompilerServer0=true` is optional: skipping it means a single upgrade with `compilerAutoscaling.enabled=true` suffices, and the existing binaries are recompiled instead of carried over. The artifact store starts empty, stopped pipelines recompile on their next start, and a running pipeline whose pod restarts cannot fetch its binary until its program is recompiled (stop and start it once). Stop running pipelines first if that is not acceptable. Fresh installations need no procedure: set `compilerAutoscaling.enabled=true` at install time. @@ -205,20 +205,36 @@ To preserve the binaries (required if pipelines are running and must survive pod kubectl scale deployment -compiler-artifact-server -n --replicas=0 ``` -2. Run a one-shot pod that mounts both the `-compiler-artifact-server` and the `compiler-storage--compiler-server-0` PVCs and copies `rust-compilation/pipeline-binaries` across (the mirror image of the seed init container). -3. Run `helm upgrade` with `compilerAutoscaling.enabled=false` and verify the replica count as above. +2. Run a one-shot pod that mounts the `-compiler-artifact-server` PVC (source) and the `compiler-storage--compiler-server-0` PVC (target) and copies the `rust-compilation/pipeline-binaries` directory from source to target. Both paths are relative to the volume root: + + ```yaml + apiVersion: v1 + kind: Pod + metadata: + name: binary-restore + spec: + restartPolicy: Never + containers: + - name: copy + image: busybox:1.37.0 + command: ["sh", "-c", "mkdir -p /target/rust-compilation/pipeline-binaries && cp -a /source/rust-compilation/pipeline-binaries/. /target/rust-compilation/pipeline-binaries/"] + volumeMounts: + - {name: source, mountPath: /source, readOnly: true} + - {name: target, mountPath: /target} + volumes: + - {name: source, persistentVolumeClaim: {claimName: -compiler-artifact-server}} + - {name: target, persistentVolumeClaim: {claimName: compiler-storage--compiler-server-0}} + ``` -If the runner starts with autoscaling disabled and finds the compiler StatefulSet scaled to zero (for example after a disable that raced the old autoscaler), it patches the StatefulSet to 1 replica so compilation never stays wedged; the next `helm upgrade` restores the configured count. + Wait for the pod to reach `Succeeded`, then delete it. +3. Run `helm upgrade` with `compilerAutoscaling.enabled=false` and verify the replica count as above. -:::warning -Avoid `helm upgrade --force` while autoscaling is enabled: it replaces the StatefulSet, which resets `spec.replicas` and restarts compiler pods; the autoscaler restores the correct count within one poll interval, but in-flight compilations restart. Do not `helm rollback` across the enable boundary either: the rollback tries to revert the immutable `podManagementPolicy` field and fails on the StatefulSet, leaving the release half rolled back. To return to a pre-autoscaling revision, follow the Disabling procedure instead. -::: ### Autoscaling Troubleshooting - **`/cluster_healthz` reports `scaled_to_zero`:** -While the compiler workers are parked at zero, the health endpoint reports the compiler section as healthy with a `"scaled_to_zero": true` marker. This is the intended idle state, not a failure. +While the compiler workers are parked at zero, the health endpoint reports the compiler section as healthy with a `"scaled_to_zero": true` marker. - **`/cluster_healthz` reports the compiler not ready during scale-up:** @@ -230,7 +246,7 @@ Compiler pods that observe a replica change exit with `SCALING DETECTED` and res - **`/cluster_healthz` reports the compiler unhealthy with a storage message:** -The compiler health check fails once the binary store filesystem is 95% full, before uploads start failing with `No space left on device`. Grow the artifact server PVC (the storage class must support volume expansion) or delete unused pipelines so the garbage collector reclaims their binaries. Budget roughly 200 to 300 MB per optimized program version when sizing `compilerAutoscaling.artifactServer.pvcSize`. +The compiler health check fails once the binary store filesystem is 95% full, before uploads start failing with `No space left on device`. Grow the artifact server PVC (the storage class must support volume expansion) or delete unused pipelines so the garbage collector reclaims their binaries. Budget at least 200 to 300 MB per optimized program version when sizing `compilerAutoscaling.artifactServer.pvcSize`. - **Compilers never scale down:** @@ -256,7 +272,7 @@ Upload failures fall into three classes: - Permanent: surface immediately as `SystemError` with the underlying cause and skip the retry budget: an HTTP 4xx rejection (for example a proxy body-size limit), or HTTP 507 when the binary store volume is full (`Insufficient storage on the binary store`). - Retry budget exhausted: a retryable failure that outlives the retry budget also ends in `SystemError`. -After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. Check `-compiler-server-0` health via the `/cluster_healthz` endpoint; if your upgrades take long, raise the upload retry settings so the upload target has time to come back up before the retry budget runs out. +After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. Check `-compiler-server-0` health via the `/cluster_healthz` endpoint. - **error: process didn't exit successfully: `sccache .. rustc -vV`:** From 9319daf8cf473299f283ac715ac2f42765b3f9f1 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Tue, 4 Aug 2026 16:37:49 -0700 Subject: [PATCH 10/11] compiler: address review comments on upload and health check Rename the /healthz storage probe from '?deep=true' to '?check_storage=true', which says what it checks instead of naming a variant. Treat EROFS like ENOSPC: both mean the binary store cannot accept writes and only an operator can fix it, so uploads fail fast with 507 rather than burning the retry budget. 'unwritable_store_cause' now returns the cause plus the resolving action, and the 507 message names both and links the operator documentation. Extend the Out-of-storage Errors guide with a compiler binary store section, since the existing text covered only the per-pipeline volume. Build the temp upload name with 'with_added_extension', and assert the errno-to-ErrorKind mapping with 'libc::ENOSPC'/'libc::EROFS' instead of a hardcoded 28. Signed-off-by: Gerd Zellweger --- Cargo.lock | 1 + crates/pipeline-manager/Cargo.toml | 1 + .../pipeline-manager/src/cluster_monitor.rs | 4 +- crates/pipeline-manager/src/compiler/main.rs | 187 ++++++++++++------ .../src/compiler/rust_compiler.rs | 4 +- .../enterprise/parallel-compilation.md | 2 +- docs.feldera.com/docs/operations/guide.md | 24 +++ 7 files changed, 159 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 746eea100f8..c32393fc905 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8927,6 +8927,7 @@ dependencies = [ "indoc", "itertools 0.14.0", "jsonwebtoken", + "libc", "nix 0.29.0", "openssl", "pg-client-config", diff --git a/crates/pipeline-manager/Cargo.toml b/crates/pipeline-manager/Cargo.toml index 71fb96d8ba9..93a3c467e9b 100644 --- a/crates/pipeline-manager/Cargo.toml +++ b/crates/pipeline-manager/Cargo.toml @@ -151,6 +151,7 @@ wiremock = { workspace = true } feldera-types = { workspace = true, features = ["testing"] } itertools = { workspace = true } tokio = { workspace = true, features = ["sync"] } +libc = { workspace = true } # For: asserting errno-to-ErrorKind mapping [package.metadata.cargo-machete] ignored = ["static-files", "compare", "tikv-jemallocator"] diff --git a/crates/pipeline-manager/src/cluster_monitor.rs b/crates/pipeline-manager/src/cluster_monitor.rs index c81c93eab07..0ab9205f85f 100644 --- a/crates/pipeline-manager/src/cluster_monitor.rs +++ b/crates/pipeline-manager/src/cluster_monitor.rs @@ -72,10 +72,10 @@ pub async fn cluster_monitor( "{protocol}://{}:{}/healthz", common_config.api_host, common_config.api_port ); - // The deep variant additionally fails on storage pressure of the compiler + // `check_storage` additionally fails on storage pressure of the compiler // working directory, which would make binary uploads fail with ENOSPC. let compiler_url = format!( - "{protocol}://{}:{}/healthz?deep=true", + "{protocol}://{}:{}/healthz?check_storage=true", common_config.compiler_host, common_config.compiler_port ); let runner_url = format!( diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index 0a9f9594bd7..18fb54b5bee 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -367,10 +367,10 @@ async fn upload_binary( let total_size = match save_file(&target_file_path, payload, &expected_integrity_checksum).await { Ok(total_size) => total_size, - Err(error) if is_out_of_storage_error(&error) => { - return Ok(insufficient_storage_response(&error)); - } - Err(error) => return Err(error), + Err(error) => match unwritable_store_cause(&error) { + Some(cause) => return Ok(insufficient_storage_response(cause, &error)), + None => return Err(error), + }, }; info!( @@ -469,10 +469,10 @@ async fn upload_program_info( let total_size = match save_file(&target_file_path, payload, &expected_integrity_checksum).await { Ok(total_size) => total_size, - Err(error) if is_out_of_storage_error(&error) => { - return Ok(insufficient_storage_response(&error)); - } - Err(error) => return Err(error), + Err(error) => match unwritable_store_cause(&error) { + Some(cause) => return Ok(insufficient_storage_response(cause, &error)), + None => return Err(error), + }, }; info!( @@ -526,12 +526,7 @@ async fn save_file( payload: impl Stream> + Unpin, expected_integrity_checksum: &str, ) -> Result { - let target_file_name = target_file_path - .file_name() - .expect("target_file_path must name a file") - .to_string_lossy(); - let temp_file_path = - target_file_path.with_file_name(format!("{target_file_name}.tmp-{}", Uuid::now_v7())); + let temp_file_path = target_file_path.with_added_extension(format!("tmp-{}", Uuid::now_v7())); let write_result = stream_to_file_and_verify(&temp_file_path, payload, expected_integrity_checksum).await; @@ -652,24 +647,46 @@ async fn stream_to_file_and_verify( Ok(total_size) } -/// Whether the error is an out-of-storage I/O error (ENOSPC). -fn is_out_of_storage_error(error: &ManagerError) -> bool { +/// Operator documentation on resolving a full or read-only storage volume. +const OUT_OF_STORAGE_DOCS_URL: &str = + "https://docs.feldera.com/operations/guide/#out-of-storage-errors"; + +/// Why the binary store cannot accept writes, phrased as the cause plus the +/// action that resolves it, or `None` when the error is something else. Only +/// ENOSPC and EROFS qualify: both need an operator, so an upload that hits +/// either must fail rather than retry. +fn unwritable_store_cause(error: &ManagerError) -> Option<&'static str> { let ManagerError::CommonError { common_error: CommonError::IoError { io_error, .. }, } = error else { - return false; + return None; }; - io_error.kind() == std::io::ErrorKind::StorageFull + match io_error.kind() { + std::io::ErrorKind::StorageFull => Some( + "the storage volume is full; an operator must grow it or delete unused pipelines \ + to reclaim space", + ), + // EROFS in practice means the underlying disk failed and the kernel + // remounted the filesystem read-only. + std::io::ErrorKind::ReadOnlyFilesystem => Some( + "the storage volume is read-only, which usually means the underlying disk failed; \ + an operator must repair or replace it", + ), + _ => None, + } } -/// 507 Insufficient Storage response for an upload that failed with ENOSPC. -/// The distinct status lets workers fail the compilation fast with a -/// user-visible error instead of burning their retry budget on a volume that -/// only an operator can grow. -fn insufficient_storage_response(error: &ManagerError) -> HttpResponse { +/// 507 Insufficient Storage response naming the cause and how to resolve it. +/// The distinct status lets workers fail the compilation fast with this +/// message instead of burning their retry budget on a volume that only an +/// operator can grow or repair. +fn insufficient_storage_response(cause: &str, error: &ManagerError) -> HttpResponse { HttpResponse::InsufficientStorage().json(serde_json::json!({ - "message": format!("Insufficient storage on the binary store: {error}"), + "message": format!( + "Unable to write to the binary store: {cause}. \ + See {OUT_OF_STORAGE_DOCS_URL}. Underlying error: {error}" + ), })) } @@ -686,13 +703,13 @@ async fn remove_temp_upload_file(temp_file_path: &Path) { } } -/// Fraction of the working-directory filesystem above which the deep health -/// check reports storage pressure: binary uploads are about to fail with -/// ENOSPC and an operator must grow the volume or reclaim space. +/// Fraction of the working-directory filesystem above which `/healthz` reports +/// storage pressure: binary uploads are about to fail with ENOSPC and an +/// operator must grow the volume or reclaim space. const STORAGE_PRESSURE_THRESHOLD: f64 = 0.95; -/// Message for the deep health check when the working-directory filesystem is -/// at or above `used_fraction_threshold`, `None` when it is below. +/// Message reported when the working-directory filesystem is at or above +/// `used_fraction_threshold`, `None` when it is below. fn storage_pressure_message( disk_space: &DiskSpace, used_fraction_threshold: f64, @@ -712,29 +729,33 @@ fn storage_pressure_message( /// Query parameters of the `/healthz` endpoint. #[derive(serde::Deserialize)] struct HealthzQuery { + /// Also report storage pressure of the working-directory filesystem. #[serde(default)] - deep: bool, + check_storage: bool, } /// Health check which returns success if it is able to reach the database. -/// With `?deep=true` it additionally fails on storage pressure of -/// the working-directory filesystem. Kubernetes probes use the shallow -/// variant: a full disk must not restart the pod, which still serves already -/// compiled binaries; the cluster monitor uses the deep variant so -/// /v0/cluster_healthz surfaces the condition to operators. +/// With `?check_storage=true` it also fails when the working-directory +/// filesystem is nearly full. +/// +/// Kubernetes probes must omit the parameter, because restarting the pod +/// cannot free disk space and the pod still serves already compiled binaries. +/// The cluster monitor passes it so that /v0/cluster_healthz surfaces the +/// condition to operators, who can act on it. #[get("/healthz")] async fn healthz( probe: web::Data>>, config: web::Data, query: web::Query, ) -> Result { - if query.deep { - if let Some(disk_space) = DiskSpace::new_from_path(&config.working_dir()) { - if let Some(message) = storage_pressure_message(&disk_space, STORAGE_PRESSURE_THRESHOLD) - { - return Ok(HttpResponse::ServiceUnavailable() - .json(serde_json::json!({ "status": message }))); - } + if query.check_storage { + let pressure = DiskSpace::new_from_path(&config.working_dir()).and_then(|disk_space| { + storage_pressure_message(&disk_space, STORAGE_PRESSURE_THRESHOLD) + }); + if let Some(message) = pressure { + return Ok( + HttpResponse::ServiceUnavailable().json(serde_json::json!({ "status": message })) + ); } } Ok(probe.lock().await.as_http_response()) @@ -1336,28 +1357,76 @@ mod test { ); } - /// ENOSPC I/O errors are recognized as out-of-storage; other errors are not. + /// A full or read-only store yields a cause naming the fix; other I/O + /// errors yield none. #[test] - fn out_of_storage_error_detection() { - let enospc = ManagerError::from(crate::common_error::CommonError::io_error( - "writing".to_string(), - std::io::Error::from_raw_os_error(28), - )); - assert!(super::is_out_of_storage_error(&enospc)); - let storage_full = ManagerError::from(crate::common_error::CommonError::io_error( - "writing".to_string(), - std::io::Error::new(std::io::ErrorKind::StorageFull, "full"), - )); - assert!(super::is_out_of_storage_error(&storage_full)); - let other_io = ManagerError::from(crate::common_error::CommonError::io_error( + fn unwritable_store_cause_detection() { + let io_error = |kind| { + ManagerError::from(crate::common_error::CommonError::io_error( + "writing".to_string(), + std::io::Error::new(kind, "test"), + )) + }; + let full = super::unwritable_store_cause(&io_error(std::io::ErrorKind::StorageFull)) + .expect("a full volume is unwritable"); + assert!(full.contains("full"), "unexpected cause: {full}"); + let read_only = + super::unwritable_store_cause(&io_error(std::io::ErrorKind::ReadOnlyFilesystem)) + .expect("a read-only volume is unwritable"); + assert!( + read_only.contains("read-only"), + "unexpected cause: {read_only}" + ); + assert!( + super::unwritable_store_cause(&io_error(std::io::ErrorKind::PermissionDenied)) + .is_none() + ); + } + + /// The 507 response names the cause and links the operator documentation. + #[actix_web::test] + async fn insufficient_storage_response_is_actionable() { + let error = ManagerError::from(crate::common_error::CommonError::io_error( "writing".to_string(), - std::io::Error::new(std::io::ErrorKind::PermissionDenied, "denied"), + std::io::Error::new(std::io::ErrorKind::StorageFull, "test"), )); - assert!(!super::is_out_of_storage_error(&other_io)); + let cause = super::unwritable_store_cause(&error).unwrap(); + let response = super::insufficient_storage_response(cause, &error); + assert_eq!( + response.status(), + actix_web::http::StatusCode::INSUFFICIENT_STORAGE + ); + let body = actix_web::body::to_bytes(response.into_body()) + .await + .unwrap(); + let message = serde_json::from_slice::(&body).unwrap()["message"] + .as_str() + .unwrap() + .to_string(); + assert!( + message.starts_with("Unable to write to the binary store:"), + "unexpected message: {message}" + ); + assert!(message.contains(super::OUT_OF_STORAGE_DOCS_URL)); + } + + /// The kernel errnos the upload path actually sees map to the + /// `ErrorKind`s `unwritable_store_cause` matches on. + #[cfg(unix)] + #[test] + fn unwritable_store_errnos_map_to_expected_error_kinds() { + assert_eq!( + std::io::Error::from_raw_os_error(libc::ENOSPC).kind(), + std::io::ErrorKind::StorageFull + ); + assert_eq!( + std::io::Error::from_raw_os_error(libc::EROFS).kind(), + std::io::ErrorKind::ReadOnlyFilesystem + ); } - /// The deep health check reports storage pressure at or above the - /// threshold and stays silent below it. + /// Storage pressure is reported at or above the threshold and stays silent + /// below it. #[test] fn storage_pressure_threshold() { let disk_space = |used_fraction: f64| crate::compiler::util::DiskSpace { diff --git a/crates/pipeline-manager/src/compiler/rust_compiler.rs b/crates/pipeline-manager/src/compiler/rust_compiler.rs index ef4871b65e4..00519b0ee8c 100644 --- a/crates/pipeline-manager/src/compiler/rust_compiler.rs +++ b/crates/pipeline-manager/src/compiler/rust_compiler.rs @@ -732,8 +732,8 @@ async fn upload_program_info_to_endpoint_with_retries( /// Returns true when the upload response status indicates a permanent /// rejection that no retry can fix: a 4xx other than 408 and 429, or 507 -/// Insufficient Storage (the binary store is full and only an operator can -/// grow it, so retrying just delays the user-visible error). +/// Insufficient Storage (the binary store cannot accept writes and only an +/// operator can fix that, so retrying just delays the user-visible error). fn is_permanent_upload_rejection(status: reqwest::StatusCode) -> bool { if status == reqwest::StatusCode::INSUFFICIENT_STORAGE { return true; diff --git a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md index 6ecea10f2c7..c673434723b 100644 --- a/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md +++ b/docs.feldera.com/docs/get-started/enterprise/parallel-compilation.md @@ -269,7 +269,7 @@ If a pipeline is assigned to a worker pod that is not yet running or is unhealth Upload failures fall into three classes: - Transient (network errors, HTTP 5xx): retried with exponential backoff inside the compilation attempt; the default retry settings absorb roughly half an hour of outage, for example a restart of the upload target during an upgrade. - - Permanent: surface immediately as `SystemError` with the underlying cause and skip the retry budget: an HTTP 4xx rejection (for example a proxy body-size limit), or HTTP 507 when the binary store volume is full (`Insufficient storage on the binary store`). + - Permanent: surface immediately as `SystemError` with the underlying cause and skip the retry budget: an HTTP 4xx rejection (for example a proxy body-size limit), or HTTP 507 when the binary store volume is full or has remounted itself read-only after a storage failure (`Unable to write to the binary store`, see [Out-of-storage Errors](/operations/guide#out-of-storage-errors)). - Retry budget exhausted: a retryable failure that outlives the retry budget also ends in `SystemError`. After fixing the cause (for example growing the artifact store PVC), recompile affected pipelines by editing or re-saving their program. Check `-compiler-server-0` health via the `/cluster_healthz` endpoint. diff --git a/docs.feldera.com/docs/operations/guide.md b/docs.feldera.com/docs/operations/guide.md index 0b15740dce8..907156d96fe 100644 --- a/docs.feldera.com/docs/operations/guide.md +++ b/docs.feldera.com/docs/operations/guide.md @@ -119,6 +119,30 @@ explicitly request larger volumes for each pipeline: } ``` +#### Compiler binary store + +**Error**: a pipeline fails to compile with: +``` +Unable to write to the binary store: the storage volume is full; an operator +must grow it or delete unused pipelines to reclaim space +``` + +The compiler stores every compiled pipeline binary on its own volume, separate +from the pipeline storage above. Compilation fails immediately rather than +retrying, because only an operator can resolve it. + +**Solution**: grow the compiler volume or reclaim space. Budget at least 200 to +300 MB per compiled program version. Deleting unused pipelines lets the +compiler's garbage collector reclaim their binaries. The compiler health check +reports this condition through `/cluster_healthz` once the volume is 95% full, +before uploads start failing. + +If the message instead says the volume is read-only, the kernel remounted the +filesystem after a disk failure; repair or replace the underlying disk. + +After resolving the cause, recompile affected pipelines by editing or re-saving +their program. + ### Kubernetes evictions **Error**: the pipeline becomes `UNAVAILABLE` with no errors in the logs. From 1f2e05298a8e6df55565d286350f25a5c7027e4b Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Tue, 4 Aug 2026 16:54:18 -0700 Subject: [PATCH 11/11] pipeline-manager: migrate to edition 2024 Enables let chains, which collapse 33 nested 'if' blocks across the crate and the storage check in the compiler health handler. Two changes are more than mechanical: - 'build_app' returns 'impl Trait' that edition 2024 would make capture its argument lifetimes, forcing every caller's config to live for 'static. 'use<>' keeps the returned app capturing nothing, which holds because it owns everything it needs. - 'emit_logs' set the environment it had already been handed: 'run_child' passes RUST_LOG, NO_COLOR and the log format to the child, and 'use_json_log_format' reads FELDERA_LOG_JSON=0 as false. Dropping the dead 'set_var' calls avoids the edition 2024 unsafety rather than wrapping it, which matters because 'set_var' races in a threaded test harness. The rest is rustfmt style edition 2024 reordering imports, plus two clippy fixes in test code. Signed-off-by: Gerd Zellweger --- crates/pipeline-manager/Cargo.toml | 2 +- crates/pipeline-manager/build.rs | 2 +- crates/pipeline-manager/src/api/demo.rs | 2 +- .../src/api/endpoints/api_key.rs | 3 +- .../src/api/endpoints/cluster.rs | 2 +- .../src/api/endpoints/config.rs | 3 +- .../src/api/endpoints/metrics.rs | 23 +-- .../src/api/endpoints/oidc_trust.rs | 3 +- .../src/api/endpoints/pipeline_interaction.rs | 5 +- .../pipeline_interaction/support_bundle.rs | 17 +- .../src/api/endpoints/pipeline_management.rs | 18 +- .../pipeline_management/pipeline_events.rs | 3 +- .../src/api/endpoints/tenant.rs | 3 +- crates/pipeline-manager/src/api/error.rs | 2 +- crates/pipeline-manager/src/api/examples.rs | 4 +- crates/pipeline-manager/src/api/main.rs | 37 ++-- crates/pipeline-manager/src/api/rbac.rs | 6 +- .../src/api/support_data_collector.rs | 6 +- crates/pipeline-manager/src/auth.rs | 116 ++++++------ .../src/bin/pipeline-manager.rs | 8 +- .../pipeline-manager/src/cluster_monitor.rs | 18 +- crates/pipeline-manager/src/common_error.rs | 4 +- crates/pipeline-manager/src/compiler/error.rs | 2 +- crates/pipeline-manager/src/compiler/main.rs | 74 ++++---- .../src/compiler/rust_compiler.rs | 119 ++++++++----- .../src/compiler/sql_compiler.rs | 77 ++++---- crates/pipeline-manager/src/compiler/test.rs | 2 +- crates/pipeline-manager/src/compiler/util.rs | 30 ++-- crates/pipeline-manager/src/config.rs | 14 +- crates/pipeline-manager/src/db/error.rs | 70 ++++++-- .../pipeline-manager/src/db/listen_table.rs | 14 +- .../src/db/operations/oidc_trust.rs | 12 +- .../src/db/operations/pipeline.rs | 40 ++--- .../src/db/operations/pipeline_monitor.rs | 6 +- .../src/db/operations/pipeline_parsing.rs | 12 +- .../src/db/operations/utils.rs | 35 ++-- crates/pipeline-manager/src/db/test.rs | 166 ++++++++++-------- .../src/db/types/combined_status.rs | 8 +- .../pipeline-manager/src/db/types/program.rs | 2 +- crates/pipeline-manager/src/db/types/utils.rs | 53 ++++-- crates/pipeline-manager/src/error.rs | 2 +- crates/pipeline-manager/src/events_cleaner.rs | 4 +- crates/pipeline-manager/src/lib.rs | 4 +- crates/pipeline-manager/src/license.rs | 8 +- crates/pipeline-manager/src/logging.rs | 2 +- crates/pipeline-manager/src/oidc/fetch.rs | 4 +- .../pipeline-manager/src/oidc/trust_name.rs | 4 +- crates/pipeline-manager/src/runner/error.rs | 17 +- .../src/runner/interaction.rs | 4 +- .../src/runner/local_runner.rs | 28 +-- crates/pipeline-manager/src/runner/main.rs | 48 ++--- .../src/runner/pipeline_automata.rs | 40 +++-- .../src/runner/pipeline_logs.rs | 27 ++- crates/pipeline-manager/tests/logging_demo.rs | 16 +- 54 files changed, 694 insertions(+), 537 deletions(-) diff --git a/crates/pipeline-manager/Cargo.toml b/crates/pipeline-manager/Cargo.toml index 93a3c467e9b..fbe1bfd1323 100644 --- a/crates/pipeline-manager/Cargo.toml +++ b/crates/pipeline-manager/Cargo.toml @@ -5,7 +5,7 @@ keywords = ["DBSP", "streaming", "analytics", "database", "webui"] categories = ["database", "gui"] publish = false -edition = { workspace = true } +edition = "2024" version = { workspace = true } homepage = { workspace = true } repository = { workspace = true } diff --git a/crates/pipeline-manager/build.rs b/crates/pipeline-manager/build.rs index a2356761c1c..b79b67fa7cb 100644 --- a/crates/pipeline-manager/build.rs +++ b/crates/pipeline-manager/build.rs @@ -1,5 +1,5 @@ use change_detection::ChangeDetection; -use static_files::{resource_dir, NpmBuild}; +use static_files::{NpmBuild, resource_dir}; use std::env; use std::path::{Path, PathBuf}; use vergen_gitcl::*; diff --git a/crates/pipeline-manager/src/api/demo.rs b/crates/pipeline-manager/src/api/demo.rs index f6afb8e5b64..2fb135c44b5 100644 --- a/crates/pipeline-manager/src/api/demo.rs +++ b/crates/pipeline-manager/src/api/demo.rs @@ -250,7 +250,7 @@ pub fn read_demos_from_directories(demos_dir: &Vec) -> Vec { #[cfg(test)] mod test { - use super::{parse_demo, read_demos_from_directories, Demo, DemoError}; + use super::{Demo, DemoError, parse_demo, read_demos_from_directories}; use std::fs; use std::fs::File; use std::io::Write; diff --git a/crates/pipeline-manager/src/api/endpoints/api_key.rs b/crates/pipeline-manager/src/api/endpoints/api_key.rs index 3f09a02766e..5c888e0492b 100644 --- a/crates/pipeline-manager/src/api/endpoints/api_key.rs +++ b/crates/pipeline-manager/src/api/endpoints/api_key.rs @@ -9,11 +9,10 @@ use crate::db::types::tenant::TenantId; use crate::error::ManagerError; use crate::{api::examples, db::storage::Storage}; use actix_web::{ - delete, get, + HttpRequest, HttpResponse, delete, get, http::header::{CacheControl, CacheDirective}, post, web::{self, Data as WebData, ReqData}, - HttpRequest, HttpResponse, }; use serde::{Deserialize, Serialize}; use tracing::info; diff --git a/crates/pipeline-manager/src/api/endpoints/cluster.rs b/crates/pipeline-manager/src/api/endpoints/cluster.rs index 326b17b2b36..d12001478af 100644 --- a/crates/pipeline-manager/src/api/endpoints/cluster.rs +++ b/crates/pipeline-manager/src/api/endpoints/cluster.rs @@ -7,7 +7,7 @@ use crate::{ error::ManagerError, }; use actix_web::http::header::{CacheControl, CacheDirective}; -use actix_web::{get, web, web::Data as WebData, HttpResponse}; +use actix_web::{HttpResponse, get, web, web::Data as WebData}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use std::str::FromStr; diff --git a/crates/pipeline-manager/src/api/endpoints/config.rs b/crates/pipeline-manager/src/api/endpoints/config.rs index 4d82cb40d87..1835726701f 100644 --- a/crates/pipeline-manager/src/api/endpoints/config.rs +++ b/crates/pipeline-manager/src/api/endpoints/config.rs @@ -1,8 +1,7 @@ // Configuration API to retrieve the current authentication configuration and list of demos use actix_web::{ - get, + HttpRequest, HttpResponse, get, web::{Data as WebData, ReqData}, - HttpRequest, HttpResponse, }; use feldera_cloud1_client::license::DisplaySchedule; use serde::Serialize; diff --git a/crates/pipeline-manager/src/api/endpoints/metrics.rs b/crates/pipeline-manager/src/api/endpoints/metrics.rs index bfc53bbd0d1..fe38689f9e1 100644 --- a/crates/pipeline-manager/src/api/endpoints/metrics.rs +++ b/crates/pipeline-manager/src/api/endpoints/metrics.rs @@ -2,10 +2,9 @@ use crate::api::main::ServerState; use crate::db::{storage::Storage as _, types::tenant::TenantId}; use crate::error::ManagerError; use actix_web::{ - get, + HttpResponse, get, http::Method, web::{Data as WebData, ReqData}, - HttpResponse, }; use awc::body::MessageBody as _; use feldera_types::runtime_status::RuntimeStatus; @@ -45,10 +44,9 @@ pub(crate) async fn get_metrics( let mut result = Vec::new(); for pipeline in pipelines { - if pipeline.deployment_runtime_status == Some(RuntimeStatus::Running) - || pipeline.deployment_runtime_status == Some(RuntimeStatus::Paused) - { - if let Ok(res) = state + if (pipeline.deployment_runtime_status == Some(RuntimeStatus::Running) + || pipeline.deployment_runtime_status == Some(RuntimeStatus::Paused)) + && let Ok(res) = state .runner .forward_http_request_to_pipeline_by_name( client.as_ref(), @@ -61,14 +59,11 @@ pub(crate) async fn get_metrics( None, ) .await - { - if res.status().is_success() { - if let Ok(bytes) = res.into_body().try_into_bytes() { - result.extend(bytes); - result.push(NEWLINE); - } - } - } + && res.status().is_success() + && let Ok(bytes) = res.into_body().try_into_bytes() + { + result.extend(bytes); + result.push(NEWLINE); } } diff --git a/crates/pipeline-manager/src/api/endpoints/oidc_trust.rs b/crates/pipeline-manager/src/api/endpoints/oidc_trust.rs index e5ffbef7ab6..3fc44ea82c1 100644 --- a/crates/pipeline-manager/src/api/endpoints/oidc_trust.rs +++ b/crates/pipeline-manager/src/api/endpoints/oidc_trust.rs @@ -19,11 +19,10 @@ use crate::db::types::role::{MemberRole, Role}; use crate::db::types::tenant::TenantId; use crate::error::ManagerError; use actix_web::{ - delete, get, + HttpRequest, HttpResponse, delete, get, http::header::{CacheControl, CacheDirective}, post, web::{self, Data as WebData, ReqData}, - HttpRequest, HttpResponse, }; use serde::{Deserialize, Serialize}; use tracing::info; diff --git a/crates/pipeline-manager/src/api/endpoints/pipeline_interaction.rs b/crates/pipeline-manager/src/api/endpoints/pipeline_interaction.rs index e301a571304..df9a7fa3918 100644 --- a/crates/pipeline-manager/src/api/endpoints/pipeline_interaction.rs +++ b/crates/pipeline-manager/src/api/endpoints/pipeline_interaction.rs @@ -9,11 +9,10 @@ use crate::db::types::tenant::TenantId; use crate::error::ManagerError; use actix_http::StatusCode; use actix_web::{ - get, - http::{header, Method}, + HttpRequest, HttpResponse, get, + http::{Method, header}, post, web::{self, Data as WebData, ReqData}, - HttpRequest, HttpResponse, }; #[allow(unused_imports)] use feldera_types::checkpoint::RemoteCheckpoint; diff --git a/crates/pipeline-manager/src/api/endpoints/pipeline_interaction/support_bundle.rs b/crates/pipeline-manager/src/api/endpoints/pipeline_interaction/support_bundle.rs index 6f82421fb29..243eb3bd796 100644 --- a/crates/pipeline-manager/src/api/endpoints/pipeline_interaction/support_bundle.rs +++ b/crates/pipeline-manager/src/api/endpoints/pipeline_interaction/support_bundle.rs @@ -9,14 +9,13 @@ use crate::api::main::ServerState; use crate::api::support_data_collector::{ CollectionSummary, SupportBundleData, SupportBundleParameters, }; -use crate::db::types::combined_status::{combine_since, CombinedDesiredStatus, CombinedStatus}; +use crate::db::types::combined_status::{CombinedDesiredStatus, CombinedStatus, combine_since}; use crate::db::types::pipeline::ExtendedPipelineDescrMonitoring; use crate::db::{storage::Storage, types::tenant::TenantId}; use crate::error::ManagerError; use actix_web::{ - get, + HttpResponse, get, web::{self, Data as WebData, ReqData}, - HttpResponse, }; use chrono::{DateTime, Utc}; use serde::Serialize; @@ -459,11 +458,13 @@ mod tests { assert_eq!(collections.len(), 2); for c in collections { assert!(dirs.contains(c["directory"].as_str().unwrap())); - assert!(c["collected"] - .as_array() - .unwrap() - .iter() - .any(|f| f == "pipeline_events.json")); + assert!( + c["collected"] + .as_array() + .unwrap() + .iter() + .any(|f| f == "pipeline_events.json") + ); } } diff --git a/crates/pipeline-manager/src/api/endpoints/pipeline_management.rs b/crates/pipeline-manager/src/api/endpoints/pipeline_management.rs index ea1616a3922..f06cdf2f356 100644 --- a/crates/pipeline-manager/src/api/endpoints/pipeline_management.rs +++ b/crates/pipeline-manager/src/api/endpoints/pipeline_management.rs @@ -7,7 +7,7 @@ use crate::compiler::{ProgramValidationRequest, ValidateProgramResponse}; use crate::config::CommonConfig; use crate::db::error::DBError; use crate::db::storage::Storage; -use crate::db::types::combined_status::{combine_since, CombinedDesiredStatus, CombinedStatus}; +use crate::db::types::combined_status::{CombinedDesiredStatus, CombinedStatus, combine_since}; use crate::db::types::pipeline::{ ClientMetadata, ExtendedPipelineDescr, ExtendedPipelineDescrMonitoring, PatchClientMetadata, PipelineDescr, PipelineId, @@ -24,17 +24,16 @@ use crate::has_unstable_feature; #[cfg(feature = "feldera-enterprise")] use actix_web::http::Method; use actix_web::{ - delete, get, + HttpResponse, delete, get, http::header::{CacheControl, CacheDirective}, patch, post, put, web::{self, Data as WebData, ReqData}, - HttpResponse, }; use chrono::{DateTime, Utc}; use feldera_types::adapter_stats::PipelineStatsErrorsResponse; use feldera_types::config::{InputEndpointConfig, OutputEndpointConfig, RuntimeConfig}; use feldera_types::error::ErrorResponse; -use feldera_types::pipeline_diff::{compute_pipeline_diff, PipelineDiff}; +use feldera_types::pipeline_diff::{PipelineDiff, compute_pipeline_diff}; use feldera_types::program_schema::ProgramSchema; use feldera_types::runtime_status::{ BootstrapConfig, BootstrapPolicy, ConnectorStats, RuntimeDesiredStatus, RuntimeStatus, @@ -878,12 +877,11 @@ async fn fetch_connector_error_stats( let details = backward_compatible_runtime_status_details( pipeline.deployment_runtime_status_details.clone(), ); - if let Some(value) = details { - if let Ok(details) = serde_json::from_value::(value) { - if let Some(connector_stats) = details.connector_stats { - return Some(connector_stats); - } - } + if let Some(value) = details + && let Ok(details) = serde_json::from_value::(value) + && let Some(connector_stats) = details.connector_stats + { + return Some(connector_stats); }; // Only forward the request if the pipeline is in a valid runtime status diff --git a/crates/pipeline-manager/src/api/endpoints/pipeline_management/pipeline_events.rs b/crates/pipeline-manager/src/api/endpoints/pipeline_management/pipeline_events.rs index 0d5d6385229..b8a1f76998b 100644 --- a/crates/pipeline-manager/src/api/endpoints/pipeline_management/pipeline_events.rs +++ b/crates/pipeline-manager/src/api/endpoints/pipeline_management/pipeline_events.rs @@ -10,10 +10,9 @@ use crate::db::types::storage::StorageStatus; use crate::db::types::tenant::TenantId; use crate::error::ManagerError; use actix_web::{ - get, + HttpResponse, get, http::header::{CacheControl, CacheDirective}, web::{self, Data as WebData, ReqData}, - HttpResponse, }; use chrono::{DateTime, Utc}; use feldera_types::error::ErrorResponse; diff --git a/crates/pipeline-manager/src/api/endpoints/tenant.rs b/crates/pipeline-manager/src/api/endpoints/tenant.rs index 24aae9a2f3b..5333077319e 100644 --- a/crates/pipeline-manager/src/api/endpoints/tenant.rs +++ b/crates/pipeline-manager/src/api/endpoints/tenant.rs @@ -15,11 +15,10 @@ use crate::db::types::tenant::TenantId; use crate::db::types::user::{TenantInfo, UserId}; use crate::error::ManagerError; use actix_web::{ - delete, get, + HttpRequest, HttpResponse, delete, get, http::header::{CacheControl, CacheDirective}, patch, post, put, web::{self, Data as WebData, ReqData}, - HttpRequest, HttpResponse, }; use serde::{Deserialize, Serialize}; use tracing::info; diff --git a/crates/pipeline-manager/src/api/error.rs b/crates/pipeline-manager/src/api/error.rs index 74ccb0add12..2ac7ddb4da6 100644 --- a/crates/pipeline-manager/src/api/error.rs +++ b/crates/pipeline-manager/src/api/error.rs @@ -1,5 +1,5 @@ use actix_web::{ - body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, + HttpResponse, HttpResponseBuilder, ResponseError, body::BoxBody, http::StatusCode, }; use feldera_types::error::{DetailedError, ErrorResponse}; use serde::Serialize; diff --git a/crates/pipeline-manager/src/api/examples.rs b/crates/pipeline-manager/src/api/examples.rs index 9a6362e18cf..94be63679b9 100644 --- a/crates/pipeline-manager/src/api/examples.rs +++ b/crates/pipeline-manager/src/api/examples.rs @@ -14,13 +14,13 @@ use crate::db::types::program::{CompilationProfile, ProgramConfig, ProgramError, use crate::db::types::resources_status::{ResourcesDesiredStatus, ResourcesStatus}; use crate::db::types::storage::StorageStatus; use crate::db::types::utils::{ - validate_program_config, validate_program_info, validate_runtime_config, PATTERN_KUBERNETES_LABEL_VALUE, PATTERN_KUBERNETES_LABEL_VALUE_DESCRIPTION, + validate_program_config, validate_program_info, validate_runtime_config, }; use crate::db::types::version::Version; use crate::runner::error::RunnerError; use crate::runner::interaction::{ - format_disconnected_error_message, format_timeout_error_message, RunnerInteraction, + RunnerInteraction, format_disconnected_error_message, format_timeout_error_message, }; use feldera_types::config::{DevTweaks, FtConfig, ResourceConfig, StorageOptions}; use feldera_types::runtime_status::{RuntimeStatusDetails, StorageStatusDetails}; diff --git a/crates/pipeline-manager/src/api/main.rs b/crates/pipeline-manager/src/api/main.rs index 8f8d33614f3..a05b3fd1d8d 100644 --- a/crates/pipeline-manager/src/api/main.rs +++ b/crates/pipeline-manager/src/api/main.rs @@ -1,4 +1,4 @@ -use crate::api::demo::{read_demos_from_directories, Demo}; +use crate::api::demo::{Demo, read_demos_from_directories}; use crate::api::endpoints; use crate::api::support_data_collector::SupportDataCollector; use crate::auth::{IssuerJwkCache, JwkCache}; @@ -9,17 +9,16 @@ use crate::error::ManagerError; use crate::license::LicenseCheck; use crate::runner::interaction::RunnerInteraction; use crate::unstable_features; -use actix_http::body::BoxBody; use actix_http::StatusCode; +use actix_http::body::BoxBody; +use actix_web::Scope; use actix_web::body::MessageBody; use actix_web::dev::{Service, ServiceResponse}; -use actix_web::http::{header, Method}; -use actix_web::Scope; +use actix_web::http::{Method, header}; use actix_web::{ - get, middleware, + App, HttpResponse, HttpServer, get, middleware, web::Data as WebData, web::{self}, - App, HttpResponse, HttpServer, }; use actix_web_httpauth::middleware::HttpAuthentication; use actix_web_static_files::ResourceFiles; @@ -29,11 +28,11 @@ use futures_util::FutureExt; use std::io::Write; use std::time::Duration; use std::{env, io, net::TcpListener, sync::Arc}; -use termbg::{theme, Theme}; +use termbg::{Theme, theme}; use tokio::signal; use tokio::sync::watch; use tokio::sync::{Mutex, RwLock}; -use tracing::{error, info, Level}; +use tracing::{Level, error, info}; use utoipa::openapi::security::{HttpAuthScheme, HttpBuilder, SecurityScheme}; use utoipa::{Modify, OpenApi}; use utoipa_swagger_ui::SwaggerUi; @@ -608,13 +607,16 @@ fn build_app( api_config: &ApiServerConfig, auth_configuration: &Option, ) -> App< + // `use<>`: the returned app owns everything it needs, so it must not + // capture the argument lifetimes that edition 2024 would capture by + // default. Callers pass short-lived config to an app that outlives it. impl actix_web::dev::ServiceFactory< actix_web::dev::ServiceRequest, Config = (), - Response = actix_web::dev::ServiceResponse, + Response = actix_web::dev::ServiceResponse>, Error = actix_web::Error, InitError = (), - >, + > + use<>, > { let cors = api_config.cors(); let app = App::new() @@ -1119,7 +1121,7 @@ Version: {} v{}{} }); #[cfg(unix)] { - use tokio::signal::unix::{signal as unix_signal, SignalKind}; + use tokio::signal::unix::{SignalKind, signal as unix_signal}; let mut term_stream = unix_signal(SignalKind::terminate()).expect("SIGTERM handler"); let server_handle_term = server.handle(); tokio::spawn(async move { @@ -1221,10 +1223,11 @@ mod tests { "public, max-age=86400", ); // Only the immutable branch emits ACAO/EXPIRES. - assert!(res - .headers() - .get(header::ACCESS_CONTROL_ALLOW_ORIGIN) - .is_none()); + assert!( + res.headers() + .get(header::ACCESS_CONTROL_ALLOW_ORIGIN) + .is_none() + ); assert!(res.headers().get(header::EXPIRES).is_none()); } @@ -1288,7 +1291,9 @@ mod tests { let res = test::call_service(&app, req).await; assert!( - res.headers().get(header::ACCESS_CONTROL_ALLOW_CREDENTIALS).is_none(), + res.headers() + .get(header::ACCESS_CONTROL_ALLOW_CREDENTIALS) + .is_none(), "static asset leaked Access-Control-Allow-Credentials — actix-cors regressed onto the static scope", ); if let Some(vary) = res.headers().get(header::VARY) { diff --git a/crates/pipeline-manager/src/api/rbac.rs b/crates/pipeline-manager/src/api/rbac.rs index 1fc1b11f53a..acf5b2c3e26 100644 --- a/crates/pipeline-manager/src/api/rbac.rs +++ b/crates/pipeline-manager/src/api/rbac.rs @@ -510,7 +510,7 @@ mod test { #[actix_web::test] async fn middleware_enforces_in_a_real_pipeline() { use actix_web::middleware::from_fn; - use actix_web::{test, web, App, HttpResponse}; + use actix_web::{App, HttpResponse, test, web}; use std::str::FromStr; // Installs a principal whose role comes from the `x-test-role` header, @@ -603,8 +603,8 @@ mod test { /// Paths outside `/v0` are unauthenticated (public scope) and not gated. fn openapi_v0_routes() -> Vec<(String, String)> { use crate::api::main::ApiDoc; - use utoipa::openapi::PathItemType; use utoipa::OpenApi; + use utoipa::openapi::PathItemType; let method = |t: &PathItemType| match t { PathItemType::Get => "GET", @@ -701,8 +701,8 @@ mod test { #[test] fn every_v0_operation_documents_its_min_role() { use crate::api::main::ApiDoc; - use utoipa::openapi::PathItemType; use utoipa::OpenApi; + use utoipa::openapi::PathItemType; let method = |t: &PathItemType| match t { PathItemType::Get => "GET", diff --git a/crates/pipeline-manager/src/api/support_data_collector.rs b/crates/pipeline-manager/src/api/support_data_collector.rs index f2df1986cfc..1c179c8f1f1 100644 --- a/crates/pipeline-manager/src/api/support_data_collector.rs +++ b/crates/pipeline-manager/src/api/support_data_collector.rs @@ -11,9 +11,9 @@ use crate::db::types::combined_status::CombinedStatus; use crate::db::types::pipeline::PipelineId; use crate::db::types::tenant::TenantId; use crate::error::ManagerError; +use actix_web::HttpResponse; use actix_web::http::Method; use actix_web::rt::time::timeout; -use actix_web::HttpResponse; use awc::Client; use chrono::{DateTime, Utc}; use feldera_types::error::ErrorResponse; @@ -24,7 +24,7 @@ use std::collections::BTreeMap; use std::io::Write; use std::sync::Arc; use tokio::sync::watch; -use tokio::time::{sleep, Duration, Instant}; +use tokio::time::{Duration, Instant, sleep}; use tracing::{debug, error, info}; use utoipa::{IntoParams, ToSchema}; @@ -1367,7 +1367,7 @@ mod tests { use serde_json::json; use std::sync::Arc; use tokio::sync::Mutex; - use tokio::time::{sleep, Duration}; + use tokio::time::{Duration, sleep}; use uuid::Uuid; #[test] diff --git a/crates/pipeline-manager/src/auth.rs b/crates/pipeline-manager/src/auth.rs index 5d6eca2de5f..dd6688f6774 100644 --- a/crates/pipeline-manager/src/auth.rs +++ b/crates/pipeline-manager/src/auth.rs @@ -48,24 +48,24 @@ use std::{collections::HashMap, env, sync::Arc}; +use actix_web::HttpMessage; use actix_web::body::MessageBody; use actix_web::http::header::{self, HeaderMap, HeaderName, HeaderValue}; use actix_web::middleware::Next; -use actix_web::HttpMessage; use actix_web::{ dev::{ServiceRequest, ServiceResponse}, error::ErrorUnauthorized, web::Data, }; use actix_web_httpauth::extractors::{ - bearer::{BearerAuth, Config}, AuthenticationError, + bearer::{BearerAuth, Config}, }; -use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; +use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use cached::{Cached, TimedCache}; -use jsonwebtoken::{decode, decode_header, Algorithm, DecodingKey, TokenData, Validation}; +use jsonwebtoken::{Algorithm, DecodingKey, TokenData, Validation, decode, decode_header}; use rand::rngs::ThreadRng; -use rand::{distributions::Alphanumeric, Rng}; +use rand::{Rng, distributions::Alphanumeric}; use serde::{Deserialize, Serialize}; use serde_json::Value; use static_assertions::assert_impl_any; @@ -82,7 +82,7 @@ use crate::db::storage_postgres::StoragePostgres; use crate::db::types::role::Role; use crate::db::types::tenant::TenantId; use crate::oidc::fetch::{ - fetch_issuer_jwks, fetch_jwks_uri_from_discovery, oidc_http_client, OidcDestination, + OidcDestination, fetch_issuer_jwks, fetch_jwks_uri_from_discovery, oidc_http_client, }; use reqwest::Certificate; @@ -191,20 +191,17 @@ pub(crate) async fn promote_websocket_subprotocol_auth( mut req: ServiceRequest, next: Next, ) -> Result, actix_web::Error> { - if !req.headers().contains_key(header::AUTHORIZATION) { - if let Some(token) = decode_ws_subprotocol(req.headers(), WS_BEARER_PROTOCOL_PREFIX) { - if let Ok(authorization) = HeaderValue::from_str(&format!("Bearer {token}")) { - req.headers_mut() - .insert(header::AUTHORIZATION, authorization); - if let Some(tenant) = - decode_ws_subprotocol(req.headers(), WS_TENANT_PROTOCOL_PREFIX) - { - if let Ok(tenant) = HeaderValue::from_str(&tenant) { - req.headers_mut() - .insert(HeaderName::from_static(TENANT_HEADER), tenant); - } - } - } + if !req.headers().contains_key(header::AUTHORIZATION) + && let Some(token) = decode_ws_subprotocol(req.headers(), WS_BEARER_PROTOCOL_PREFIX) + && let Ok(authorization) = HeaderValue::from_str(&format!("Bearer {token}")) + { + req.headers_mut() + .insert(header::AUTHORIZATION, authorization); + if let Some(tenant) = decode_ws_subprotocol(req.headers(), WS_TENANT_PROTOCOL_PREFIX) + && let Ok(tenant) = HeaderValue::from_str(&tenant) + { + req.headers_mut() + .insert(HeaderName::from_static(TENANT_HEADER), tenant); } } next.call(req).await @@ -571,7 +568,7 @@ async fn bearer_auth( "Database error while fetching tenant: {e}" )), req, - )) + )); } } } @@ -772,27 +769,27 @@ impl OidcClaimExt for TokenData { let sub = &self.claims.sub; // Check if we have explicit tenant authorization in the claim - if let Some(authorized) = self.authorized_tenants() { - if !authorized.is_empty() { - let selected = headers - .get(TENANT_HEADER) - .and_then(|h| h.to_str().ok()) - .filter(|s| !s.is_empty()); - // A selector is always checked against the claim, never ignored. - // Honouring it only when the claim lists several tenants would - // silently place a caller in its one authorized tenant while it - // believes it named another. - return match selected { - Some(selected) if authorized.iter().any(|t| t == selected) => { - Ok(selected.to_string()) - } - Some(selected) => Err(AuthError::UnauthorizedTenant(selected.to_string())), - None if authorized.len() == 1 => Ok(authorized[0].clone()), - None => Err(AuthError::MissingTenantHeader), - }; - } - // Empty array falls through to fallback logic + if let Some(authorized) = self.authorized_tenants() + && !authorized.is_empty() + { + let selected = headers + .get(TENANT_HEADER) + .and_then(|h| h.to_str().ok()) + .filter(|s| !s.is_empty()); + // A selector is always checked against the claim, never ignored. + // Honouring it only when the claim lists several tenants would + // silently place a caller in its one authorized tenant while it + // believes it named another. + return match selected { + Some(selected) if authorized.iter().any(|t| t == selected) => { + Ok(selected.to_string()) + } + Some(selected) => Err(AuthError::UnauthorizedTenant(selected.to_string())), + None if authorized.len() == 1 => Ok(authorized[0].clone()), + None => Err(AuthError::MissingTenantHeader), + }; } + // Empty array falls through to fallback logic // Fallback when the token claims no tenant at all: derive one. // Priority: issuer-domain > sub (if enabled) @@ -1212,17 +1209,17 @@ async fn decode_token_with_validation( // Provider-specific validations based on optional fields // Validate client_id if present (AWS Cognito puts it in a separate field) - if let Some(ref client_id) = token_data.claims.client_id { - if configuration.client_id != *client_id { - return Err(jsonwebtoken::errors::ErrorKind::InvalidAudience.into()); - } + if let Some(ref client_id) = token_data.claims.client_id + && configuration.client_id != *client_id + { + return Err(jsonwebtoken::errors::ErrorKind::InvalidAudience.into()); } // Validate token_use if present (AWS Cognito requires "access" for access tokens) - if let Some(ref token_use) = token_data.claims.token_use { - if token_use != "access" { - return Err(jsonwebtoken::errors::ErrorKind::InvalidToken.into()); - } + if let Some(ref token_use) = token_data.claims.token_use + && token_use != "access" + { + return Err(jsonwebtoken::errors::ErrorKind::InvalidToken.into()); } Ok(token_data) @@ -1442,10 +1439,10 @@ pub(crate) fn parse_rsa_jwks(value: &Value) -> Result(key: &str, check: &str, json: &'a Value) -> Option<&'a Value> { - if let Some(value) = validate_field_is_str(key, json) { - if value == check { - return Some(json); - } + if let Some(value) = validate_field_is_str(key, json) + && value == check + { + return Some(json); } debug!( "Skipping JWK key because it did not match the required shape {} {}", @@ -1456,10 +1453,10 @@ fn check_key_as_str<'a>(key: &str, check: &str, json: &'a Value) -> Option<&'a V fn validate_field_is_str<'a>(key: &str, json: &'a Value) -> Option<&'a str> { let value = json.get(key); - if let Some(value) = value { - if let Some(value) = value.as_str() { - return Some(value); - } + if let Some(value) = value + && let Some(value) = value.as_str() + { + return Some(value); } None } @@ -1487,16 +1484,17 @@ mod test { use actix_http::{HttpMessage, StatusCode}; use actix_web::{ + App, HttpRequest, HttpResponse, body::{BoxBody, EitherBody}, dev::ServiceResponse, http::{self}, - test, web, App, HttpRequest, HttpResponse, + test, web, }; use actix_web_httpauth::middleware::HttpAuthentication; use base64::Engine; use cached::Cached; use chrono::Utc; - use jsonwebtoken::{encode, Algorithm, DecodingKey, EncodingKey, Header, Validation}; + use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, Validation, encode}; use tokio::sync::{Mutex, RwLock}; use uuid::Uuid; diff --git a/crates/pipeline-manager/src/bin/pipeline-manager.rs b/crates/pipeline-manager/src/bin/pipeline-manager.rs index 1f139b56907..b53b48c78db 100644 --- a/crates/pipeline-manager/src/bin/pipeline-manager.rs +++ b/crates/pipeline-manager/src/bin/pipeline-manager.rs @@ -5,7 +5,7 @@ use clap::{Args, Command, FromArgMatches}; use colored::Colorize; use feldera_observability as observability; use pipeline_manager::api::main::ApiDoc; -use pipeline_manager::cluster_monitor::{cluster_monitor, LocalResourcesPoller}; +use pipeline_manager::cluster_monitor::{LocalResourcesPoller, cluster_monitor}; use pipeline_manager::compiler::main::{compiler_main, compiler_precompile}; #[cfg(feature = "postgresql_embedded")] use pipeline_manager::config::PgEmbedConfig; @@ -25,7 +25,11 @@ use utoipa::OpenApi; fn main() -> anyhow::Result<()> { ensure_default_crypto_provider(); init_fd_limit(); - let _guard = observability::init("https://18aa37ae23e7130b57b91aaad432bc18@o4510219052253184.ingest.us.sentry.io/4510298809827328", "pipeline-manager", env!("CARGO_PKG_VERSION")); + let _guard = observability::init( + "https://18aa37ae23e7130b57b91aaad432bc18@o4510219052253184.ingest.us.sentry.io/4510298809827328", + "pipeline-manager", + env!("CARGO_PKG_VERSION"), + ); pipeline_manager::logging::init_service_logging( "[manager]".cyan(), feldera_observability::json_logging::ServiceName::Manager, diff --git a/crates/pipeline-manager/src/cluster_monitor.rs b/crates/pipeline-manager/src/cluster_monitor.rs index 0ab9205f85f..b91b28a88ed 100644 --- a/crates/pipeline-manager/src/cluster_monitor.rs +++ b/crates/pipeline-manager/src/cluster_monitor.rs @@ -101,7 +101,9 @@ pub async fn cluster_monitor( if matches!(e, DBError::NoClusterMonitorEventsAvailable) { None } else { - error!("Cluster monitor cannot perform monitoring because it is unable to retrieve the latest event due to: {e}"); + error!( + "Cluster monitor cannot perform monitoring because it is unable to retrieve the latest event due to: {e}" + ); tokio::time::sleep(MONITOR_INTERVAL).await; continue; } @@ -234,8 +236,8 @@ pub async fn cluster_monitor( }; // Clean up events that no longer need to be retained - if stored { - if let Err(e) = db + if stored + && let Err(e) = db .lock() .await .delete_cluster_monitor_events_beyond_retention( @@ -243,9 +245,8 @@ pub async fn cluster_monitor( MONITOR_RETENTION_NUM, ) .await - { - error!("Cluster monitor is unable to clean up based on retention due to: {e}"); - } + { + error!("Cluster monitor is unable to clean up based on retention due to: {e}"); } } else { iterations_without_insert += 1; @@ -281,7 +282,9 @@ async fn poll_service_health_endpoint( { Ok(resp) if resp.status().is_success() => ( true, - format!("Healthy: The {service_name} service responded successfully to the last health check."), + format!( + "Healthy: The {service_name} service responded successfully to the last health check." + ), ), Ok(resp) => { let status = resp.status(); @@ -323,7 +326,6 @@ async fn poll_service_health_endpoint( source: {}. Please check the {service_name} logs for more information.", source_error(&e) ), - ), } } diff --git a/crates/pipeline-manager/src/common_error.rs b/crates/pipeline-manager/src/common_error.rs index 36c99db1a6e..26efb841836 100644 --- a/crates/pipeline-manager/src/common_error.rs +++ b/crates/pipeline-manager/src/common_error.rs @@ -1,9 +1,9 @@ use actix_web::{ - body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, + HttpResponse, HttpResponseBuilder, ResponseError, body::BoxBody, http::StatusCode, }; use feldera_types::error::{DetailedError, ErrorResponse}; use serde::Serialize; -use serde::{ser::SerializeStruct, Serializer}; +use serde::{Serializer, ser::SerializeStruct}; use std::backtrace::Backtrace; use std::io::Error as IOError; use std::{borrow::Cow, error::Error as StdError, fmt, fmt::Display}; diff --git a/crates/pipeline-manager/src/compiler/error.rs b/crates/pipeline-manager/src/compiler/error.rs index 5ec4fbff2c0..da88aeedc4f 100644 --- a/crates/pipeline-manager/src/compiler/error.rs +++ b/crates/pipeline-manager/src/compiler/error.rs @@ -1,5 +1,5 @@ use actix_web::{ - body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, + HttpResponse, HttpResponseBuilder, ResponseError, body::BoxBody, http::StatusCode, }; use feldera_types::error::{DetailedError, ErrorResponse}; use serde::Serialize; diff --git a/crates/pipeline-manager/src/compiler/main.rs b/crates/pipeline-manager/src/compiler/main.rs index 18fb54b5bee..2eadfa3af18 100644 --- a/crates/pipeline-manager/src/compiler/main.rs +++ b/crates/pipeline-manager/src/compiler/main.rs @@ -2,17 +2,17 @@ use crate::api::error::ApiError; use crate::common_error::CommonError; use crate::compiler::error::CompilerError; use crate::compiler::rust_compiler::{ - cleanup_pipeline_binaries, perform_rust_compilation, rust_compiler_task, RustCompilationError, - RustCompilationResult, CLEANUP_INTERVAL, + CLEANUP_INTERVAL, RustCompilationError, RustCompilationResult, cleanup_pipeline_binaries, + perform_rust_compilation, rust_compiler_task, }; use crate::compiler::sql_compiler::{ - decide_stale_jar, ephemeral_compilation_dir, jar_cache_dir, perform_sql_compilation, - sql_compiler_task, validate_program, ProgramValidationRequest, SqlCompilationError, - SqlCompilationOutput, + ProgramValidationRequest, SqlCompilationError, SqlCompilationOutput, decide_stale_jar, + ephemeral_compilation_dir, jar_cache_dir, perform_sql_compilation, sql_compiler_task, + validate_program, }; use crate::compiler::util::{ - cleanup_specific_directories, cleanup_specific_files, pipeline_binary_filename, - program_info_filename, validate_is_sha256_checksum, CleanupDecision, DiskSpace, + CleanupDecision, DiskSpace, cleanup_specific_directories, cleanup_specific_files, + pipeline_binary_filename, program_info_filename, validate_is_sha256_checksum, }; use crate::config::{CommonConfig, CompilerConfig}; use crate::db::probe::DbProbe; @@ -24,7 +24,7 @@ use crate::db::types::version::Version; use crate::error::ManagerError; use actix_files::NamedFile; use actix_web::error::PayloadError; -use actix_web::{get, post, web, HttpRequest, HttpResponse, HttpServer, Responder}; +use actix_web::{HttpRequest, HttpResponse, HttpServer, Responder, get, post, web}; use futures_util::{Stream, StreamExt}; use std::net::TcpListener; use std::path::Path; @@ -52,7 +52,9 @@ fn decode_url_encoded_parameter( /// Checks if the required compilation artifacts exist for the specified pipeline and version. /// If `program_info_integrity_checksum` is "none", only the binary existence is checked. -#[get("/artifacts/{pipeline_id}/{program_version}/{source_checksum}/{binary_integrity_checksum}/{program_info_integrity_checksum}")] +#[get( + "/artifacts/{pipeline_id}/{program_version}/{source_checksum}/{binary_integrity_checksum}/{program_info_integrity_checksum}" +)] async fn check_compilation_artifacts( config: web::Data, req: HttpRequest, @@ -693,13 +695,13 @@ fn insufficient_storage_response(cause: &str, error: &ManagerError) -> HttpRespo /// Removes a temp upload file. Failure only leaves an orphan file behind, so /// it is logged rather than propagated. async fn remove_temp_upload_file(temp_file_path: &Path) { - if let Err(e) = fs::remove_file(temp_file_path).await { - if e.kind() != std::io::ErrorKind::NotFound { - warn!( - "Unable to remove temp upload file '{}': {e}", - temp_file_path.display() - ); - } + if let Err(e) = fs::remove_file(temp_file_path).await + && e.kind() != std::io::ErrorKind::NotFound + { + warn!( + "Unable to remove temp upload file '{}': {e}", + temp_file_path.display() + ); } } @@ -748,15 +750,13 @@ async fn healthz( config: web::Data, query: web::Query, ) -> Result { - if query.check_storage { - let pressure = DiskSpace::new_from_path(&config.working_dir()).and_then(|disk_space| { - storage_pressure_message(&disk_space, STORAGE_PRESSURE_THRESHOLD) - }); - if let Some(message) = pressure { - return Ok( - HttpResponse::ServiceUnavailable().json(serde_json::json!({ "status": message })) - ); - } + if query.check_storage + && let Some(disk_space) = DiskSpace::new_from_path(&config.working_dir()) + && let Some(message) = storage_pressure_message(&disk_space, STORAGE_PRESSURE_THRESHOLD) + { + return Ok( + HttpResponse::ServiceUnavailable().json(serde_json::json!({ "status": message })) + ); } Ok(probe.lock().await.as_http_response()) } @@ -1076,8 +1076,8 @@ async fn artifact_server_janitor_task(config: CompilerConfig, db: Arc { - error!("Rust worker {worker_id}: compilation cleanup failed: database error occurred: {e}"); + error!( + "Rust worker {worker_id}: compilation cleanup failed: database error occurred: {e}" + ); } RustCompilationCleanupError::Utility(e) => { - error!("Rust worker {worker_id}: compilation cleanup failed: utility error occurred: {e}"); + error!( + "Rust worker {worker_id}: compilation cleanup failed: utility error occurred: {e}" + ); } RustCompilationCleanupError::TargetCleared => { if allow_exit_upon_target_cleared { // This restart behavior only occurs for a standalone compiler server - warn!("Rust worker {worker_id}: target directory has been cleared due to lack of space -- restarting to have any precompilation re-applied"); + warn!( + "Rust worker {worker_id}: target directory has been cleared due to lack of space -- restarting to have any precompilation re-applied" + ); return Err(()); } else { - warn!("Rust worker {worker_id}: target directory has been cleared due to lack of space -- the next compilation will be slow"); + warn!( + "Rust worker {worker_id}: target directory has been cleared due to lack of space -- the next compilation will be slow" + ); } } } @@ -144,11 +152,15 @@ pub async fn rust_compiler_task( outdated_version, latest_version, } => { - debug!("Rust worker {worker_id}: compilation canceled: pipeline program version ({outdated_version}) is outdated by latest ({latest_version})"); + debug!( + "Rust worker {worker_id}: compilation canceled: pipeline program version ({outdated_version}) is outdated by latest ({latest_version})" + ); } e => { unexpected_error = true; - error!("Rust worker {worker_id}: compilation canceled: unexpected database error occurred: {e}"); + error!( + "Rust worker {worker_id}: compilation canceled: unexpected database error occurred: {e}" + ); } } } @@ -585,7 +597,7 @@ async fn upload_binary_to_endpoint_with_retries( _ => { return Err(RustCompilationError::SystemError( "Invalid delivery mode for HTTP upload".to_string(), - )) + )); } }; @@ -667,7 +679,7 @@ async fn upload_program_info_to_endpoint_with_retries( _ => { return Err(RustCompilationError::SystemError( "Invalid delivery mode for HTTP upload".to_string(), - )) + )); } }; @@ -1155,15 +1167,19 @@ async fn checkout_runtime_version( match clone(repo_location).await { Ok(output) => { if !output.status.success() { - return Err(RustCompilationError::SystemError(format!("Unable to clone latest runtime version for '{requested_runtime_version}' for compilation.\n`git clone` failed with exit code: {}\nstderr:\n{}\nstdout:\n{}", + return Err(RustCompilationError::SystemError(format!( + "Unable to clone latest runtime version for '{requested_runtime_version}' for compilation.\n`git clone` failed with exit code: {}\nstderr:\n{}\nstdout:\n{}", output.status.code().unwrap_or(-1), String::from_utf8_lossy(&output.stderr), - String::from_utf8_lossy(&output.stdout)))); + String::from_utf8_lossy(&output.stdout) + ))); } } - Err(e) => return Err(RustCompilationError::SystemError(format!( - "Unable to clone repo for runtime version override to '{requested_runtime_version}' for compilation, `git clone` failed: {e}", - ))), + Err(e) => { + return Err(RustCompilationError::SystemError(format!( + "Unable to clone repo for runtime version override to '{requested_runtime_version}' for compilation, `git clone` failed: {e}", + ))); + } } } @@ -1219,14 +1235,18 @@ async fn checkout_runtime_version( if output.status.success() { Ok(()) } else { - debug!("Failed to checkout requested runtime, trying to sync with latest git repository."); + debug!( + "Failed to checkout requested runtime, trying to sync with latest git repository." + ); let fetch_result = fetch(repo_location, requested_runtime_version).await.map_err(|e| { RustCompilationError::SystemError(format!("Unable to switch runtime version to '{requested_runtime_version}' for compilation, `git fetch` failed: {e}"))})?; if !fetch_result.status.success() { - return Err(RustCompilationError::SystemError(format!("Unable to fetch latest runtime version for '{requested_runtime_version}' for compilation.\nGit command failed with exit code: {}\nstderr:\n{}\nstdout:\n{}", + return Err(RustCompilationError::SystemError(format!( + "Unable to fetch latest runtime version for '{requested_runtime_version}' for compilation.\nGit command failed with exit code: {}\nstderr:\n{}\nstdout:\n{}", fetch_result.status.code().unwrap_or(-1), String::from_utf8_lossy(&fetch_result.stderr), - String::from_utf8_lossy(&fetch_result.stdout)))); + String::from_utf8_lossy(&fetch_result.stdout) + ))); } let output = checkout(repo_location, requested_runtime_version) @@ -1263,12 +1283,7 @@ pub async fn resolve_runtime_sha( let repo_location = runtime_version.runtime_sources(config); match Command::new("git") .current_dir(&repo_location) - .args([ - "-c", - "protocol.version=2", - "rev-parse", - version, - ]) + .args(["-c", "protocol.version=2", "rev-parse", version]) .output() .await { @@ -2146,7 +2161,10 @@ async fn cleanup_rust_compilation( .is_ok_and(|duration| duration >= CLEANUP_RETENTION) { deletion.insert(artifact_name.clone()); - trace!("Rust compilation cleanup: retention for artifact '{}' has expired -- marked for deletion", artifact_name); + trace!( + "Rust compilation cleanup: retention for artifact '{}' has expired -- marked for deletion", + artifact_name + ); } } let deletion: Vec = deletion.into_iter().collect(); @@ -2196,10 +2214,14 @@ async fn cleanup_rust_compilation( match target_size_byte { Ok(Ok(target_size_byte)) => { if target_size_byte < disk_space.total_byte / 20 { - error!("Not clearing `target` directory because its size ({target_size_byte} byte) is less than 5%. Please reduce disk usage in another way."); + error!( + "Not clearing `target` directory because its size ({target_size_byte} byte) is less than 5%. Please reduce disk usage in another way." + ); false } else { - error!("Removing `target` directory to make space (should clear up {target_size_byte} byte)..."); + error!( + "Removing `target` directory to make space (should clear up {target_size_byte} byte)..." + ); if let Err(e) = fs::remove_dir_all(&target_dir).await { error!( "Unable to remove `target` directory to make space due to: {e}" @@ -2222,11 +2244,15 @@ async fn cleanup_rust_compilation( } } Ok(Err(e)) => { - error!("Not clearing `target` directory because its size cannot be determined. Reduce disk usage in another way. Due to error: {e}"); + error!( + "Not clearing `target` directory because its size cannot be determined. Reduce disk usage in another way. Due to error: {e}" + ); false } Err(e) => { - error!("Not clearing `target` directory because its size cannot be determined due to a thread join error: {e}"); + error!( + "Not clearing `target` directory because its size cannot be determined due to a thread join error: {e}" + ); false } } @@ -2249,7 +2275,10 @@ async fn cleanup_rust_compilation( false } } else { - warn!("Unable to determine disk space remaining: unable to find disk corresponding to '{}'", target_dir.display()); + warn!( + "Unable to determine disk space remaining: unable to find disk corresponding to '{}'", + target_dir.display() + ); false } } else { @@ -2397,12 +2426,12 @@ mod test { use crate::auth::TenantRecord; use crate::compiler::rust_compiler::prepare_workspace; use crate::compiler::rust_compiler::{ - calculate_source_checksum, decide_cleanup, decide_pipeline_binary_cleanup, - is_permanent_upload_rejection, STALE_TEMP_UPLOAD_MAX_AGE, + STALE_TEMP_UPLOAD_MAX_AGE, calculate_source_checksum, decide_cleanup, + decide_pipeline_binary_cleanup, is_permanent_upload_rejection, }; - use crate::compiler::test::{list_content_as_sorted_names, CompilerTest}; + use crate::compiler::test::{CompilerTest, list_content_as_sorted_names}; use crate::compiler::util::{ - crate_name_pipeline_globals, crate_name_pipeline_main, read_file_content, CleanupDecision, + CleanupDecision, crate_name_pipeline_globals, crate_name_pipeline_main, read_file_content, }; use crate::db::types::program::{CompilationProfile, ProgramStatus, RuntimeSelector}; use crate::db::types::utils::validate_program_info; @@ -2549,18 +2578,22 @@ mod test { .unwrap(), pipeline_descr.udf_rust ); - assert!(read_file_content(&globals_crate_path.join("Cargo.toml")) - .await - .unwrap() - .contains(&pipeline_descr.udf_toml)); + assert!( + read_file_content(&globals_crate_path.join("Cargo.toml")) + .await + .unwrap() + .contains(&pipeline_descr.udf_toml) + ); // Workspace-wide Cargo.toml let workspace_toml_file = test.rust_workdir.join("Cargo.toml"); assert!(workspace_toml_file.is_file()); - assert!(read_file_content(&workspace_toml_file) - .await - .unwrap() - .contains(&format!("members = [\n \"crates/{main_crate_name}\"\n]"))); + assert!( + read_file_content(&workspace_toml_file) + .await + .unwrap() + .contains(&format!("members = [\n \"crates/{main_crate_name}\"\n]")) + ); } /// Tests the binary delivery mode configuration. diff --git a/crates/pipeline-manager/src/compiler/sql_compiler.rs b/crates/pipeline-manager/src/compiler/sql_compiler.rs index 8453ce0bbfe..7e98453e565 100644 --- a/crates/pipeline-manager/src/compiler/sql_compiler.rs +++ b/crates/pipeline-manager/src/compiler/sql_compiler.rs @@ -1,9 +1,8 @@ use crate::common_error::CommonError; use crate::compiler::util::{ - cleanup_specific_directories, cleanup_specific_files, crate_name_pipeline_base, - crate_name_pipeline_globals, create_new_file, create_new_file_with_content, - encode_dir_as_string, read_file_content, recreate_dir, CleanupDecision, ProcessGroupTerminator, - UtilError, + CleanupDecision, ProcessGroupTerminator, UtilError, cleanup_specific_directories, + cleanup_specific_files, crate_name_pipeline_base, crate_name_pipeline_globals, create_new_file, + create_new_file_with_content, encode_dir_as_string, read_file_content, recreate_dir, }; use crate::config::{CommonConfig, CompilerConfig}; use crate::db::error::DBError; @@ -11,7 +10,7 @@ use crate::db::storage::Storage; use crate::db::storage_postgres::StoragePostgres; use crate::db::types::pipeline::PipelineId; use crate::db::types::program::{ - generate_program_info, RuntimeSelector, SqlCompilationInfo, SqlCompilerMessage, + RuntimeSelector, SqlCompilationInfo, SqlCompilerMessage, generate_program_info, }; use crate::db::types::tenant::TenantId; use crate::db::types::utils::validate_program_config; @@ -33,7 +32,7 @@ use tokio::{ fs, process::Command, sync::Mutex, - time::{sleep, Duration}, + time::{Duration, sleep}, }; use tracing::{debug, error, info, trace, warn}; use utoipa::ToSchema; @@ -81,10 +80,14 @@ pub async fn sql_compiler_task( if let Err(e) = cleanup_sql_compilation(&config, db.clone()).await { match e { SqlCompilationCleanupError::Database(e) => { - error!("SQL worker {worker_id}: compilation cleanup failed: database error occurred: {e}"); + error!( + "SQL worker {worker_id}: compilation cleanup failed: database error occurred: {e}" + ); } SqlCompilationCleanupError::Utility(e) => { - error!("SQL worker {worker_id}: compilation cleanup failed: filesystem operation error occurred: {e}"); + error!( + "SQL worker {worker_id}: compilation cleanup failed: filesystem operation error occurred: {e}" + ); } } unexpected_error = true; @@ -114,11 +117,15 @@ pub async fn sql_compiler_task( outdated_version, latest_version, } => { - debug!("SQL worker {worker_id}: compilation canceled: pipeline program version ({outdated_version}) is outdated by latest ({latest_version})"); + debug!( + "SQL worker {worker_id}: compilation canceled: pipeline program version ({outdated_version}) is outdated by latest ({latest_version})" + ); } e => { unexpected_error = true; - error!("SQL worker {worker_id}: compilation canceled: unexpected database error occurred: {e}"); + error!( + "SQL worker {worker_id}: compilation canceled: unexpected database error occurred: {e}" + ); } } } @@ -353,11 +360,15 @@ pub(crate) fn decide_stale_jar(jar_name: &str, metadata: Option) -> Cl } }; let Ok(elapsed) = atime.elapsed() else { - warn!("Unable to determine access time for JAR file, your system clock may be set incorrectly."); + warn!( + "Unable to determine access time for JAR file, your system clock may be set incorrectly." + ); return CleanupDecision::Ignore; }; if elapsed < JAR_CACHE_RETENTION { - trace!("Keeping {jar_name} because it was accessed within the retention window ({elapsed:?} ago)"); + trace!( + "Keeping {jar_name} because it was accessed within the retention window ({elapsed:?} ago)" + ); CleanupDecision::Keep { motivation: "Accessed within the retention window".to_string(), } @@ -789,9 +800,9 @@ pub(crate) async fn perform_sql_compilation( Ok(messages) => messages, Err(e) => { if !exit_status.success() { - return Err(SqlCompilationError::SystemError( - format!("SQL compiler process returned with exit status code ({exit_code}) and stderr which cannot be deserialized due to {e}:\n{stderr_str}") - )); + return Err(SqlCompilationError::SystemError(format!( + "SQL compiler process returned with exit status code ({exit_code}) and stderr which cannot be deserialized due to {e}:\n{stderr_str}" + ))); } else { error!( pipeline_id = %pipeline_id, @@ -1090,7 +1101,7 @@ mod test { /// accessed one is kept, and missing metadata never removes. #[test] fn stale_jar_decision() { - use crate::compiler::sql_compiler::{decide_stale_jar, JAR_CACHE_RETENTION}; + use crate::compiler::sql_compiler::{JAR_CACHE_RETENTION, decide_stale_jar}; use crate::compiler::util::CleanupDecision; let tempdir = tempfile::tempdir().unwrap(); let jar_path = tempdir.path().join("a.jar"); @@ -1118,7 +1129,7 @@ mod test { assert_eq!(decide_stale_jar("a.jar", None), CleanupDecision::Ignore); } - use crate::compiler::test::{list_content_as_sorted_names, CompilerTest}; + use crate::compiler::test::{CompilerTest, list_content_as_sorted_names}; use crate::compiler::util::{create_new_file, recreate_dir}; use crate::db::types::program::ProgramStatus; use crate::db::types::utils::validate_program_info; @@ -1484,9 +1495,11 @@ mod test { assert_eq!(table_properties_only.name, "t1"); assert_eq!(table_properties_only.properties.len(), 1); assert!(table_properties_only.properties.contains_key("connectors")); - assert!(table_properties_only.properties["connectors"] - .value - .contains("\"name\": \"c1\"")); + assert!( + table_properties_only.properties["connectors"] + .value + .contains("\"name\": \"c1\"") + ); } /// Tests that SQL compiler recovers from an incorrect platform version. @@ -1570,11 +1583,13 @@ mod test { test.sql_compiler_tick().await; let pipeline_descr = test.get_pipeline(tenant_id, pipeline_id).await; assert_eq!(pipeline_descr.program_status, ProgramStatus::SqlError); - assert!(pipeline_descr - .program_error - .sql_compilation - .is_some_and(|info| info.messages.len() == 1 - && info.messages[0].to_owned().error_type == "Error parsing SQL")); + assert!( + pipeline_descr + .program_error + .sql_compilation + .is_some_and(|info| info.messages.len() == 1 + && info.messages[0].to_owned().error_type == "Error parsing SQL") + ); } /// Tests that compilation fails with an invalid connector. @@ -1597,11 +1612,13 @@ mod test { assert_eq!(pipeline_descr.program_status, ProgramStatus::SqlError); // First message is a warning about the connector missing a name // Second message is an error - assert!(pipeline_descr - .program_error - .sql_compilation - .is_some_and(|info| info.messages.len() == 2 - && info.messages[1].to_owned().error_type == "ConnectorGenerationError")); + assert!( + pipeline_descr + .program_error + .sql_compilation + .is_some_and(|info| info.messages.len() == 2 + && info.messages[1].to_owned().error_type == "ConnectorGenerationError") + ); } /// Tests that the cleanup ignores files and directories that do not follow the pattern. diff --git a/crates/pipeline-manager/src/compiler/test.rs b/crates/pipeline-manager/src/compiler/test.rs index a237ef109cf..0815641ebdc 100644 --- a/crates/pipeline-manager/src/compiler/test.rs +++ b/crates/pipeline-manager/src/compiler/test.rs @@ -1,5 +1,5 @@ use crate::compiler::sql_compiler::{attempt_end_to_end_sql_compilation, cleanup_sql_compilation}; -use crate::compiler::util::{encode_dir_as_string, read_file_content, DirectoryContent}; +use crate::compiler::util::{DirectoryContent, encode_dir_as_string, read_file_content}; use crate::config::{CommonConfig, CompilerConfig}; use crate::db::storage::Storage; use crate::db::storage_postgres::StoragePostgres; diff --git a/crates/pipeline-manager/src/compiler/util.rs b/crates/pipeline-manager/src/compiler/util.rs index 7898edd095d..7eafec1d092 100644 --- a/crates/pipeline-manager/src/compiler/util.rs +++ b/crates/pipeline-manager/src/compiler/util.rs @@ -1,12 +1,12 @@ use crate::db::types::pipeline::PipelineId; use crate::db::types::version::Version; -use base64::prelude::{Engine, BASE64_STANDARD}; +use base64::prelude::{BASE64_STANDARD, Engine}; use flate2::Compression; use hex; +use nix::NixPath; use nix::libc::pid_t; -use nix::sys::signal::{killpg, Signal}; +use nix::sys::signal::{Signal, killpg}; use nix::unistd::Pid; -use nix::NixPath; use openssl::sha::sha256; use sha2::{Digest, Sha256}; use std::fs::Metadata; @@ -69,7 +69,10 @@ impl Drop for ProcessGroupTerminator { }; // Send the SIGKILL to the PGRP if let Err(e) = killpg(Pid::from_raw(pgrp), Signal::SIGKILL) { - error!("Failed to cancel {}: attempt to kill the process and its subprocesses (PGRP: {}) failed: {e}", self.subject, self.process_group); + error!( + "Failed to cancel {}: attempt to kill the process and its subprocesses (PGRP: {}) failed: {e}", + self.subject, self.process_group + ); } debug!( "Successfully cancelled {} by killing its process group", @@ -518,7 +521,10 @@ pub async fn cleanup_specific_files( } } } else if warn_ignore { - warn!("{cleanup_name} cleanup: ignoring '{}' (unexpected directory entry which is not a file)", path.display()); + warn!( + "{cleanup_name} cleanup: ignoring '{}' (unexpected directory entry which is not a file)", + path.display() + ); } } Ok(keep_motivations) @@ -567,7 +573,10 @@ pub async fn cleanup_specific_directories( } } } else if warn_ignore { - warn!("{cleanup_name} cleanup: ignoring '{}' (unexpected directory entry which is not a directory)", path.display()); + warn!( + "{cleanup_name} cleanup: ignoring '{}' (unexpected directory entry which is not a directory)", + path.display() + ); } } Ok(keep_motivations) @@ -716,13 +725,12 @@ impl DiskSpace { #[cfg(test)] mod test { use crate::compiler::util::{ - cleanup_specific_directories, cleanup_specific_files, copy_file, - copy_file_if_checksum_differs, crate_name_pipeline_base, crate_name_pipeline_globals, - crate_name_pipeline_main, create_dir_if_not_exists, create_new_file, - create_new_file_with_content, decode_string_as_dir, encode_dir_as_string, + CleanupDecision, DirectoryContent, cleanup_specific_directories, cleanup_specific_files, + copy_file, copy_file_if_checksum_differs, crate_name_pipeline_base, + crate_name_pipeline_globals, crate_name_pipeline_main, create_dir_if_not_exists, + create_new_file, create_new_file_with_content, decode_string_as_dir, encode_dir_as_string, pipeline_binary_filename, read_file_content, read_file_content_bytes, recreate_dir, recreate_file_with_content, truncate_sha256_checksum, validate_is_sha256_checksum, - CleanupDecision, DirectoryContent, }; use crate::db::types::pipeline::PipelineId; use crate::db::types::version::Version; diff --git a/crates/pipeline-manager/src/config.rs b/crates/pipeline-manager/src/config.rs index cff01b60b66..5d7f1fef9ee 100644 --- a/crates/pipeline-manager/src/config.rs +++ b/crates/pipeline-manager/src/config.rs @@ -16,7 +16,7 @@ use openssl::pkey::PKey; use openssl::rsa::Rsa; use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; use openssl::x509::extension::SubjectAlternativeName; -use openssl::x509::{X509NameBuilder, X509}; +use openssl::x509::{X509, X509NameBuilder}; use postgres_openssl::MakeTlsConnector; use reqwest::Certificate; use rustls::pki_types::pem::PemObject; @@ -812,7 +812,9 @@ impl DatabaseConfig { let mut connector = MakeTlsConnector::new(builder.build()); if self.disable_tls_hostname_verify { - warn!("PostgreSQL TLS hostname verification is disabled. The PostgreSQL server's hostname may not match the one specified in the SSL certificate."); + warn!( + "PostgreSQL TLS hostname verification is disabled. The PostgreSQL server's hostname may not match the one specified in the SSL certificate." + ); connector.set_callback(|ctx, _| { ctx.set_verify_hostname(false); Ok(()) @@ -1411,9 +1413,11 @@ mod tests { // A trust that names no issuer or no subject would match on the wrong // half of the pair, so it is refused at startup rather than at auth time. assert!(r#"[{"issuer": "", "subject": "x"}]"#.parse::().is_err()); - assert!(r#"[{"issuer": "https://idp.example", "subject": ""}]"# - .parse::() - .is_err()); + assert!( + r#"[{"issuer": "https://idp.example", "subject": ""}]"# + .parse::() + .is_err() + ); assert!("not json".parse::().is_err()); } diff --git a/crates/pipeline-manager/src/db/error.rs b/crates/pipeline-manager/src/db/error.rs index ba914cb803d..d3b4eb88792 100644 --- a/crates/pipeline-manager/src/db/error.rs +++ b/crates/pipeline-manager/src/db/error.rs @@ -9,13 +9,13 @@ use crate::db::types::tenant::TenantId; use crate::db::types::utils::ValidationError; use crate::db::types::version::Version; use actix_web::{ - body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, + HttpResponse, HttpResponseBuilder, ResponseError, body::BoxBody, http::StatusCode, }; use deadpool_postgres::PoolError; use feldera_types::error::DetailedError; use feldera_types::error::ErrorResponse; use refinery::Error as RefineryError; -use serde::{ser::SerializeStruct, Serialize, Serializer}; +use serde::{Serialize, Serializer, ser::SerializeStruct}; use std::{backtrace::Backtrace, borrow::Cow, error::Error as StdError, fmt, fmt::Display}; use tokio_postgres::error::{Error as PgError, SqlState}; @@ -551,7 +551,10 @@ impl Display for DBError { ) } DBError::InvalidProgramError { value, error } => { - write!(f, "JSON for 'program_error' field:\n{value:#}\n\n... is not valid due to: {error}") + write!( + f, + "JSON for 'program_error' field:\n{value:#}\n\n... is not valid due to: {error}" + ) } DBError::EditRestrictedToClearedStorage { not_allowed } => { write!( @@ -561,19 +564,34 @@ impl Display for DBError { ) } DBError::InvalidErrorResponse { value, error } => { - write!(f, "JSON for 'deployment_error' field:\n{value:#}\n\n... is not valid due to: {error}") + write!( + f, + "JSON for 'deployment_error' field:\n{value:#}\n\n... is not valid due to: {error}" + ) } DBError::FailedToSerializeRuntimeConfig { error } => { - write!(f, "Unable to serialize runtime configuration for 'runtime_config' field as JSON due to: {error}") + write!( + f, + "Unable to serialize runtime configuration for 'runtime_config' field as JSON due to: {error}" + ) } DBError::FailedToSerializeProgramConfig { error } => { - write!(f, "Unable to serialize program configuration for 'program_config' field as JSON due to: {error}") + write!( + f, + "Unable to serialize program configuration for 'program_config' field as JSON due to: {error}" + ) } DBError::FailedToSerializeProgramError { error } => { - write!(f, "Unable to serialize program error for 'program_error' field as JSON due to: {error}") + write!( + f, + "Unable to serialize program error for 'program_error' field as JSON due to: {error}" + ) } DBError::FailedToSerializeErrorResponse { error } => { - write!(f, "Unable to serialize error response for 'deployment_error' field as JSON due to: {error}") + write!( + f, + "Unable to serialize error response for 'deployment_error' field as JSON due to: {error}" + ) } DBError::UniqueKeyViolation { constraint, .. } => { write!(f, "Unique key violation for '{constraint}'") @@ -720,16 +738,25 @@ impl Display for DBError { write!(f, "Unknown pipeline name '{pipeline_name}'") } DBError::UpdateRestrictedToStopped => { - write!(f, "Pipeline can only be updated while stopped. Stop it first by invoking '/stop'.") + write!( + f, + "Pipeline can only be updated while stopped. Stop it first by invoking '/stop'." + ) } DBError::ProgramStatusUpdateRestrictedToStopped => { write!(f, "Program status can only be updated while stopped.") } DBError::DeleteRestrictedToFullyStopped => { - write!(f, "Cannot delete a pipeline which is not fully stopped. Stop the pipeline first fully by invoking the '/stop' endpoint.") + write!( + f, + "Cannot delete a pipeline which is not fully stopped. Stop the pipeline first fully by invoking the '/stop' endpoint." + ) } DBError::CannotRenameNonExistingPipeline => { - write!(f, "The pipeline name in the request body does not match the one provided in the URL path. This is not allowed when no pipeline with the name provided in the URL path exists.") + write!( + f, + "The pipeline name in the request body does not match the one provided in the URL path. This is not allowed when no pipeline with the name provided in the URL path exists." + ) } DBError::OutdatedProgramVersion { outdated_version, @@ -930,7 +957,11 @@ impl Display for DBError { write!(f, "Invalid monitor status: '{value}'") } DBError::UnknownClusterMonitorEvent { event_id } => { - write!(f, "Cluster monitor event with identifier '{event_id}' does not exist -- it might have been deleted as monitor events are only retained for {}h and at most {}", MONITOR_RETENTION_HOURS, MONITOR_RETENTION_NUM) + write!( + f, + "Cluster monitor event with identifier '{event_id}' does not exist -- it might have been deleted as monitor events are only retained for {}h and at most {}", + MONITOR_RETENTION_HOURS, MONITOR_RETENTION_NUM + ) } DBError::NoClusterMonitorEventsAvailable => { write!(f, "There are not yet any cluster monitor events recorded") @@ -946,16 +977,25 @@ impl Display for DBError { ) } DBError::UnknownPipelineMonitorEvent { event_id } => { - write!(f, "Pipeline monitor event with identifier '{event_id}' does not exist -- it might have been deleted as only a limited number of events are retained") + write!( + f, + "Pipeline monitor event with identifier '{event_id}' does not exist -- it might have been deleted as only a limited number of events are retained" + ) } DBError::NoPipelineMonitorEventsAvailable => { write!(f, "There are not yet any pipeline monitor events recorded") } DBError::LockTookTooLong => { - write!(f, "The lock required for this operation took too long to acquire. Try this operation again later.") + write!( + f, + "The lock required for this operation took too long to acquire. Try this operation again later." + ) } DBError::DeadlockDetected => { - write!(f, "A deadlock was detected while performing the operation. Try this operation again later. Please also file a bug report, as this error should not happen.") + write!( + f, + "A deadlock was detected while performing the operation. Try this operation again later. Please also file a bug report, as this error should not happen." + ) } } } diff --git a/crates/pipeline-manager/src/db/listen_table.rs b/crates/pipeline-manager/src/db/listen_table.rs index 213661e5c92..1826698b263 100644 --- a/crates/pipeline-manager/src/db/listen_table.rs +++ b/crates/pipeline-manager/src/db/listen_table.rs @@ -2,7 +2,7 @@ use crate::db::error::DBError; use crate::db::storage_postgres::StoragePostgres; use crate::db::types::pipeline::PipelineId; use crate::db::types::tenant::TenantId; -use futures_util::{stream, StreamExt}; +use futures_util::{StreamExt, stream}; use std::sync::Arc; use std::time::Duration; use thiserror::Error as ThisError; @@ -128,7 +128,9 @@ async fn attempt_listen_table( if let Err(e) = notification_sender.try_send(n) { match e { TrySendError::Full(_n) => { - error!("Notifier is unable to send notification out on channel because it has reached capacity"); + error!( + "Notifier is unable to send notification out on channel because it has reached capacity" + ); } TrySendError::Closed(_n) => { break ListenError::NotificationReceiverClosed; @@ -148,7 +150,9 @@ async fn attempt_listen_table( } // AsyncMessage is marked non-exhaustive _ => { - error!("Notifier received AsyncMessage that isn't a notification or notice") + error!( + "Notifier received AsyncMessage that isn't a notification or notice" + ) } } } @@ -248,9 +252,9 @@ fn parse_notification( #[cfg(test)] mod test { use super::{ - listen_table, NotificationError, PipelineNotification, PIPELINE_NOTIFY_CHANNEL_CAPACITY, + NotificationError, PIPELINE_NOTIFY_CHANNEL_CAPACITY, PipelineNotification, listen_table, }; - use super::{parse_notification, Operation}; + use super::{Operation, parse_notification}; use crate::db::types::pipeline::{PatchClientMetadata, PipelineDescr, PipelineId}; use crate::db::types::program::ProgramConfig; use crate::db::types::tenant::TenantId; diff --git a/crates/pipeline-manager/src/db/operations/oidc_trust.rs b/crates/pipeline-manager/src/db/operations/oidc_trust.rs index 69a5f13f974..d756c292b45 100644 --- a/crates/pipeline-manager/src/db/operations/oidc_trust.rs +++ b/crates/pipeline-manager/src/db/operations/oidc_trust.rs @@ -2,10 +2,10 @@ use crate::db::error::DBError; use crate::db::operations::utils::{ maybe_tenant_id_foreign_key_constraint_err, maybe_unique_violation, }; -use crate::db::types::oidc_trust::{claim_matches, OidcTrustDescr, OidcTrustId}; +use crate::db::types::oidc_trust::{OidcTrustDescr, OidcTrustId, claim_matches}; use crate::db::types::role::Role; use crate::db::types::tenant::TenantId; -use crate::oidc::destination::{validate_tenant_oidc_url, TenantIssuerPolicy}; +use crate::oidc::destination::{TenantIssuerPolicy, validate_tenant_oidc_url}; use crate::oidc::trust_name::validate_oidc_trust_name; use deadpool_postgres::Transaction; use std::str::FromStr; @@ -211,10 +211,10 @@ pub async fn match_oidc_trust( if !claim_matches(&pattern_subject, subject) { continue; } - if let Some(aud_pattern) = &pattern_audience { - if !audiences.iter().any(|a| claim_matches(aud_pattern, a)) { - continue; - } + if let Some(aud_pattern) = &pattern_audience + && !audiences.iter().any(|a| claim_matches(aud_pattern, a)) + { + continue; } match matched.iter_mut().find(|(t, _)| *t == tenant_id) { Some(entry) => entry.1 = entry.1.max(role), diff --git a/crates/pipeline-manager/src/db/operations/pipeline.rs b/crates/pipeline-manager/src/db/operations/pipeline.rs index d741dc06272..059b0294df8 100644 --- a/crates/pipeline-manager/src/db/operations/pipeline.rs +++ b/crates/pipeline-manager/src/db/operations/pipeline.rs @@ -2,27 +2,27 @@ use crate::api::support_data_collector::SupportBundleData; use crate::db::error::DBError; use crate::db::operations::pipeline_monitor::new_pipeline_monitor_event; use crate::db::operations::pipeline_parsing::{ + PIPELINE_COLUMNS_ALL, PIPELINE_COLUMNS_EVENT_INFO, PIPELINE_COLUMNS_MONITORING, parse_pipeline_row_all, parse_pipeline_row_event_info, parse_pipeline_row_monitoring, - serialize_error_response, serialize_program_error, PIPELINE_COLUMNS_ALL, - PIPELINE_COLUMNS_EVENT_INFO, PIPELINE_COLUMNS_MONITORING, + serialize_error_response, serialize_program_error, }; use crate::db::operations::utils::{ maybe_tenant_id_foreign_key_constraint_err, maybe_unique_violation, }; use crate::db::types::pipeline::{ - bootstrap_config_to_string, runtime_desired_status_to_string, runtime_status_to_string, ExtendedPipelineDescr, ExtendedPipelineDescrEventInfo, ExtendedPipelineDescrMonitoring, - PatchClientMetadata, PipelineDescr, PipelineId, + PatchClientMetadata, PipelineDescr, PipelineId, bootstrap_config_to_string, + runtime_desired_status_to_string, runtime_status_to_string, }; use crate::db::types::program::{ - validate_program_status_transition, ProgramError, ProgramStatus, RustCompilationInfo, - SqlCompilationInfo, + ProgramError, ProgramStatus, RustCompilationInfo, SqlCompilationInfo, + validate_program_status_transition, }; use crate::db::types::resources_status::{ - validate_resources_desired_status_transition, validate_resources_status_transition, - ResourcesDesiredStatus, ResourcesStatus, + ResourcesDesiredStatus, ResourcesStatus, validate_resources_desired_status_transition, + validate_resources_status_transition, }; -use crate::db::types::storage::{validate_storage_status_transition, StorageStatus}; +use crate::db::types::storage::{StorageStatus, validate_storage_status_transition}; use crate::db::types::tenant::TenantId; use crate::db::types::utils::{ validate_deployment_config, validate_pipeline_name, validate_program_config, @@ -1128,21 +1128,19 @@ pub(crate) async fn set_deployment_resources_desired_status( }; // If the current initial desired runtime status is already set, it cannot be changed - if let Some(current_initial) = current.deployment_initial { - if let Some(new_initial) = final_deployment_initial { - if current_initial != new_initial { - return Err(DBError::InitialImmutableUnlessStopped); - } - } + if let Some(current_initial) = current.deployment_initial + && let Some(new_initial) = final_deployment_initial + && current_initial != new_initial + { + return Err(DBError::InitialImmutableUnlessStopped); } // If the current bootstrap policy is already set, it cannot be changed - if let Some(current_bootstrap_config) = current.bootstrap_policy { - if let Some(new_bootstrap_config) = final_bootstrap_config { - if current_bootstrap_config != new_bootstrap_config { - return Err(DBError::BootstrapPolicyImmutableUnlessStopped); - } - } + if let Some(current_bootstrap_config) = current.bootstrap_policy + && let Some(new_bootstrap_config) = final_bootstrap_config + && current_bootstrap_config != new_bootstrap_config + { + return Err(DBError::BootstrapPolicyImmutableUnlessStopped); } // Desired status cannot be set to standby if no file backend is configured diff --git a/crates/pipeline-manager/src/db/operations/pipeline_monitor.rs b/crates/pipeline-manager/src/db/operations/pipeline_monitor.rs index deab05c97cf..4cd7723f5db 100644 --- a/crates/pipeline-manager/src/db/operations/pipeline_monitor.rs +++ b/crates/pipeline-manager/src/db/operations/pipeline_monitor.rs @@ -3,15 +3,15 @@ use crate::db::operations::pipeline::{ get_pipeline_by_id_for_event_info, get_pipeline_for_monitoring, }; use crate::db::operations::pipeline_parsing::{ - parse_pipeline_event_row_extended, parse_pipeline_event_row_short, serialize_error_response, - PIPELINE_EVENT_COLUMNS_ALL, PIPELINE_EVENT_COLUMNS_SHORT, + PIPELINE_EVENT_COLUMNS_ALL, PIPELINE_EVENT_COLUMNS_SHORT, parse_pipeline_event_row_extended, + parse_pipeline_event_row_short, serialize_error_response, }; use crate::db::operations::utils::maybe_unique_violation; use crate::db::types::monitor::{ ExtendedPipelineMonitorEvent, PipelineMonitorEvent, PipelineMonitorEventId, }; use crate::db::types::pipeline::{ - runtime_desired_status_to_string, runtime_status_to_string, PipelineId, + PipelineId, runtime_desired_status_to_string, runtime_status_to_string, }; use crate::db::types::tenant::TenantId; use deadpool_postgres::Transaction; diff --git a/crates/pipeline-manager/src/db/operations/pipeline_parsing.rs b/crates/pipeline-manager/src/db/operations/pipeline_parsing.rs index 937a93b3e77..b328b782851 100644 --- a/crates/pipeline-manager/src/db/operations/pipeline_parsing.rs +++ b/crates/pipeline-manager/src/db/operations/pipeline_parsing.rs @@ -3,9 +3,9 @@ use crate::db::types::monitor::{ ExtendedPipelineMonitorEvent, PipelineMonitorEvent, PipelineMonitorEventId, }; use crate::db::types::pipeline::{ - parse_string_as_bootstrap_config, parse_string_as_runtime_desired_status, - parse_string_as_runtime_status, ClientMetadata, ExtendedPipelineDescr, - ExtendedPipelineDescrEventInfo, ExtendedPipelineDescrMonitoring, PipelineId, + ClientMetadata, ExtendedPipelineDescr, ExtendedPipelineDescrEventInfo, + ExtendedPipelineDescrMonitoring, PipelineId, parse_string_as_bootstrap_config, + parse_string_as_runtime_desired_status, parse_string_as_runtime_status, }; use crate::db::types::program::{ProgramError, ProgramStatus}; use crate::db::types::resources_status::{ResourcesDesiredStatus, ResourcesStatus}; @@ -524,10 +524,10 @@ fn deserialize_program_error_with_default(s: &str) -> ProgramError { #[cfg(test)] mod tests { use super::{ - deserialize_error_response, deserialize_json_value, deserialize_program_error, - deserialize_program_error_with_default, serialize_error_response, serialize_program_error, PIPELINE_COLUMNS_ALL, PIPELINE_COLUMNS_EVENT_INFO, PIPELINE_COLUMNS_MONITORING, - PIPELINE_EVENT_COLUMNS_ALL, PIPELINE_EVENT_COLUMNS_SHORT, + PIPELINE_EVENT_COLUMNS_ALL, PIPELINE_EVENT_COLUMNS_SHORT, deserialize_error_response, + deserialize_json_value, deserialize_program_error, deserialize_program_error_with_default, + serialize_error_response, serialize_program_error, }; use crate::db::error::DBError; use crate::db::types::monitor::{ diff --git a/crates/pipeline-manager/src/db/operations/utils.rs b/crates/pipeline-manager/src/db/operations/utils.rs index 7ed46c05ca2..ead34de7dda 100644 --- a/crates/pipeline-manager/src/db/operations/utils.rs +++ b/crates/pipeline-manager/src/db/operations/utils.rs @@ -43,14 +43,12 @@ pub(crate) fn maybe_tenant_id_foreign_key_constraint_err( ) -> DBError { if let DBError::PostgresError { error, .. } = &err { let db_err = error.as_db_error(); - if let Some(db_err) = db_err { - if db_err.code() == &tokio_postgres::error::SqlState::FOREIGN_KEY_VIOLATION { - if let Some(constraint_name) = db_err.constraint() { - if constraint_name.ends_with("tenant_id_fkey") { - return DBError::UnknownTenant { tenant_id }; - } - } - } + if let Some(db_err) = db_err + && db_err.code() == &tokio_postgres::error::SqlState::FOREIGN_KEY_VIOLATION + && let Some(constraint_name) = db_err.constraint() + && constraint_name.ends_with("tenant_id_fkey") + { + return DBError::UnknownTenant { tenant_id }; } } err @@ -61,18 +59,15 @@ pub(crate) fn maybe_tenant_id_foreign_key_constraint_err( /// instead of surfacing a raw Postgres error. Other errors pass through /// unchanged. pub(crate) fn maybe_user_id_foreign_key_constraint_err(err: DBError, user_id: UserId) -> DBError { - if let DBError::PostgresError { error, .. } = &err { - if let Some(db_err) = error.as_db_error() { - if db_err.code() == &tokio_postgres::error::SqlState::FOREIGN_KEY_VIOLATION { - if let Some(constraint_name) = db_err.constraint() { - if constraint_name.ends_with("user_id_fkey") { - return DBError::UnknownUser { - user_id: user_id.to_string(), - }; - } - } - } - } + if let DBError::PostgresError { error, .. } = &err + && let Some(db_err) = error.as_db_error() + && db_err.code() == &tokio_postgres::error::SqlState::FOREIGN_KEY_VIOLATION + && let Some(constraint_name) = db_err.constraint() + && constraint_name.ends_with("user_id_fkey") + { + return DBError::UnknownUser { + user_id: user_id.to_string(), + }; } err } diff --git a/crates/pipeline-manager/src/db/test.rs b/crates/pipeline-manager/src/db/test.rs index 1724bfcfe8c..e3bdecca54d 100644 --- a/crates/pipeline-manager/src/db/test.rs +++ b/crates/pipeline-manager/src/db/test.rs @@ -1,41 +1,41 @@ use crate::api::support_data_collector::SupportBundleData; -use crate::auth::{generate_api_key, TenantRecord}; +use crate::auth::{TenantRecord, generate_api_key}; use crate::db::error::DBError; use crate::db::error::DBError::InvalidResourcesStatusNotRemain; use crate::db::operations::pipeline::get_pipeline_by_id_for_monitoring; use crate::db::storage::{ExtendedPipelineDescrRunner, Storage}; -use crate::db::storage_postgres::{is_pipeline_assigned_to_worker, StoragePostgres}; +use crate::db::storage_postgres::{StoragePostgres, is_pipeline_assigned_to_worker}; use crate::db::types::api_key::{ApiKeyDescr, ApiKeyId}; use crate::db::types::monitor::{ ClusterMonitorEvent, ClusterMonitorEventId, ExtendedClusterMonitorEvent, ExtendedPipelineMonitorEvent, MonitorStatus, NewClusterMonitorEvent, PipelineMonitorEvent, PipelineMonitorEventId, }; -use crate::db::types::oidc_trust::{claim_matches, OidcTrustDescr, OidcTrustId}; +use crate::db::types::oidc_trust::{OidcTrustDescr, OidcTrustId, claim_matches}; use crate::db::types::pipeline::{ ClientMetadata, ExtendedPipelineDescr, ExtendedPipelineDescrMonitoring, PatchClientMetadata, PipelineDescr, PipelineId, }; use crate::db::types::program::{ - generate_pipeline_config, validate_program_status_transition, CompilationProfile, - ProgramConfig, ProgramError, ProgramInfo, ProgramStatus, RustCompilationInfo, - SqlCompilationInfo, + CompilationProfile, ProgramConfig, ProgramError, ProgramInfo, ProgramStatus, + RustCompilationInfo, SqlCompilationInfo, generate_pipeline_config, + validate_program_status_transition, }; use crate::db::types::resources_status::{ - validate_resources_desired_status_transition, validate_resources_status_transition, - ResourcesDesiredStatus, ResourcesStatus, + ResourcesDesiredStatus, ResourcesStatus, validate_resources_desired_status_transition, + validate_resources_status_transition, }; use crate::db::types::role::{MemberRole, MintableKeyRole, Role}; -use crate::db::types::storage::{validate_storage_status_transition, StorageStatus}; +use crate::db::types::storage::{StorageStatus, validate_storage_status_transition}; use crate::db::types::tenant::TenantId; use crate::db::types::user::{TenantInfo, TenantMember, UserId}; use crate::db::types::utils::{ - validate_api_key_name, validate_deployment_config, validate_pipeline_name, + MAXIMUM_TAG_LENGTH, validate_api_key_name, validate_deployment_config, validate_pipeline_name, validate_program_config, validate_program_info, validate_runtime_config, - validate_storage_status_details, MAXIMUM_TAG_LENGTH, + validate_storage_status_details, }; use crate::db::types::version::Version; -use crate::oidc::destination::{validate_tenant_oidc_url, TenantIssuerPolicy}; +use crate::oidc::destination::{TenantIssuerPolicy, validate_tenant_oidc_url}; use crate::oidc::trust_name::validate_oidc_trust_name; use async_trait::async_trait; use chrono::{TimeZone, Utc}; @@ -61,7 +61,7 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use std::vec; use tokio::spawn; -use tokio::sync::{oneshot, Mutex}; +use tokio::sync::{Mutex, oneshot}; use tokio::time::sleep; use tracing::info; use uuid::Uuid; @@ -91,7 +91,7 @@ impl Drop for DbHandle { #[cfg(not(feature = "postgresql_embedded"))] fn drop(&mut self) { use postgres_openssl::TlsStream; - use tokio_postgres::{tls::NoTlsStream, Connection, Socket}; + use tokio_postgres::{Connection, Socket, tls::NoTlsStream}; enum ConnWrapper { Tls(Connection>), NoTls(Connection), @@ -173,7 +173,10 @@ pub(crate) async fn setup_pg() -> (StoragePostgres, tempfile::TempDir) { match crate::db::pg_setup::install(temp_path.into(), false, Some(port)).await { Ok(pg) => break (_temp_dir, pg), Err(e) => { - info!("Unable to install test database on port {port} ({} attempts left) -- port might have become occupied in the meanwhile. Original error: {e}", 10 - attempt); + info!( + "Unable to install test database on port {port} ({} attempts left) -- port might have become occupied in the meanwhile. Original error: {e}", + 10 - attempt + ); sleep(Duration::from_millis(100)).await; } } @@ -637,7 +640,7 @@ fn limited_pipeline_config() -> impl Strategy { } else { val.2.invalid0 = 1; // Prevent it from being invalid let runtime_config = map_val_to_limited_runtime_config(val.2); - val.3 .0 = 1; // Prevent it from being invalid + val.3.0 = 1; // Prevent it from being invalid let program_info: ProgramInfo = serde_json::from_value(map_val_to_limited_program_info(val.3)).unwrap(); serde_json::to_value(PipelineConfig { @@ -729,15 +732,14 @@ fn limited_optional_storage_status_details() -> impl Strategy( "mismatch detected with model (left) and impl (right)" ), (Err(err_model), Ok(val_impl)) => { - panic!("step({step}): model returned error: {err_model:?}, but impl returned result: {val_impl:?}"); + panic!( + "step({step}): model returned error: {err_model:?}, but impl returned result: {val_impl:?}" + ); } (Ok(val_model), Err(err_impl)) => { - panic!("step({step}): model returned result: {val_model:?}, but impl returned error: {err_impl:?}"); + panic!( + "step({step}): model returned result: {val_model:?}, but impl returned error: {err_impl:?}" + ); } (Err(err_model), Err(err_impl)) => { assert_eq!( @@ -6057,7 +6067,9 @@ impl Storage for Mutex { _tenant_name: String, _provider: String, ) -> DBResult { - panic!("For model-based tests, we generate the TenantID using proptest, as opposed to generating a claim that we then get or create an ID for"); + panic!( + "For model-based tests, we generate the TenantID using proptest, as opposed to generating a claim that we then get or create an ID for" + ); } async fn create_tenant(&self, id: Uuid, _name: &str, _provider: &str) -> DBResult { @@ -6076,11 +6088,11 @@ impl Storage for Mutex { let s = self.lock().await; Ok(s.api_keys .iter() - .filter(|k| k.0 .0 == tenant_id) + .filter(|k| k.0.0 == tenant_id) .map(|k| ApiKeyDescr { - id: k.1 .0, - name: k.0 .1.clone(), - role: k.1 .2, + id: k.1.0, + name: k.0.1.clone(), + role: k.1.2, }) .collect()) } @@ -6124,13 +6136,13 @@ impl Storage for Mutex { let mut hasher = sha::Sha256::new(); hasher.update(key.as_bytes()); let hash = openssl::base64::encode_block(&hasher.finish()); - if s.api_keys.iter().any(|k| k.1 .0 == ApiKeyId(id)) { + if s.api_keys.iter().any(|k| k.1.0 == ApiKeyId(id)) { return Err(DBError::unique_key_violation("api_key_pkey")); } if s.api_keys.contains_key(&(tenant_id, name.to_string())) { return Err(DBError::DuplicateName); } - if s.api_keys.iter().any(|k| k.1 .1 == hash) { + if s.api_keys.iter().any(|k| k.1.1 == hash) { return Err(DBError::duplicate_key()); } s.api_keys.insert( @@ -6148,8 +6160,8 @@ impl Storage for Mutex { let record: Vec<(TenantId, Role)> = s .api_keys .iter() - .filter(|k| k.1 .1 == hash) - .map(|k| (k.0 .0, k.1 .2)) + .filter(|k| k.1.1 == hash) + .map(|k| (k.0.0, k.1.2)) .collect(); assert!(record.len() <= 1); match record.first() { @@ -6268,10 +6280,10 @@ impl Storage for Mutex { if descr.issuer != issuer || !claim_matches(&descr.subject, subject) { continue; } - if let Some(pattern) = &descr.audience { - if !audiences.iter().any(|a| claim_matches(pattern, a)) { - continue; - } + if let Some(pattern) = &descr.audience + && !audiences.iter().any(|a| claim_matches(pattern, a)) + { + continue; } match matched.iter_mut().find(|(t, _)| t == scope) { Some(entry) => entry.1 = entry.1.max(descr.role), @@ -8279,7 +8291,7 @@ impl Storage for Mutex { ) -> Result { let mut state = self.lock().await; let mut num_deleted: usize = 0; - let keys: Vec<(TenantId, PipelineId)> = state.pipelines.keys().map(|v| v.clone()).collect(); + let keys: Vec<(TenantId, PipelineId)> = state.pipelines.keys().copied().collect(); for (tenant_id, pipeline_id) in keys { let events = state .pipeline_events diff --git a/crates/pipeline-manager/src/db/types/combined_status.rs b/crates/pipeline-manager/src/db/types/combined_status.rs index 766bfc4cbd4..96a45c245cd 100644 --- a/crates/pipeline-manager/src/db/types/combined_status.rs +++ b/crates/pipeline-manager/src/db/types/combined_status.rs @@ -61,7 +61,9 @@ impl CombinedStatus { RuntimeStatus::Suspended => Self::Suspended, } } else { - error!("Generating combined status encountered unexpected scenario: resource status is Provisioned but runtime status is None -- falling back to Unavailable"); + error!( + "Generating combined status encountered unexpected scenario: resource status is Provisioned but runtime status is None -- falling back to Unavailable" + ); Self::Unavailable } } @@ -114,7 +116,9 @@ impl CombinedDesiredStatus { RuntimeDesiredStatus::Suspended => Self::Suspended, } } else { - error!("Generating combined desired status encountered unexpected scenario: resource desired status is Provisioned but initial and current runtime desired status is None -- falling back to Unavailable"); + error!( + "Generating combined desired status encountered unexpected scenario: resource desired status is Provisioned but initial and current runtime desired status is None -- falling back to Unavailable" + ); Self::Unavailable } } diff --git a/crates/pipeline-manager/src/db/types/program.rs b/crates/pipeline-manager/src/db/types/program.rs index b04ac8c1ed5..1ddf0c924a1 100644 --- a/crates/pipeline-manager/src/db/types/program.rs +++ b/crates/pipeline-manager/src/db/types/program.rs @@ -859,7 +859,7 @@ pub fn generate_pipeline_config( #[cfg(test)] mod tests { - use super::{determine_connector_endpoint_names, RuntimeSelector}; + use super::{RuntimeSelector, determine_connector_endpoint_names}; use crate::db::types::program::ConnectorGenerationError::RelationConnectorNameCollision; use feldera_types::config::{ConnectorConfig, TransportConfig}; use feldera_types::program_schema::{PropertyValue, SourcePosition}; diff --git a/crates/pipeline-manager/src/db/types/utils.rs b/crates/pipeline-manager/src/db/types/utils.rs index e6387ac3d31..41965d87587 100644 --- a/crates/pipeline-manager/src/db/types/utils.rs +++ b/crates/pipeline-manager/src/db/types/utils.rs @@ -16,8 +16,7 @@ use tracing::error; pub(crate) const PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN: &str = r"^[a-zA-Z0-9_-]+$"; /// Description of the non-empty alphanumeric-underscore-hyphen pattern. -pub(crate) const PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN_DESCRIPTION: &str = - "be non-empty and only \ +pub(crate) const PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN_DESCRIPTION: &str = "be non-empty and only \ contain lowercase (a-z), uppercase (A-Z), number (0-9), underscore (_) or hyphen (-) characters"; /// The pattern is almost the same as for Kubernetes label values but slightly stricter. @@ -202,14 +201,18 @@ pub(crate) fn validate_runtime_config( if runtime_config.fault_tolerance.is_enabled() { let e = ValidationError::EnterpriseFeature("fault tolerance".to_string()); if log_if_invalid { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid runtime configuration due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid runtime configuration due to: {e}" + ); } return Err(e); } if let Err(e) = validate_pipeline_env(&runtime_config.env) { let e = ValidationError::InvalidPipelineEnv(e); if log_if_invalid { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid runtime configuration due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid runtime configuration due to: {e}" + ); } return Err(e); } @@ -217,7 +220,9 @@ pub(crate) fn validate_runtime_config( } Err(e) => { if log_if_invalid { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid runtime configuration due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid runtime configuration due to: {e}" + ); } Err(e) } @@ -232,10 +237,12 @@ pub(crate) fn validate_program_config( ) -> Result { let deserialize_result = serde_json::from_value(value.clone()) .map_err(|e| ValidationError::DeserializationFailed(e.to_string())); - if let Err(e) = &deserialize_result { - if log_if_invalid { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid program configuration due to: {e}"); - } + if let Err(e) = &deserialize_result + && log_if_invalid + { + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid program configuration due to: {e}" + ); } deserialize_result } @@ -247,7 +254,9 @@ pub(crate) fn validate_program_info( let deserialize_result = serde_json::from_value(value.clone()) .map_err(|e| ValidationError::DeserializationFailed(e.to_string())); if let Err(e) = &deserialize_result { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid program information due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid program information due to: {e}" + ); } deserialize_result } @@ -262,14 +271,18 @@ pub(crate) fn validate_deployment_config( Ok(deployment_config) => { if let Err(e) = validate_pipeline_env(&deployment_config.global.env) { let e = ValidationError::InvalidPipelineEnv(e); - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid deployment configuration due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid deployment configuration due to: {e}" + ); Err(e) } else { Ok(deployment_config) } } Err(e) => { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid deployment configuration due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid deployment configuration due to: {e}" + ); Err(e) } } @@ -282,7 +295,9 @@ pub(crate) fn validate_storage_status_details( let deserialize_result = serde_json::from_value(value.clone()) .map_err(|e| ValidationError::DeserializationFailed(e.to_string())); if let Err(e) = &deserialize_result { - error!("Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid storage status details due to: {e}"); + error!( + "Backward incompatibility detected: the following JSON:\n{value:#}\n\n... is no longer a valid storage status details due to: {e}" + ); } deserialize_result } @@ -290,14 +305,14 @@ pub(crate) fn validate_storage_status_details( #[cfg(test)] mod tests { use super::{ - validate_api_key_name, validate_connector_name, validate_deployment_config, - validate_description, validate_pipeline_name, validate_program_config, - validate_program_info, validate_runtime_config, validate_tags, ValidationError, MAXIMUM_API_KEY_NAME_LENGTH, MAXIMUM_CONNECTOR_NAME_LENGTH, MAXIMUM_DESCRIPTION_LENGTH, - MAXIMUM_PIPELINE_NAME_LENGTH, MAXIMUM_TAGS_PER_PIPELINE, MAXIMUM_TAG_LENGTH, + MAXIMUM_PIPELINE_NAME_LENGTH, MAXIMUM_TAG_LENGTH, MAXIMUM_TAGS_PER_PIPELINE, PATTERN_KUBERNETES_LABEL_VALUE, PATTERN_KUBERNETES_LABEL_VALUE_DESCRIPTION, PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN, - PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN_DESCRIPTION, + PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN_DESCRIPTION, ValidationError, + validate_api_key_name, validate_connector_name, validate_deployment_config, + validate_description, validate_pipeline_name, validate_program_config, + validate_program_info, validate_runtime_config, validate_tags, }; use crate::db::error::DBError; use crate::db::types::program::{CompilationProfile, ProgramConfig, ProgramInfo}; @@ -517,7 +532,7 @@ mod tests { // Too long. let too_long = "a".repeat(MAXIMUM_TAG_LENGTH + 1); assert!(matches!( - validate_tags(&[too_long.clone()]), + validate_tags(std::slice::from_ref(&too_long)), Err(DBError::InvalidTag { tag, .. }) if tag == too_long )); // Disallowed characters. diff --git a/crates/pipeline-manager/src/error.rs b/crates/pipeline-manager/src/error.rs index 01e365a2f3e..ce6ea61d903 100644 --- a/crates/pipeline-manager/src/error.rs +++ b/crates/pipeline-manager/src/error.rs @@ -27,7 +27,7 @@ use crate::compiler::error::CompilerError; use crate::db::error::DBError; use crate::runner::error::RunnerError; use actix_web::{ - body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, + HttpResponse, HttpResponseBuilder, ResponseError, body::BoxBody, http::StatusCode, }; use feldera_types::error::{DetailedError, ErrorResponse}; use openssl::error::ErrorStack; diff --git a/crates/pipeline-manager/src/events_cleaner.rs b/crates/pipeline-manager/src/events_cleaner.rs index 6dcfbbfe334..76e123bd571 100644 --- a/crates/pipeline-manager/src/events_cleaner.rs +++ b/crates/pipeline-manager/src/events_cleaner.rs @@ -26,7 +26,9 @@ pub async fn events_cleaner(db: Arc>, common_config: Comm { Ok(num_deleted) => { if num_deleted > 0 { - debug!("Pipeline monitor events cleanup: deleted {num_deleted} events that exceeded retention limits"); + debug!( + "Pipeline monitor events cleanup: deleted {num_deleted} events that exceeded retention limits" + ); } } Err(e) => { diff --git a/crates/pipeline-manager/src/lib.rs b/crates/pipeline-manager/src/lib.rs index c8e3a826553..fcb2e2b2b8b 100644 --- a/crates/pipeline-manager/src/lib.rs +++ b/crates/pipeline-manager/src/lib.rs @@ -35,7 +35,9 @@ pub fn platform_enable_unstable(requested_features: &str) { if let Some(supported_feature) = all_features.get(requested_feature) { enabled.insert(*supported_feature); } else { - warn!("Requested unstable feature '{requested_feature}' is not supported by the platform."); + warn!( + "Requested unstable feature '{requested_feature}' is not supported by the platform." + ); } } UNSTABLE_FEATURES diff --git a/crates/pipeline-manager/src/license.rs b/crates/pipeline-manager/src/license.rs index 3db8fdc7bb3..68e9f760acc 100644 --- a/crates/pipeline-manager/src/license.rs +++ b/crates/pipeline-manager/src/license.rs @@ -43,10 +43,10 @@ impl LicenseCheck { } }; - if let Some(license_check) = &mut license_check { - if let LicenseValidity::Exists(license_info) = &mut license_check.check_outcome { - license_info.current += license_check.checked_at.elapsed(); - } + if let Some(license_check) = &mut license_check + && let LicenseValidity::Exists(license_info) = &mut license_check.check_outcome + { + license_info.current += license_check.checked_at.elapsed(); } Ok(license_check) diff --git a/crates/pipeline-manager/src/logging.rs b/crates/pipeline-manager/src/logging.rs index 52749e711a7..d0878a1692a 100644 --- a/crates/pipeline-manager/src/logging.rs +++ b/crates/pipeline-manager/src/logging.rs @@ -1,6 +1,6 @@ use colored::ColoredString; use feldera_observability::json_logging::{ - init_pipeline_logging, init_service_logging as init_service_logging_subscriber, ServiceName, + ServiceName, init_pipeline_logging, init_service_logging as init_service_logging_subscriber, }; use tracing::warn; use tracing_subscriber::EnvFilter; diff --git a/crates/pipeline-manager/src/oidc/fetch.rs b/crates/pipeline-manager/src/oidc/fetch.rs index 3f93629c3bc..1fc04b40ff7 100644 --- a/crates/pipeline-manager/src/oidc/fetch.rs +++ b/crates/pipeline-manager/src/oidc/fetch.rs @@ -4,8 +4,8 @@ //! verify the token's signature, so the fetch happens on behalf of whoever //! presented it. -use crate::auth::{parse_rsa_jwks, AuthError}; -use crate::oidc::destination::{is_public_ip, validate_tenant_oidc_url, TenantIssuerPolicy}; +use crate::auth::{AuthError, parse_rsa_jwks}; +use crate::oidc::destination::{TenantIssuerPolicy, is_public_ip, validate_tenant_oidc_url}; use jsonwebtoken::DecodingKey; use serde::Deserialize; use serde_json::Value; diff --git a/crates/pipeline-manager/src/oidc/trust_name.rs b/crates/pipeline-manager/src/oidc/trust_name.rs index a1abfd5673d..249871c384d 100644 --- a/crates/pipeline-manager/src/oidc/trust_name.rs +++ b/crates/pipeline-manager/src/oidc/trust_name.rs @@ -1,7 +1,7 @@ use crate::db::error::DBError; use crate::db::types::utils::{ - validate_name, PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN, - PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN_DESCRIPTION, + PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN, + PATTERN_NON_EMPTY_ALPHANUMERIC_UNDERSCORE_HYPHEN_DESCRIPTION, validate_name, }; /// Longest permitted name for a trust relationship. diff --git a/crates/pipeline-manager/src/runner/error.rs b/crates/pipeline-manager/src/runner/error.rs index 3bbbe89fb76..45f82d6033c 100644 --- a/crates/pipeline-manager/src/runner/error.rs +++ b/crates/pipeline-manager/src/runner/error.rs @@ -1,7 +1,7 @@ use crate::db::types::resources_status::{ResourcesDesiredStatus, ResourcesStatus}; use crate::db::types::utils::ValidationError; use actix_web::{ - body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, + HttpResponse, HttpResponseBuilder, ResponseError, body::BoxBody, http::StatusCode, }; use feldera_types::error::{DetailedError, ErrorResponse}; use indoc::writedoc; @@ -319,7 +319,10 @@ impl Display for RunnerError { write!(f, "Pipeline provision failed: {error}") } Self::RunnerCheckError { error } => { - write!(f, "Pipeline check failed: compute and/or storage resources encountered a fatal error.\n\n{error}") + write!( + f, + "Pipeline check failed: compute and/or storage resources encountered a fatal error.\n\n{error}" + ) } Self::RunnerStopError { error } => { write!(f, "Pipeline stop failed (will retry): {error}") @@ -337,10 +340,16 @@ impl Display for RunnerError { ) } Self::RunnerInteractionLogFollowRequestChannelFull => { - write!(f, "Log follow request channel is full -- this indicates that the runner logging is overwhelmed") + write!( + f, + "Log follow request channel is full -- this indicates that the runner logging is overwhelmed" + ) } Self::RunnerInteractionLogFollowRequestChannelClosed => { - write!(f, "Log follow request channel is closed -- this indicates that the runner crashed unexpectedly") + write!( + f, + "Log follow request channel is closed -- this indicates that the runner crashed unexpectedly" + ) } Self::PipelineInteractionNotDeployed { pipeline_name, diff --git a/crates/pipeline-manager/src/runner/interaction.rs b/crates/pipeline-manager/src/runner/interaction.rs index 483188b4aa4..c37b541c2bf 100644 --- a/crates/pipeline-manager/src/runner/interaction.rs +++ b/crates/pipeline-manager/src/runner/interaction.rs @@ -7,7 +7,7 @@ use crate::db::types::tenant::TenantId; use crate::error::ManagerError; use crate::runner::error::RunnerError; use actix_web::http::header::{self, HeaderValue}; -use actix_web::{http::Method, web::Payload, HttpRequest, HttpResponse, HttpResponseBuilder}; +use actix_web::{HttpRequest, HttpResponse, HttpResponseBuilder, http::Method, web::Payload}; use actix_ws::{CloseCode, CloseReason}; use awc::error::{ConnectError, SendRequestError}; use awc::{ClientRequest, ClientResponse}; @@ -866,7 +866,7 @@ mod tests { /// connection), while a client offering none gets no header back. #[actix_web::test] async fn ws_handshake_echoes_offered_subprotocol() { - use actix_web::{web, App}; + use actix_web::{App, web}; setup(); // awc's TLS connector needs a rustls CryptoProvider installed. // Mirrors the production handshake: complete the upgrade, then echo. diff --git a/crates/pipeline-manager/src/runner/local_runner.rs b/crates/pipeline-manager/src/runner/local_runner.rs index 949deef9951..af7c58d5742 100644 --- a/crates/pipeline-manager/src/runner/local_runner.rs +++ b/crates/pipeline-manager/src/runner/local_runner.rs @@ -4,17 +4,17 @@ use crate::common_error::CommonError; use crate::config::{CommonConfig, LocalRunnerConfig}; use crate::db::types::pipeline::{ - bootstrap_policy_to_string, runtime_desired_status_to_string, PipelineId, + PipelineId, bootstrap_policy_to_string, runtime_desired_status_to_string, }; use crate::db::types::version::Version; -use crate::error::{source_error, ManagerError}; +use crate::error::{ManagerError, source_error}; use crate::pipeline_env::validate_pipeline_env; use crate::runner::error::RunnerError; use crate::runner::pipeline_executor::{PipelineExecutor, ProvisionStatus}; use crate::runner::pipeline_logs::{LogMessage, LogsSender}; use async_trait::async_trait; -use feldera_observability::system::total_memory_megabyte; use feldera_observability::ReqwestTracingExt; +use feldera_observability::system::total_memory_megabyte; use feldera_types::config::{ PipelineConfig, PipelineConfigProgramInfo, RuntimeConfig, StorageCacheConfig, StorageConfig, }; @@ -37,7 +37,7 @@ use tokio::task::JoinHandle; use tokio::time::{sleep, timeout}; use tokio::{fs, fs::create_dir_all, select, spawn}; use tokio_stream::StreamExt; -use tracing::{error, info, warn, Level}; +use tracing::{Level, error, info, warn}; use uuid::Uuid; /// How many times to attempt to retrieve the pipeline binary. @@ -1226,7 +1226,7 @@ impl PipelineExecutor for LocalRunner { return Err(RunnerError::RunnerProvisionError { error: format!("unable to spawn process due to: {e}"), } - .into()) + .into()); } } }; @@ -1413,14 +1413,14 @@ fn per_host_share(available_mb: Option, n_hosts: usize) -> Option { /// Sets `global.resources.memory_mb_max` to `available_mb` when the pipeline /// has no memory budget. fn apply_default_memory_limit(global: &mut RuntimeConfig, available_mb: Option) { - if global.effective_memory_mb().is_none() { - if let Some(available_mb) = available_mb { - info!( - "pipeline has no memory limit ('max_rss_mb' or 'resources.memory_mb_max'): \ + if global.effective_memory_mb().is_none() + && let Some(available_mb) = available_mb + { + info!( + "pipeline has no memory limit ('max_rss_mb' or 'resources.memory_mb_max'): \ defaulting 'resources.memory_mb_max' to {available_mb} MB" - ); - global.resources.memory_mb_max = Some(available_mb); - } + ); + global.resources.memory_mb_max = Some(available_mb); } } @@ -1441,8 +1441,8 @@ mod memory_limit_tests { #[cfg(test)] mod multihost_tests { use super::{ - multihost_coordinator_ip, multihost_host_ip, multihost_host_template, MAX_MULTIHOST_HOSTS, - MULTIHOST_LOOPBACK_OCTET, + MAX_MULTIHOST_HOSTS, MULTIHOST_LOOPBACK_OCTET, multihost_coordinator_ip, multihost_host_ip, + multihost_host_template, }; use std::net::Ipv4Addr; diff --git a/crates/pipeline-manager/src/runner/main.rs b/crates/pipeline-manager/src/runner/main.rs index 270ac4d8cb6..a3122ee4eab 100644 --- a/crates/pipeline-manager/src/runner/main.rs +++ b/crates/pipeline-manager/src/runner/main.rs @@ -13,7 +13,7 @@ use crate::runner::pipeline_executor::PipelineExecutor; use crate::runner::pipeline_logs::{LogMessage, LogsSender}; use actix_web::HttpResponse; use actix_web::Responder; -use actix_web::{get, web, HttpRequest, HttpServer}; +use actix_web::{HttpRequest, HttpServer, get, web}; use async_stream::try_stream; use std::collections::BTreeMap; use std::net::TcpListener; @@ -22,7 +22,7 @@ use std::sync::Arc; use std::time::Duration; use tokio::spawn; use tokio::sync::mpsc::error::TrySendError; -use tokio::sync::mpsc::{channel, Receiver, Sender}; +use tokio::sync::mpsc::{Receiver, Sender, channel}; use tokio::sync::{Mutex, Notify}; use tokio::task::JoinHandle; use tokio::time::timeout; @@ -107,22 +107,24 @@ async fn get_logs( .append_header(("X-Content-Type-Options", "nosniff")) .streaming(logs_stream(receiver).await)) } - Err(e) => { - match e { - TrySendError::Full(_) => { - error!("Unable to follow pipeline logs because the request channel is full"); - Err(ManagerError::from( - RunnerError::RunnerInteractionLogFollowRequestChannelFull, - )) - } - TrySendError::Closed(_) => { - error!("Unable to follow pipeline logs because the request channel is closed"); - Err(ManagerError::from( - RunnerError::RunnerInteractionLogFollowRequestChannelClosed, - )) - } + Err(e) => match e { + TrySendError::Full(_) => { + error!( + "Unable to follow pipeline logs because the request channel is full" + ); + Err(ManagerError::from( + RunnerError::RunnerInteractionLogFollowRequestChannelFull, + )) } - } + TrySendError::Closed(_) => { + error!( + "Unable to follow pipeline logs because the request channel is closed" + ); + Err(ManagerError::from( + RunnerError::RunnerInteractionLogFollowRequestChannelClosed, + )) + } + }, } } } @@ -249,7 +251,9 @@ async fn reconcile( } } None => { - error!("Runner main: listen notifier sending side has disconnected -- no longer able to send notifications"); + error!( + "Runner main: listen notifier sending side has disconnected -- no longer able to send notifications" + ); break; } } @@ -269,7 +273,9 @@ async fn reconcile( match db.lock().await.list_pipeline_ids_across_all_tenants().await { Ok(pipeline_ids) => { if db_error_previously { - info!("Runner main: again able to retrieve pipeline identifiers from the database. Any new pipelines will be retroactively detected."); + info!( + "Runner main: again able to retrieve pipeline identifiers from the database. Any new pipelines will be retroactively detected." + ); db_error_previously = false; } for (tenant_id, pipeline_id) in pipeline_ids { @@ -313,7 +319,9 @@ async fn reconcile( } } Err(e) => { - error!("Runner main: unable to retrieve pipeline identifiers from the database. Any new pipelines are not detected until again able to. Error: {e}"); + error!( + "Runner main: unable to retrieve pipeline identifiers from the database. Any new pipelines are not detected until again able to. Error: {e}" + ); db_error_previously = true; } } diff --git a/crates/pipeline-manager/src/runner/pipeline_automata.rs b/crates/pipeline-manager/src/runner/pipeline_automata.rs index e9e48e47a54..f72106b8dd0 100644 --- a/crates/pipeline-manager/src/runner/pipeline_automata.rs +++ b/crates/pipeline-manager/src/runner/pipeline_automata.rs @@ -3,10 +3,10 @@ use crate::db::error::DBError; use crate::db::storage::{ExtendedPipelineDescrRunner, Storage}; use crate::db::storage_postgres::StoragePostgres; use crate::db::types::pipeline::{ - runtime_desired_status_to_string, runtime_status_to_string, ExtendedPipelineDescr, - ExtendedPipelineDescrMonitoring, PipelineId, + ExtendedPipelineDescr, ExtendedPipelineDescrMonitoring, PipelineId, + runtime_desired_status_to_string, runtime_status_to_string, }; -use crate::db::types::program::{generate_pipeline_config, ProgramStatus}; +use crate::db::types::program::{ProgramStatus, generate_pipeline_config}; use crate::db::types::resources_status::{ResourcesDesiredStatus, ResourcesStatus}; use crate::db::types::storage::StorageStatus; use crate::db::types::tenant::TenantId; @@ -18,7 +18,7 @@ use crate::is_supported_runtime; use crate::runner::error::RunnerError; use crate::runner::interaction::{format_pipeline_url, format_timeout_error_message}; use crate::runner::pipeline_executor::{PipelineExecutor, ProvisionStatus}; -use crate::runner::pipeline_logs::{start_thread_pipeline_logs, LogMessage, LogsSender}; +use crate::runner::pipeline_logs::{LogMessage, LogsSender, start_thread_pipeline_logs}; use chrono::Utc; use feldera_observability::ReqwestTracingExt; use feldera_types::error::ErrorResponse; @@ -36,7 +36,7 @@ use tokio::task::JoinHandle; use tokio::time::Instant; use tokio::{sync::Mutex, time::Duration}; use tokio::{sync::Notify, time::timeout}; -use tracing::{debug, error, info, warn, Level}; +use tracing::{Level, debug, error, info, warn}; use uuid::Uuid; /// Every cycle, the automaton decides one of these actions to undertake. @@ -851,7 +851,10 @@ impl PipelineAutomaton { match serde_json::to_value(details) { Ok(storage_status_details) => Some(storage_status_details), Err(e) => { - error!("Automaton of pipeline {} is unable to serialize storage status details due to: {e}", self.pipeline_id); + error!( + "Automaton of pipeline {} is unable to serialize storage status details due to: {e}", + self.pipeline_id + ); None } } @@ -871,7 +874,10 @@ impl PipelineAutomaton { ProgramStatus::Success if !is_supported_runtime(&self.platform_version, &pipeline.platform_version) => { - info!("Runner cannot start pipeline {} because its runtime version ({}) is incompatible with current ({})", pipeline.id, pipeline.platform_version, self.platform_version); + info!( + "Runner cannot start pipeline {} because its runtime version ({}) is incompatible with current ({})", + pipeline.id, pipeline.platform_version, self.platform_version + ); Ok(Action::RemainStoppedUpdateError { error: ErrorResponse::from_error_nolog( @@ -1207,7 +1213,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(RunnerError::AutomatonMissingDeploymentInitial.into()), storage_status_details: None, - } + }; } Some(deployment_initial) => *deployment_initial, }; @@ -1218,7 +1224,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(RunnerError::AutomatonMissingDeploymentId.into()), storage_status_details: None, - } + }; } Some(deployment_id) => *deployment_id, }; @@ -1229,7 +1235,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(RunnerError::AutomatonMissingDeploymentConfig.into()), storage_status_details: None, - } + }; } Some(deployment_config) => match validate_deployment_config(deployment_config) { Ok(deployment_config) => deployment_config, @@ -1407,7 +1413,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(RunnerError::AutomatonMissingDeploymentInitial.into()), storage_status_details: None, - } + }; } Some(deployment_initial) => *deployment_initial, }; @@ -1658,7 +1664,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(e.into()), storage_status_details: None, - } + }; } }; @@ -1669,7 +1675,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(RunnerError::AutomatonMissingDeploymentLocation.into()), storage_status_details: None, - } + }; } }; @@ -1680,7 +1686,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(RunnerError::AutomatonMissingDeploymentInitial.into()), storage_status_details: None, - } + }; } }; @@ -1700,7 +1706,7 @@ impl PipelineAutomaton { return Action::TransitionToStopping { error: Some(e), storage_status_details: None, - } + }; } }; @@ -1876,11 +1882,11 @@ mod test { use std::str::FromStr; use std::sync::Arc; use std::time::Duration; - use tokio::sync::mpsc::{channel, Sender}; + use tokio::sync::mpsc::{Sender, channel}; use tokio::sync::{Mutex, Notify}; use uuid::Uuid; use wiremock::matchers::{method, path}; - use wiremock::{http, Mock, MockServer, ResponseTemplate}; + use wiremock::{Mock, MockServer, ResponseTemplate, http}; struct MockRunner { deployment_location: String, diff --git a/crates/pipeline-manager/src/runner/pipeline_logs.rs b/crates/pipeline-manager/src/runner/pipeline_logs.rs index 876eabc45ec..b5c943b564f 100644 --- a/crates/pipeline-manager/src/runner/pipeline_logs.rs +++ b/crates/pipeline-manager/src/runner/pipeline_logs.rs @@ -8,7 +8,7 @@ use tokio::sync::{mpsc, oneshot}; use tokio::task::JoinHandle; use tokio::time::interval; use tokio::{select, spawn}; -use tracing::{debug, error, warn, Level}; +use tracing::{Level, debug, error, warn}; // Logs buffer size limit constants. const LOGS_BUFFER_LIMIT_BYTE: usize = 1_000_000; // 1 MB @@ -171,7 +171,10 @@ async fn catch_up_and_add_follower( // First line mentions the number of discarded lines due to the circular buffer if logs.num_discarded_lines() > 0 { - let first_line = format!("... {} prior log lines were discarded due to buffer constraints and are thus not shown.", logs.num_discarded_lines()); + let first_line = format!( + "... {} prior log lines were discarded due to buffer constraints and are thus not shown.", + logs.num_discarded_lines() + ); // Tag as control-plane metadata so the notice is formatted (text or JSON) consistently with other runner messages. let formatted_notice = format_log_line(&LogMessage::new_from_control_plane( module_path!(), @@ -184,7 +187,9 @@ async fn catch_up_and_add_follower( if let Err(e) = new_follower.try_send(formatted_notice) { match e { TrySendError::Full(_) => { - error!("Unable to catch up new follower because buffer is full, the follower will be dropped"); + error!( + "Unable to catch up new follower because buffer is full, the follower will be dropped" + ); } TrySendError::Closed(_) => {} } @@ -198,7 +203,9 @@ async fn catch_up_and_add_follower( if let Err(e) = new_follower.try_send(line.clone()) { match e { TrySendError::Full(_) => { - error!("Unable to catch up new follower because buffer is full, the follower will be dropped") + error!( + "Unable to catch up new follower because buffer is full, the follower will be dropped" + ) } TrySendError::Closed(_) => {} } @@ -240,7 +247,9 @@ async fn process_log_line_with_followers( // There exists a buffer to give a follower the chance to catch up. // However, if the limit of the buffer is reached and thus unable to send new, // the log follower will be removed to prevent it from slowing down the rest. - error!("Unable to send log line to follower because buffer is full: the follower will be removed") + error!( + "Unable to send log line to follower because buffer is full: the follower will be removed" + ) } TrySendError::Closed(_) => {} }, @@ -344,12 +353,16 @@ impl LogsSender { SendTimeoutError::Timeout(unsent_message) => { warn!( "Unable to send logs message because receiver buffer is full -- trying again in {}ms (attempt {} / {})", - SEND_LOG_MESSAGE_TIMEOUT.as_millis(), i, SEND_LOG_MESSAGE_TRIES + SEND_LOG_MESSAGE_TIMEOUT.as_millis(), + i, + SEND_LOG_MESSAGE_TRIES ); unsent_message } SendTimeoutError::Closed(_) => { - debug!("Unable to send logs message because receiver is closed -- this can happen when the pipeline is deleted"); + debug!( + "Unable to send logs message because receiver is closed -- this can happen when the pipeline is deleted" + ); return; } }, diff --git a/crates/pipeline-manager/tests/logging_demo.rs b/crates/pipeline-manager/tests/logging_demo.rs index 4b6bd0bf3df..a59827e9b94 100644 --- a/crates/pipeline-manager/tests/logging_demo.rs +++ b/crates/pipeline-manager/tests/logging_demo.rs @@ -7,8 +7,8 @@ use tracing::info; #[test] fn emits_sample_log() { // When invoked as a subprocess, just emit the logs and exit so the parent can assert on output. - if let Ok(mode) = std::env::var("LOGGING_DEMO_CHILD") { - emit_logs(mode == "json"); + if std::env::var("LOGGING_DEMO_CHILD").is_ok() { + emit_logs(); return; } @@ -82,15 +82,9 @@ fn run_child(mode: &str) -> String { String::from_utf8(output.stdout).expect("child stdout to be utf-8") } -fn emit_logs(json: bool) { - if json { - std::env::set_var("FELDERA_LOG_JSON", "1"); - } else { - std::env::remove_var("FELDERA_LOG_JSON"); - } - // Force INFO output for the demo regardless of upstream defaults. - std::env::set_var("RUST_LOG", "info"); - std::env::set_var("NO_COLOR", "1"); +/// `run_child` passes the log format, `RUST_LOG` and `NO_COLOR` in the child's +/// environment, so this only has to initialize logging and emit. +fn emit_logs() { init_logging("[logging-demo]".cyan()); info!("logging demo event"); info!(