-
Notifications
You must be signed in to change notification settings - Fork 9
feat: cluster variants, ADBC health/reconcile, and distributed runnin… #93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,125 @@ | ||||||||||||||||||||||||
| use async_trait::async_trait; | ||||||||||||||||||||||||
| use queryflux_core::query::ClusterName; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| use super::introspection::AdbcIntrospection; | ||||||||||||||||||||||||
| use super::sql_helpers; | ||||||||||||||||||||||||
| use super::AdbcPool; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /// BigQuery introspection via `INFORMATION_SCHEMA.JOBS_BY_PROJECT` (metadata only, no slot cost). | ||||||||||||||||||||||||
| pub struct BigQueryIntrospection { | ||||||||||||||||||||||||
| pool: AdbcPool, | ||||||||||||||||||||||||
| project_id: String, | ||||||||||||||||||||||||
| region: Option<String>, | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| pub fn try_from_adbc_config( | ||||||||||||||||||||||||
| cluster_name: &ClusterName, | ||||||||||||||||||||||||
| uri: &str, | ||||||||||||||||||||||||
| db_kwargs: &[(String, String)], | ||||||||||||||||||||||||
| pool: AdbcPool, | ||||||||||||||||||||||||
| ) -> Option<BigQueryIntrospection> { | ||||||||||||||||||||||||
| let project_id = sql_helpers::db_kwarg(db_kwargs, "project_id") | ||||||||||||||||||||||||
| .or_else(|| sql_helpers::db_kwarg(db_kwargs, "project")) | ||||||||||||||||||||||||
| .or_else(|| uri.split('/').next().map(str::to_string)) | ||||||||||||||||||||||||
| .filter(|p| !p.is_empty())?; | ||||||||||||||||||||||||
|
Comment on lines
+15
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Look for real BigQuery ADBC URI examples/conventions used elsewhere in the repo (docs, tests, config samples).
rg -n -i "bigquery" --type=rust -C3 -g '!target' | rg -i "uri|://"
rg -n -i "bigquery" website/docs -C3Repository: lakeops-org/queryflux Length of output: 159 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== bigquery.rs =="
cat -n crates/queryflux-engine-adapters/src/adbc/bigquery.rs | sed -n '1,120p'
echo
echo "== repo search: BigQuery / ADBC / project_id / uri conventions =="
rg -n -i "bigquery|adbc|project_id|project=" crates website docs . \
-g '!target' -g '!node_modules' -g '!dist' -g '!build' \
| sed -n '1,220p'
echo
echo "== explicit URI token search =="
rg -n "://|bigquery://" . -g '!target' -g '!node_modules' -g '!dist' -g '!build' | sed -n '1,220p'Repository: lakeops-org/queryflux Length of output: 46260 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from urllib.parse import urlsplit
samples = [
"project/dataset",
"bigquery://project/dataset",
"bigquery:///project/dataset",
"https://example.com/x/y",
"scheme:project/dataset",
]
for s in samples:
print(s, "=> split('/')=", s.split('/')[0], "urlsplit=", urlsplit(s))
PYRepository: lakeops-org/queryflux Length of output: 885 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== call sites of try_from_adbc_config =="
rg -n "try_from_adbc_config\(" crates -g '!target' -g '!node_modules' | sed -n '1,80p'
echo
echo "== BigQuery docs around virtual fields / built-in introspection =="
sed -n '88,110p' website/docs/architecture/cluster-variants-and-health.md
echo
sed -n '176,205p' website/docs/architecture/cluster-variants-and-health.md
echo
echo "== remainder of bigquery.rs tests =="
sed -n '109,170p' crates/queryflux-engine-adapters/src/adbc/bigquery.rsRepository: lakeops-org/queryflux Length of output: 4544 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== bigquery adapter wiring =="
rg -n "BigQueryIntrospection|build_introspection|uri:" crates/queryflux-engine-adapters/src -C3 | sed -n '1,160p'Repository: lakeops-org/queryflux Length of output: 12852 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== call sites of try_from_adbc_config =="
rg -n "try_from_adbc_config\(" crates -g '!target' -g '!node_modules' | sed -n '1,80p'
echo
echo "== BigQuery docs around virtual fields / built-in introspection =="
sed -n '88,110p' website/docs/architecture/cluster-variants-and-health.md
echo
sed -n '176,205p' website/docs/architecture/cluster-variants-and-health.md
echo
echo "== remainder of bigquery.rs tests =="
sed -n '109,170p' crates/queryflux-engine-adapters/src/adbc/bigquery.rs
echo
echo "== bigquery adapter wiring =="
rg -n "BigQueryIntrospection|build_introspection|uri:" crates/queryflux-engine-adapters/src -C3 | sed -n '1,160p'Repository: lakeops-org/queryflux Length of output: 17238 🌐 Web query:
💡 Result: The Apache ADBC BigQuery driver uses the following URI format [1][2]: bigquery://[Host]:[Port]/[ProjectID]?[Key]=[Value]&[Key]=[Value]... Scheme: The required scheme is bigquery:// [1][2]. Host and Port: These are optional. The host defaults to bigquery.googleapis.com, and the port defaults to 443 [1][2]. Project ID: This is a required component of the URI path [1][2]. Dataset ID: While the Project ID is part of the URI path, the Dataset ID is typically provided as a query parameter (e.g.,?DatasetId=my_dataset) [1][2]. Examples: Using Application Default Credentials: bigquery:///my-project-123 Specifying a Dataset ID and Location: bigquery://bigquery.googleapis.com/my-project-123?OAuthType=0&DatasetId=analytics&Location=US [1][2] Notes: - Reserved characters in any URI element must be URI-encoded (e.g., @ becomes %40) [1][2]. - Additional configuration options can be passed as query parameters [1][2]. - For programmatic access, the Project ID and Dataset ID can also be set via driver-specific options such as adbc.bigquery.sql.project_id and adbc.bigquery.sql.dataset_id [3]. Citations:
Parse the BigQuery project ID from the URI path, not 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| let region = sql_helpers::db_kwarg(db_kwargs, "location") | ||||||||||||||||||||||||
| .or_else(|| sql_helpers::db_kwarg(db_kwargs, "region")); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| tracing::info!( | ||||||||||||||||||||||||
| cluster = %cluster_name.0, | ||||||||||||||||||||||||
| project_id = %project_id, | ||||||||||||||||||||||||
| region = ?region, | ||||||||||||||||||||||||
| "BigQuery introspection initialized (INFORMATION_SCHEMA.JOBS_BY_PROJECT)" | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Some(BigQueryIntrospection { | ||||||||||||||||||||||||
| pool, | ||||||||||||||||||||||||
| project_id, | ||||||||||||||||||||||||
| region, | ||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| impl BigQueryIntrospection { | ||||||||||||||||||||||||
| pub(crate) fn build_reconcile_sql(project_id: &str, region: Option<&str>) -> String { | ||||||||||||||||||||||||
| if let Some(region) = region { | ||||||||||||||||||||||||
| format!( | ||||||||||||||||||||||||
| "SELECT COUNT(*) FROM `{region}`.INFORMATION_SCHEMA.JOBS_BY_PROJECT \ | ||||||||||||||||||||||||
| WHERE state = 'RUNNING' AND project_id = '{}'", | ||||||||||||||||||||||||
| sql_helpers::escape_sql_literal(project_id) | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||
| format!( | ||||||||||||||||||||||||
| "SELECT COUNT(*) FROM `{project}`.INFORMATION_SCHEMA.JOBS_BY_PROJECT \ | ||||||||||||||||||||||||
| WHERE state = 'RUNNING'", | ||||||||||||||||||||||||
| project = project_id | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| fn reconcile_sql(&self) -> String { | ||||||||||||||||||||||||
| Self::build_reconcile_sql(&self.project_id, self.region.as_deref()) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[async_trait] | ||||||||||||||||||||||||
| impl AdbcIntrospection for BigQueryIntrospection { | ||||||||||||||||||||||||
| async fn health_check(&self) -> bool { | ||||||||||||||||||||||||
| // Metadata queries are cheap; no warehouse to wake. | ||||||||||||||||||||||||
| true | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+66
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Returning 🩺 Proposed fix: run a cheap metadata probe async fn health_check(&self) -> bool {
- // Metadata queries are cheap; no warehouse to wake.
- true
+ let pool = self.pool.clone();
+ let sql = format!("SELECT 1 FROM `{}`.INFORMATION_SCHEMA.SCHEMATA LIMIT 1", self.project_id);
+ tokio::task::spawn_blocking(move || sql_helpers::query_batches(&pool, &sql).is_some())
+ .await
+ .unwrap_or(false)
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| async fn fetch_running_query_count(&self) -> Option<u64> { | ||||||||||||||||||||||||
| let pool = self.pool.clone(); | ||||||||||||||||||||||||
| let sql = self.reconcile_sql(); | ||||||||||||||||||||||||
| tokio::task::spawn_blocking(move || { | ||||||||||||||||||||||||
| let batches = sql_helpers::query_batches(&pool, &sql)?; | ||||||||||||||||||||||||
| sql_helpers::first_cell_u64(&batches) | ||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||
| .await | ||||||||||||||||||||||||
| .ok() | ||||||||||||||||||||||||
| .flatten() | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[cfg(test)] | ||||||||||||||||||||||||
| mod tests { | ||||||||||||||||||||||||
| use super::*; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||
| fn reconcile_sql_without_region_uses_project_dataset() { | ||||||||||||||||||||||||
| let sql = BigQueryIntrospection::build_reconcile_sql("my-proj", None); | ||||||||||||||||||||||||
| assert!(sql.contains("`my-proj`.INFORMATION_SCHEMA.JOBS_BY_PROJECT")); | ||||||||||||||||||||||||
| assert!(sql.contains("state = 'RUNNING'")); | ||||||||||||||||||||||||
| assert!(!sql.contains("project_id =")); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||
| fn reconcile_sql_with_region_filters_project() { | ||||||||||||||||||||||||
| let sql = BigQueryIntrospection::build_reconcile_sql("my-proj", Some("region-us")); | ||||||||||||||||||||||||
| assert!(sql.contains("`region-us`.INFORMATION_SCHEMA.JOBS_BY_PROJECT")); | ||||||||||||||||||||||||
| assert!(sql.contains("project_id = 'my-proj'")); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||
| fn reconcile_sql_escapes_project_id_quotes() { | ||||||||||||||||||||||||
| let sql = BigQueryIntrospection::build_reconcile_sql("proj'x", Some("region-us")); | ||||||||||||||||||||||||
| assert!(sql.contains("project_id = 'proj''x'")); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||
| fn project_id_from_db_kwargs_or_uri() { | ||||||||||||||||||||||||
| let kwargs = vec![("project_id".into(), "from-kw".into())]; | ||||||||||||||||||||||||
| assert_eq!( | ||||||||||||||||||||||||
| sql_helpers::db_kwarg(&kwargs, "project_id").as_deref(), | ||||||||||||||||||||||||
| Some("from-kw") | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| assert_eq!( | ||||||||||||||||||||||||
| "my-proj/dataset" | ||||||||||||||||||||||||
| .split('/') | ||||||||||||||||||||||||
| .next() | ||||||||||||||||||||||||
| .map(str::to_string) | ||||||||||||||||||||||||
| .as_deref(), | ||||||||||||||||||||||||
| Some("my-proj") | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| use async_trait::async_trait; | ||
| use queryflux_core::query::ClusterName; | ||
|
|
||
| use super::introspection::AdbcIntrospection; | ||
|
|
||
| /// REST-based introspection for Databricks SQL Warehouses. | ||
| /// Avoids waking the warehouse via SQL `SELECT 1` or system tables. | ||
| pub struct DatabricksIntrospection { | ||
| http: reqwest::Client, | ||
| workspace_url: String, | ||
| warehouse_id: String, | ||
| auth_token: String, | ||
| } | ||
|
|
||
| pub fn try_from_adbc_config( | ||
| cluster_name: &ClusterName, | ||
| uri: &str, | ||
| db_kwargs: &[(String, String)], | ||
| ) -> Option<DatabricksIntrospection> { | ||
| let workspace_url = uri.trim_end_matches('/').to_string(); | ||
| if workspace_url.is_empty() { | ||
| tracing::warn!(cluster = %cluster_name.0, "Databricks REST: no workspace URL in ADBC URI"); | ||
| return None; | ||
| } | ||
|
|
||
| let find = |key: &str| { | ||
| db_kwargs | ||
| .iter() | ||
| .find(|(k, _)| k == key) | ||
| .map(|(_, v)| v.clone()) | ||
| }; | ||
|
|
||
| let auth_token = find("token") | ||
| .or_else(|| find("access_token")) | ||
| .or_else(|| find("databricks_token")) | ||
| .unwrap_or_default(); | ||
| if auth_token.is_empty() { | ||
| tracing::warn!(cluster = %cluster_name.0, "Databricks REST: no auth token found in dbKwargs"); | ||
| return None; | ||
| } | ||
|
|
||
| let http_path = find("http_path").unwrap_or_default(); | ||
| let warehouse_id = http_path.rsplit('/').next().unwrap_or("").to_string(); | ||
| if warehouse_id.is_empty() { | ||
| tracing::warn!(cluster = %cluster_name.0, "Databricks REST: cannot parse warehouse ID from http_path"); | ||
| return None; | ||
| } | ||
|
|
||
| tracing::info!( | ||
| cluster = %cluster_name.0, | ||
| workspace = %workspace_url, | ||
| warehouse_id = %warehouse_id, | ||
| "Databricks REST client initialized for health check and reconciliation" | ||
| ); | ||
|
|
||
| Some(DatabricksIntrospection { | ||
| http: reqwest::Client::new(), | ||
| workspace_url, | ||
| warehouse_id, | ||
| auth_token, | ||
| }) | ||
| } | ||
|
|
||
| #[async_trait] | ||
| impl AdbcIntrospection for DatabricksIntrospection { | ||
| /// Health check via `GET /api/2.0/sql/warehouses/{warehouse_id}`. | ||
| /// Healthy = state is RUNNING, STARTING, or STOPPED (warehouse exists and is accessible). | ||
| async fn health_check(&self) -> bool { | ||
| let url = format!( | ||
| "{}/api/2.0/sql/warehouses/{}", | ||
| self.workspace_url, self.warehouse_id | ||
| ); | ||
| match self | ||
| .http | ||
| .get(&url) | ||
| .bearer_auth(&self.auth_token) | ||
| .send() | ||
| .await | ||
| { | ||
| Ok(resp) if resp.status().is_success() => { | ||
| if let Ok(body) = resp.json::<serde_json::Value>().await { | ||
| let state = body.get("state").and_then(|v| v.as_str()).unwrap_or(""); | ||
| matches!(state, "RUNNING" | "STARTING" | "STOPPED") | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| Ok(resp) => { | ||
| tracing::warn!( | ||
| warehouse_id = %self.warehouse_id, | ||
| status = %resp.status(), | ||
| "Databricks REST health check failed" | ||
| ); | ||
| false | ||
| } | ||
| Err(e) => { | ||
| tracing::warn!( | ||
| warehouse_id = %self.warehouse_id, | ||
| error = %e, | ||
| "Databricks REST health check request error" | ||
| ); | ||
| false | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Reconciliation via `GET /api/2.0/sql/history/queries`. | ||
| /// Returns the count of RUNNING queries for this warehouse. | ||
| async fn fetch_running_query_count(&self) -> Option<u64> { | ||
| let url = format!("{}/api/2.0/sql/history/queries", self.workspace_url); | ||
| let filter = serde_json::json!({ | ||
| "filter_by": { | ||
| "statuses": ["RUNNING"], | ||
| "warehouse_ids": [&self.warehouse_id] | ||
| } | ||
| }); | ||
| match self | ||
| .http | ||
| .get(&url) | ||
| .bearer_auth(&self.auth_token) | ||
| .json(&filter) | ||
| .send() | ||
| .await | ||
| { | ||
| Ok(resp) if resp.status().is_success() => { | ||
| if let Ok(body) = resp.json::<serde_json::Value>().await { | ||
| body.get("res") | ||
| .and_then(|v| v.as_array()) | ||
| .map(|arr| arr.len() as u64) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| Ok(resp) => { | ||
| tracing::warn!( | ||
| warehouse_id = %self.warehouse_id, | ||
| status = %resp.status(), | ||
| "Databricks REST reconcile query failed" | ||
| ); | ||
| None | ||
| } | ||
| Err(e) => { | ||
| tracing::warn!( | ||
| warehouse_id = %self.warehouse_id, | ||
| error = %e, | ||
| "Databricks REST reconcile request error" | ||
| ); | ||
| None | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::adbc::sql_helpers; | ||
| use queryflux_core::query::ClusterName; | ||
|
|
||
| #[test] | ||
| fn extracts_warehouse_id_from_http_path() { | ||
| let kwargs = vec![( | ||
| "http_path".into(), | ||
| "/sql/1.0/warehouses/abc123def456".into(), | ||
| )]; | ||
| let path = sql_helpers::db_kwarg(&kwargs, "http_path").unwrap(); | ||
| let id = path.rsplit('/').next().unwrap(); | ||
| assert_eq!(id, "abc123def456"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn try_from_requires_token_and_http_path() { | ||
| let name = ClusterName("wh".into()); | ||
| assert!(try_from_adbc_config(&name, "https://dbc.example.com", &[],).is_none()); | ||
| assert!(try_from_adbc_config( | ||
| &name, | ||
| "https://dbc.example.com", | ||
| &[("token".into(), "tok".into())], | ||
| ) | ||
| .is_none()); | ||
| assert!(try_from_adbc_config( | ||
| &name, | ||
| "", | ||
| &[ | ||
| ("token".into(), "tok".into(),), | ||
| ("http_path".into(), "/sql/1.0/warehouses/id1".into(),) | ||
| ], | ||
| ) | ||
| .is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn try_from_accepts_token_and_http_path() { | ||
| let name = ClusterName("wh".into()); | ||
| let intro = try_from_adbc_config( | ||
| &name, | ||
| "https://dbc.example.com/", | ||
| &[ | ||
| ("token".into(), "tok".into()), | ||
| ("http_path".into(), "/sql/1.0/warehouses/wh-id".into()), | ||
| ], | ||
| ); | ||
| assert!(intro.is_some()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| use async_trait::async_trait; | ||
|
|
||
| /// Non-pool introspection for ADBC drivers that must not use default SQL health | ||
| /// checks or reconcile queries (Databricks REST, auto-suspending SaaS warehouses). | ||
| #[async_trait] | ||
| pub trait AdbcIntrospection: Send + Sync { | ||
| async fn health_check(&self) -> bool; | ||
| async fn fetch_running_query_count(&self) -> Option<u64>; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: lakeops-org/queryflux
Length of output: 11758
🏁 Script executed:
Repository: lakeops-org/queryflux
Length of output: 27024
🏁 Script executed:
Repository: lakeops-org/queryflux
Length of output: 4166
🏁 Script executed:
Repository: lakeops-org/queryflux
Length of output: 50377
Persist probe queries in the cluster JSONB.
UpsertClusterConfig::from_coredropshealth_check_queryandreconcile_query, even thoughcluster_config_from_persisted_jsonreadshealthCheckQueryandreconcileQueryback out. YAML-seeded clusters with custom probe SQL will lose that config on the first Postgres write and reload with defaults.🤖 Prompt for AI Agents