Skip to content

refactor: consolidate Capability into sketch_algebra; warm_tier_analysis becomes facade (Step 2a) #129

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.

Sign up for GitHub

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

Merged
zzylol merged 1 commit into from
May 11, 2026
Merged

refactor: consolidate Capability into sketch_algebra; warm_tier_analysis becomes facade (Step 2a) #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions asap-query-engine/src/drivers/ingest/otel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,15 @@ async fn route_modified_otlp_sketches_to_precompute(
| SketchKindHandle::CmsWithHeap => {
Capability::FrequencyTopk(kind)
}
// `Any` is the controller-side analysis-
// time wildcard — it should never appear
// on the ingest path (which detects a
// concrete sketch kind from the OTLP
// wire variant). Default defensively to
// QuantileApprox so a stray `Any`
// doesn't panic; the analyzer's
// `is_satisfied_by` rejects mismatches.
SketchKindHandle::Any => Capability::QuantileApprox(kind),
};
let group_by_keys: BTreeSet<String> =
dp.attrs.keys().cloned().collect();
Expand Down
8 changes: 5 additions & 3 deletions asap-query-engine/src/engines/simple/engine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3736,10 +3736,12 @@ impl crate::routing::engine_router::QueryEngine for SimpleEngine {
}

// Verify each sid carries the analyzer's required
// capability (the controller's
// `Capability::is_satisfied_by` honors `Any` semantics).
// capability. After Step 2a there's exactly one
// `Capability` enum (defined in the controller and
// re-exported by `sketch_index`), so no `From`
// conversion is needed — just clone.
let required: crate::stores::sketch_db::sketch_index::Capability =
candidate.required_capability.clone().into();
candidate.required_capability.clone();
let mut hit_sids: Vec<u64> = Vec::with_capacity(sids.len());
for sid in &sids {
match idx.classify(*sid) {
Expand Down
40 changes: 33 additions & 7 deletions asap-query-engine/src/engines/warm_tier/sketch_reducer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ impl WarmTierResult {
/// Family of sketch query the user's function maps onto. Determined
/// once per call so the per-sid loop doesn't re-string-match.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum QueryFamily {
pub(crate) enum QueryFamily {
Quantile,
Cardinality,
FrequencyTopk,
Expand All @@ -188,19 +188,45 @@ impl<'a> SketchReducer<'a> {
}

/// Map a PromQL function name to the warm-tier query family it
/// addresses. Returns `Err(UnsupportedFunction)` for anything
/// outside the dispatch table.
/// addresses. After the Step 2a refactor the canonical dispatch is
/// off the analyzer's `Capability` (see [`capability_to_family`]);
/// this string-based fallback exists ONLY for the
/// `SketchReducer::evaluate` entry point's `function_name: &str`
/// API surface, which is preserved for the existing call sites.
/// Unrecognised names route to the canonical family via the
/// downstream `require_capability` check.
fn function_to_family(function_name: &str) -> Result<QueryFamily, WarmTierError> {
match function_name {
"quantile_over_time" | "histogram_quantile" | "quantile" => Ok(QueryFamily::Quantile),
"count_distinct_over_time" | "cardinality_estimate" | "count_distinct" => {
Ok(QueryFamily::Cardinality)
}
// `distinct_over_time` (MetricsQL) is the canonical
// distinct-count-over-window name. `cardinality_estimate`
// and `count_distinct_over_time` are accepted as historical
// aliases for back-compat with PR #128's reducer tests; new
// callers should pass the analyzer's `required_capability`
// and dispatch via [`capability_to_family`] instead.
"distinct_over_time"
| "count_distinct_over_time"
| "cardinality_estimate"
| "count_distinct" => Ok(QueryFamily::Cardinality),
"topk" | "topk_over_time" | "bottomk" => Ok(QueryFamily::FrequencyTopk),
other => Err(WarmTierError::UnsupportedFunction(other.to_string())),
}
}

/// Map a [`Capability`] to a [`QueryFamily`]. This is the canonical
/// dispatch path after Step 2a: the controller's analyzer hands
/// each `WarmTierCandidate` a `required_capability`, and the
/// reducer picks a family without ever matching on the PromQL
/// function-name string.
#[allow(dead_code)]
pub(crate) fn capability_to_family(cap: &Capability) -> QueryFamily {
match cap {
Capability::QuantileApprox(_) => QueryFamily::Quantile,
Capability::CardinalityApprox => QueryFamily::Cardinality,
Capability::FrequencyTopk(_) => QueryFamily::FrequencyTopk,
}
}

/// Validate that a sid's capability is compatible with the
/// requested query family. Returns the `Capability` on match,
/// `Err(UnsupportedCapability)` on mismatch.
Expand Down Expand Up @@ -513,7 +539,7 @@ impl<'a> SketchReducer<'a> {
Ok(sk.estimate())
}
other => Err(WarmTierError::UnsupportedCapability {
function: "cardinality_estimate".to_string(),
function: "cardinality".to_string(),
capability: Capability::QuantileApprox(other),
}),
}
Expand Down
131 changes: 9 additions & 122 deletions asap-query-engine/src/stores/sketch_db/sketch_index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,129 +26,16 @@ use dashmap::DashMap;

use super::epoch_columnar::{LabelValuesId, SidStoreData, TimestampRange};

/// Capability the controller's plan made for this sketch instance.
/// Mirrors the design-doc Capability enum (§4.5). One Capability variant
/// per logical query family the warm tier can answer. The inner
/// `SketchKind` is the implementation choice (e.g. DDSketch vs KLL for
/// QuantileApprox); query routing keys on the variant, not the
/// implementation, so two CMS instances and one CountSketch instance
/// for the same metric-and-group-by all map to FrequencyTopk and the
/// query path picks any of them.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum Capability {
QuantileApprox(SketchKindHandle),
CardinalityApprox,
FrequencyTopk(SketchKindHandle),
// Sum / Rate / LastOverTime are answered from raw counter via
// Thanos forward; not represented as warm-tier capabilities.
}

/// Compact, hashable handle for sketch implementation choice.
/// Mirrors `controller::sketch_algebra::params::SketchKind` — duplicated
/// here as a thin enum so this module can be used independently of the
/// controller's full sketch algebra. The wire-format pdata variant tag
/// (`Metric.data_case`) maps 1:1 onto these handles at ingest time.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum SketchKindHandle {
DDSketch,
Kll,
Hll,
CountSketch,
CountMin,
/// CMS-with-heap. Detected at the application level via the
/// precompute_operators `count_min_sketch_with_heap_accumulator`
/// flow — the OTLP `CountMinSketch` wire struct doesn't carry the
/// heap natively, so the gateway/precompute layer marks the
/// sid with this variant when the parent container's heap field
/// is non-empty. The warm-tier reducer reads the heap directly
/// when answering `topk` / `topk_over_time`.
CmsWithHeap,
}

// ── Controller ↔ backend Capability adapters ─────────────────────────────────
//
// `controller::warm_tier_analysis::Capability` is the canonical
// PromQL-shape-recognition output (the controller is the single owner
// of "is this PromQL warm-tier-answerable" knowledge — see
// `controller/src/warm_tier_analysis.rs`). The backend's `Capability`
// here is the source-of-truth for INDEXED sketch instances (driven by
// the OTLP receive path). The two enums are kept structurally
// identical and the backend adapts at the boundary so the controller
// crate stays free of any backend dep.
// ── Capability re-exports ────────────────────────────────────────────────────
//
// `SketchKindHandle::Any` from the controller side maps to ALL
// concrete handles when used in capability matching — the backend's
// `is_compatible_with` helper consumes that semantics so callers don't
// need to enumerate the cross product.

impl From<controller::warm_tier_analysis::SketchKindHandle> for SketchKindHandle {
fn from(h: controller::warm_tier_analysis::SketchKindHandle) -> Self {
use controller::warm_tier_analysis::SketchKindHandle as C;
match h {
C::DDSketch => SketchKindHandle::DDSketch,
C::Kll => SketchKindHandle::Kll,
C::Hll => SketchKindHandle::Hll,
C::CountSketch => SketchKindHandle::CountSketch,
C::CountMin => SketchKindHandle::CountMin,
C::CmsWithHeap => SketchKindHandle::CmsWithHeap,
// `Any` has no single concrete handle. Callers that need
// to match against a specific instance should use
// `Capability::is_satisfied_by` instead of `From` for the
// handle directly. Defensive default: return DDSketch (the
// QuantileApprox catalog default) so a stray `Any` doesn't
// panic, though the canonical flow goes through
// `Capability::is_satisfied_by`.
C::Any => SketchKindHandle::DDSketch,
}
}
}

impl Capability {
/// True when the indexed sketch instance's capability satisfies
/// the controller-side required capability. The controller emits
/// `SketchKindHandle::Any` to mean "any implementation in the
/// family is acceptable" (e.g. QuantileApprox(Any) is satisfied
/// by both DDSketch and KLL); concrete handles must match
/// exactly.
pub fn is_satisfied_by(&self, indexed: &Capability) -> bool {
use controller::warm_tier_analysis::SketchKindHandle as Any;
let _ = Any::Any; // silence unused-import warning when compiled standalone
match (self, indexed) {
(Capability::QuantileApprox(_), Capability::QuantileApprox(_)) => true,
(Capability::CardinalityApprox, Capability::CardinalityApprox) => true,
// FrequencyTopk(CmsWithHeap) is the only sub-variant the
// warm tier can answer top-k against (CountMin /
// CountSketch carry no heap). Match exactly on the inner
// handle so the reducer's MissingHeap path stays
// accessible.
(Capability::FrequencyTopk(req), Capability::FrequencyTopk(have))
if req == have =>
{
true
}
_ => false,
}
}
}

/// Adapt a controller-side analyzed Capability into the backend
/// `Capability` enum. Used by the engine warm-tier hook to compare
/// the analyzer's required-capability against the index's recorded
/// per-instance capability. `SketchKindHandle::Any` from the
/// controller side is preserved as the catalog default for the
/// outer family (DDSketch for QuantileApprox); call sites that need
/// the "matches any concrete impl" semantic should call
/// [`Capability::is_satisfied_by`] instead.
impl From<controller::warm_tier_analysis::Capability> for Capability {
fn from(c: controller::warm_tier_analysis::Capability) -> Self {
use controller::warm_tier_analysis::Capability as C;
match c {
C::QuantileApprox(h) => Capability::QuantileApprox(h.into()),
C::CardinalityApprox => Capability::CardinalityApprox,
C::FrequencyTopk(h) => Capability::FrequencyTopk(h.into()),
}
}
}
// Step 2a consolidated all capability state into
// `controller::sketch_algebra::capability`. The backend no longer
// defines its own `Capability` / `SketchKindHandle`; it re-exports the
// canonical types so there's exactly one definition in the codebase.
// `is_satisfied_by` (used by the engine warm-tier hook) now lives on
// the controller-side `Capability` impl.

pub use controller::sketch_algebra::{Capability, SketchKindHandle};

/// Sketch-instance configuration carried per-Metric on the OTLP wire
/// (Phase 2 lifted these from per-DP up to the parent sketch container).
Expand Down
Loading