fix(vortex-array): restore load_full + HashMap import dropped in spiceai-54 rebase#70
Merged
Merged
Conversation
The spiceai-54 rebase onto the 0.75.0 base (e09ec32) dropped two symbols still used by the per-ExecutionCtx ArrayKernels snapshot path: - ArcSwapMap::load_full (arc_swap_map.rs), called by ArrayKernels::snapshot() - the `vortex_utils::aliases::hash_map::HashMap` import in kernels.rs, used by the KernelSnapshot struct field Without them vortex-array fails to compile (E0599 load_full not found, E0425 HashMap not in scope). executor.rs still consumes snapshot()/ KernelSnapshot, so the correct fix is to restore the symbols (byte-identical to the prior good commit 729249d), not to remove the usages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR restores two symbols that were inadvertently dropped during the spiceai-54 rebase, unbreaking vortex-array compilation when the optimizer is enabled. It reintroduces ArcSwapMap::load_full() (used by ArrayKernels::snapshot()) and the HashMap alias import needed by KernelSnapshot.
Changes:
- Re-add
use vortex_utils::aliases::hash_map::HashMap;tovortex-array/src/optimizer/kernels.rssoKernelSnapshot’s stored map type resolves. - Re-add
ArcSwapMap::load_full()tovortex-array/src/arc_swap_map.rs, delegating toArcSwap::load_full()to return an ownedArcsnapshot.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vortex-array/src/optimizer/kernels.rs | Restores HashMap import required for KernelSnapshot’s Arc<HashMap<...>> field type. |
| vortex-array/src/arc_swap_map.rs | Restores load_full() used by ArrayKernels::snapshot() to capture an owned Arc snapshot without copying the map. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pull Bot
pushed a commit
to TheRakeshPurohit/spiceai
that referenced
this pull request
Jun 26, 2026
…ode fix) (spiceai#11466) Bumps vortex/vortex-* from 729249dd to ab4f0b17 (spiceai-54 HEAD), bringing in "Fix flat reader subrange decode reuse" plus the rebase of the spiceai-54 perf work (intra-file decode parallelism, per-ExecutionCtx ArrayKernels snapshot) onto the Vortex 0.75.0 base. Net source delta vs the prior pin is internal-only (flat/reader.rs, arc_swap_map.rs, optimizer/kernels.rs) with no public API change, so no Spice-side code changes are required. The spiceai-54 tip (e09ec32c) was briefly un-buildable — the rebase dropped ArcSwapMap::load_full and a HashMap import still used by the kernel snapshot; restored upstream in spiceai/vortex#70, folded into the ab4f0b17 HEAD pinned here. Verified: cargo check -p cayenne -p vortex-datafusion clean; Cargo.lock regenerated (git rev only, dependency tree unchanged). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions Bot
pushed a commit
to spiceai/spiceai
that referenced
this pull request
Jun 26, 2026
…ode fix) (#11466) Bumps vortex/vortex-* from 729249dd to ab4f0b17 (spiceai-54 HEAD), bringing in "Fix flat reader subrange decode reuse" plus the rebase of the spiceai-54 perf work (intra-file decode parallelism, per-ExecutionCtx ArrayKernels snapshot) onto the Vortex 0.75.0 base. Net source delta vs the prior pin is internal-only (flat/reader.rs, arc_swap_map.rs, optimizer/kernels.rs) with no public API change, so no Spice-side code changes are required. The spiceai-54 tip (e09ec32c) was briefly un-buildable — the rebase dropped ArcSwapMap::load_full and a HashMap import still used by the kernel snapshot; restored upstream in spiceai/vortex#70, folded into the ab4f0b17 HEAD pinned here. Verified: cargo check -p cayenne -p vortex-datafusion clean; Cargo.lock regenerated (git rev only, dependency tree unchanged). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lukekim
added a commit
to spiceai/spiceai
that referenced
this pull request
Jun 26, 2026
…ode fix) (#11466) Bumps vortex/vortex-* from 729249dd to ab4f0b17 (spiceai-54 HEAD), bringing in "Fix flat reader subrange decode reuse" plus the rebase of the spiceai-54 perf work (intra-file decode parallelism, per-ExecutionCtx ArrayKernels snapshot) onto the Vortex 0.75.0 base. Net source delta vs the prior pin is internal-only (flat/reader.rs, arc_swap_map.rs, optimizer/kernels.rs) with no public API change, so no Spice-side code changes are required. The spiceai-54 tip (e09ec32c) was briefly un-buildable — the rebase dropped ArcSwapMap::load_full and a HashMap import still used by the kernel snapshot; restored upstream in spiceai/vortex#70, folded into the ab4f0b17 HEAD pinned here. Verified: cargo check -p cayenne -p vortex-datafusion clean; Cargo.lock regenerated (git rev only, dependency tree unchanged). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pull Bot
pushed a commit
to TheRakeshPurohit/spiceai
that referenced
this pull request
Jun 28, 2026
… IMDS, I/O-cliff fast path, infeasible-SLO feedback (spiceai#11463) * feat(cayenne): storage-aware adaptive CDC tuning — calibration probe, IMDS, I/O-cliff fast path, infeasible-SLO feedback Make the closed-loop CDC auto-tuner environment- and EBS-aware. The tuner previously classified storage by device identity (a 4-value enum) and reacted to I/O latency with a slow additive crawl; this measures real storage performance, reacts decisively to burst-credit cliffs, and surfaces when an SLO is infeasible on the hardware. Environment detection - Calibration probe at registration (off-runtime, memoized per volume, fail-open) measures real write throughput, so the slow-tier bias is continuous instead of a device-identity class — and it is the only storage signal a cdc_durability:memory table that never spills produces. Cloud-agnostic. - EC2 instance probe via aws_config::imds (tight timeout, opt out with SPICE_DISABLE_IMDS): T-family burstable CPU + EBS-optimized baseline bandwidth. - Wire the detected storage class + measured throughput onto the Cayenne config (was left at Unknown, so the loop applied the EBS bias even on local NVMe). Closed-loop control - Continuous slow-tier bias (tier_scale): a fast io2 volume no longer gets the same commit-amortization pressure as slow gp3. - I/O-cliff fast path: a fast/slow latency-EWMA divergence (EBS burst-credit depletion / instance EBS pipe saturating) triggers one decisive multiplicative backoff instead of an additive crawl — the I/O analogue of the critical-memory fast path; applies in both legacy and goal modes. - Infeasible-SLO feedback: when a goal stays violated with every relevant lever clamped, warn once naming the binding constraint and latch a telemetry gauge. - Earlier/smaller drain on a confirmed slow tier (EBS/measured-slow; Unknown keeps the standard thresholds). Burstable-CPU gate withholds CPU-stealing moves sooner. Resource coordination - Process-global encode budget bounded by the instance EBS write bandwidth. - Conservative floating mem-tier ceiling for query-light deployments (host/8 -> host/6), preserving the spiceai#11449 no-overcommit invariant exactly. - Low-free-space startup warning on the data/spill volume. Telemetry gauges for measured throughput + goal-infeasible. Unit tests across all of the above; scoped cargo check, targeted tests (cayenne tuning 73/0, runtime storage/imds/autotune/builder 54/0), and scoped clippy are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: cargo fmt Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review — per-volume probe cache key + imds intra-doc link - storage.rs: extract a shared probe_dir() and key the calibration-probe cache on the canonicalized *probed directory* (the parent when handed a file path), so two files on the same volume dedupe to a single probe (the cache key now matches the dir actually probed). - imds.rs: link the in-scope [`Client`] in the module docs instead of `aws_config::imds::Client`, so the intra-doc link resolves (no rustdoc warning). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review round 2 (Copilot) - autotune: ebs_upload_concurrency_cap floors (not rounds) the stream count, so a cap never rounds up past the bandwidth ceiling. - storage: probe cache stores a per-key shared OnceCell so concurrent registrations on one volume coalesce on a single probe (no duplicate 8 MiB writes). - cayenne/mod: warn_if_low_disk is async and runs the canonicalize + mount enumeration via spawn_blocking (off the Tokio worker); dedup key is canonicalized so equivalent paths (trailing slash, symlinks) collapse to one check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review round 3 (Copilot) - write_budget: make cap_global_encode_concurrency atomic — check-and-replace under a single write lock (re-read total, replace only while it still exceeds the cap), so concurrent caps can't race and 'tightest wins, never raises' holds. - storage: move probe_dir (is_dir) + canonicalize into spawn_blocking, so probe_storage_perf_async does no blocking filesystem I/O on the runtime thread. - docs: correct the infeasible-SLO signal wording — it is intentionally NOT a permanent latch; the gauge reflects current state and self-clears if the SLO becomes reachable again, and the warning fires once per infeasible episode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): CI Rust Lint (pedantic doc_markdown) + PR review round 4 - Backtick doc-comment identifiers flagged by clippy::doc_markdown (-Dpedantic): NVMe (tuning.rs, autotune.rs) and IMDSv2/IMDSv1 (imds.rs) — the Rust Lint failure. - storage: move static PROBE_SEQ before statements (clippy::items_after_statements); open the probe temp file with create_new (O_EXCL) so it never truncates/clobbers a pre-existing or attacker-planted file (fails open instead). - cayenne/mod: correct warn_if_low_disk docs — the dedup key is the canonicalized path, not the mount, so 'once per canonicalized path' (distinct dirs on the same mount each warn once). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): CI Rust Lint on test code (--tests pedantic) The CI lint runs clippy --tests too (not just --lib): fix the two test-code lints it caught: - tuning.rs: use .expect() instead of .unwrap() in a test (clippy::unwrap_used; tests allow expect_used but not unwrap_used). - datafusion/builder.rs: drop a '+ 0' in the no-overcommit assertion (identity_op). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review — binding_constraint message accuracy - 'memory-bound' no longer claims buffers are 'shrinking' (shrinking only starts at MEM_PRESSURE_HIGH); reworded to 'at/over the RAM budget — can't grow buffers'. - Reserve the 'EBS' wording for the confirmed Ebs class; a slow/undetected (Unknown) tier now reports 'slow or undetected tier' instead of misnaming it EBS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(deps): bump vortex to spiceai-54 HEAD (flat reader subrange decode fix) (spiceai#11466) Bumps vortex/vortex-* from 729249dd to ab4f0b17 (spiceai-54 HEAD), bringing in "Fix flat reader subrange decode reuse" plus the rebase of the spiceai-54 perf work (intra-file decode parallelism, per-ExecutionCtx ArrayKernels snapshot) onto the Vortex 0.75.0 base. Net source delta vs the prior pin is internal-only (flat/reader.rs, arc_swap_map.rs, optimizer/kernels.rs) with no public API change, so no Spice-side code changes are required. The spiceai-54 tip (e09ec32c) was briefly un-buildable — the rebase dropped ArcSwapMap::load_full and a HashMap import still used by the kernel snapshot; restored upstream in spiceai/vortex#70, folded into the ab4f0b17 HEAD pinned here. Verified: cargo check -p cayenne -p vortex-datafusion clean; Cargo.lock regenerated (git rev only, dependency tree unchanged). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cluster): route Ballista shuffle/temp to the data PVC (spiceai#11454) * fix(cluster): route Ballista shuffle/temp to the data PVC Cluster-bench executors wrote Ballista shuffle output to the default work_dir (env::temp_dir() = /tmp), a small EmptyDir whose sizeLimit a SF10+ shuffle exceeds, getting the pod evicted mid-query and livelocking the run. The dispatch workflow already exports SPIDAPTER_SHUFFLE_LOCATION and SPIDAPTER_QUERY_TEMP_DIRECTORY (pointing at the executor's /data PVC) but nothing consumed them. - spidapter: inject SPIDAPTER_SHUFFLE_LOCATION into runtime.params and SPIDAPTER_QUERY_TEMP_DIRECTORY into runtime.query.temp_directory when generating the cluster app spicepod (mirrors SCHEDULER_STATE_LOCATION). - runtime: create the configured shuffle work_dir (create_dir_all) instead of only warning, since Ballista uses work_dir as-is without creating it. * style: rustfmt the spidapter shuffle-location injection * fix(cluster): drive Flight TLS handshakes concurrently (fix shuffle-fetch connection resets) (spiceai#11456) * fix(cluster): drive TLS handshakes concurrently to avoid shuffle-fetch resets The Flight server's tls_incoming accepted one connection, awaited its TLS handshake inline, then accepted the next — serializing acceptance. Under a distributed-join shuffle (each reducer opens up to 64 fetch connections per peer, no client-side pooling) the listener accept queue overflowed and peers saw 'connection reset by peer', failing every multi-stage query while single-stage queries (no shuffle) passed. Drive up to 1024 handshakes concurrently via buffer_unordered so acceptance keeps draining the backlog. * style: rustfmt the spidapter shuffle-location injection * fix(search): default Elasticsearch kNN candidate pool to 1000 instead of 10 (fixes spiceai#11264) (spiceai#11467) * fix(search): default Elasticsearch kNN candidate pool to 1000 instead of 10 (fixes spiceai#11264) The Elasticsearch vector index hardcoded the kNN candidate pool to k=10 in query_table_provider. The search pipeline places Filter/Sort above the index scan, so DataFusion cannot push the requested top-K below them and the scan receives limit=None — silently capping every filtered/sorted vector search at 10 hits regardless of the requested limit. Use a shared 1000 default matching the sibling DuckDB vector backend (DEFAULT_DUCKDB_VECTOR_SEARCH_LIMIT). Kept at 1000 because the kNN exec requests num_candidates = k*2 and Elasticsearch rejects num_candidates above 10,000, so a larger default would fail the search request outright. * fix(search): simplify ES kNN default to a named constant Replace the regression test and compile-time assertions with a plain DEFAULT_ELASTICSEARCH_VECTOR_SEARCH_LIMIT = 1000 constant used as the kNN candidate-pool fallback, matching the DuckDB vector backend. --------- Co-authored-by: Jeadie <jeadie@users.noreply.github.com> * feat(acceleration): add on_schema_change drop_and_recreate policy (spiceai#11462) * feat(acceleration): add on_schema_change drop_and_recreate policy Widening schema evolution (append_new_columns/sync_all_columns, spiceai#11261) adopts lossless changes in place but leaves non-widening changes block-equivalent. This adds the `drop_and_recreate` policy that recreates the accelerated table when a change cannot be applied in place (dropped columns, narrowing/incompatible type changes, new non-nullable columns) -- but only under refresh_mode: full on a drop-capable engine (duckdb/sqlite/turso/cayenne), where a full refresh re-fetches every row so the drop is lossless. Widening still applies in place; in append/changes modes an incompatible change is rejected and the table preserved. A single recreates_on_schema_mismatch() helper is the source of truth for the recreate decision, consulted by handle_schema_difference and by both the initial-load and reload schema-mismatch gates so they cannot drift. Reuses the existing snapshot -> drop_table -> clear-checkpoint machinery. Adds schema_evolution_applied{action=recreate} metric + task_history events for the evolution lifecycle. Unit tests + CSV integration tests (duckdb/sqlite/cayenne). Closes spiceai#10008 * fix: drop now-unused Mode import in init/dataset.rs The file_update/drop_and_recreate gate now goes through schema_evolution::recreates_on_schema_mismatch, so the direct Mode::FileUpdate comparison (and its import) are no longer needed. * fix: address PR review comments + rustfmt - Recreate path now clears cached logical plans (clear_cached_plans), matching the in-place evolution path, so stale plans don't survive a destructive schema change. - Clarify recreates_on_schema_mismatch doc + the init/dataset.rs gate comment to reflect the exact conditions (file_update requires refreshes enabled; drop_and_recreate requires refresh_mode: full on a recreate-capable engine). - rustfmt. * fix: DuckDB drop_table handles view layout; backtick refresh_mode in doc - DuckDB drop_table now dispatches on the object type (view vs base table) and drops every internal __data_{name}_{ts} table, so drop_and_recreate works in mode: file (full refresh) where DuckDB stores the accelerated object as a view. Previously DROP TABLE errored ('object is of type View'). Reuses the same layout helpers as evolve_table_schema; the file_update base-table path is unchanged. - Backtick `refresh_mode` in the DropAndRecreate doc (clippy::doc_markdown) and re-sync the generated spicepod.schema.json description. * fix(clippy): merge identical drop_and_recreate/sync_all_columns match arm clippy::match_same_arms — the CDC SchemaEvolutionPolicy mapping had separate SyncAllColumns and DropAndRecreate arms with identical bodies; merge them into one or-pattern (mirrors the cayenne mapping). * refactor: engine_supports_recreate delegates to engine_supports_in_place_evolution Avoids duplicating the engine match list (they are the same set today: the four engines with a real drop_table are exactly those with evolve_table_schema). Single source of truth; doc notes to split if a future engine gains one capability but not the other. * fix: align recreate metric/event action label; correct Phase 2 test comment - task_history recreate event now uses action="recreate", matching the SCHEMA_EVOLUTION_APPLIED metric label, so metrics and task_history rows correlate. - Reword the drop_and_recreate Phase 2 comment: the assertion checks the observable outcome (column added, rows preserved); in-place-vs-recreate is unit-tested, not asserted here. * feat(cayenne): predicate-aware maintained aggregates serve filtered analytical queries from the CDC delta (spiceai#11458) * feat(cayenne): predicate-aware maintained aggregates serve filtered analytical queries from the CDC delta The maintained-aggregate registry (spiceai#11389) serves grouped SUM/COUNT/AVG in O(groups) from the CDC delta, but only when the plan is a clean unfiltered aggregate directly over the Cayenne scan: a FilterExec between the AggregateExec and the scan defeated the optimizer rewrite, so every analytical query with a WHERE fell back to an O(rows) re-scan. This makes the maintained view predicate-aware. It maintains the aggregate over only the rows its filter selects, and the optimizer serves a query carrying the identical predicate from that state. A non-matching row is treated as absent (not indexed, not accumulated), so all retraction logic is reused unchanged; an UPDATE that flips a row's filter status always retracts the prior contribution first. Exact and deletion-safe via the existing per-PK contribution index (not a sketch). The serve gate requires the query's filter to equal the view's exactly, so a filtered view never answers an unfiltered query, and vice versa. - maintained_aggregate.rs: optional filter on the spec and view; mask-aware apply_insert_batch; filter-equality in the serve match; batch_for_spec serve hook; filter threaded through batch_for_aggregate_with_output. - optimizer_rules.rs: descend a single FilterExec during plan walk and capture its predicate for the serve. - tests/maintained_aggregate_filter_test.rs: full CDC-lifecycle correctness (insert / update-out-of-predicate / update-into-predicate / delete vs a from-scratch filtered recompute) plus the serve-gate guard; optimizer routing tests cover the positive (filtered query routes) and negative (unfiltered query over a filtered view is refused) paths. - benches: vs_duckdb_ and vs_chdb_maintained_filtered_groupby as separate binaries (the two bundled engines abort if driven in one process). Measured on the q1/q6 shape (~16 groups, result asserted equal before timing): Cayenne serve is flat ~3.1 us (O(groups)); DuckDB 382 us -> 639 us and chDB 2.04 ms -> 2.36 ms across 100k -> 1M rows (O(rows)). That is 117x/205x faster than DuckDB and 634x/740x faster than chDB, and the gap widens with table size. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(cayenne): user-definable filter on maintained_aggregates config (fire filtered queries end-to-end) Adds an optional `filter` SQL predicate to the spicepod `maintained_aggregates` config and parses it into a physical expression over the dataset schema, so a configured maintained view can be declared WITH a `WHERE` and serve a live filtered analytical query (e.g. CH-benCH q1/q6) from the incrementally-maintained state — the end-to-end enabler for the predicate-aware serving added earlier in this PR. - spicepod: `MaintainedAggregate.filter: Option<String>` (serde-default, so the config stays backward compatible) + deserialize test. - runtime: thread the table schema into `maintained_aggregate_specs_for_cayenne` and parse the filter via `parse_sql_expr` + `create_physical_expr` over the schema (so the predicate's column indices match both the CDC batches the view is maintained from and a filtered query's `FilterExec`); a parse/bind/plan failure surfaces as an `InvalidConfiguration` error naming the filter. - tests: filter round-trips from YAML; config filter parses onto the spec; a filter referencing an unknown column is rejected. Also merges origin/trunk (incl. the Cayenne provider type-move refactor) into the branch; the maintained-aggregate lever recompiles cleanly against it. Note: full auto-derivation of the filter from the observed recurring workload is a larger follow-up; this lands the user-definable path, which is the concrete piece that makes a configured q1/q6 serve from the maintained view. * refactor(cayenne): simplify filtered maintained-aggregate apply, share the bench fixture, document the serve-match boundary /simplify cleanup of the predicate-aware maintained-aggregate change; no behavior change. - maintained_aggregate.rs: collapse `apply_insert_batch`'s indexed/unindexed branches into one shared tail — compute the PK once, then a single match-gate + group insert — instead of duplicating the post-filter path (`continue`, group-key, insert) in both arms. - Document the serve-match boundary on `matches_query` and the runtime filter parser: the match is structural `PhysicalExpr` equality over the scan-output schema, so it silently does NOT fire across a projection / cast / type-coercion between the scan and the filter (e.g. `SchemaCastScanExec`, or `Utf8View`-over-`Utf8`) and falls back to a re-scan. Name-based predicate normalization is the follow-up. - benches: extract the duplicated Cayenne-side fixture (schema, spec, row generator, registry load, serve decode) into `maintained_filtered_helpers/common.rs`, shared by the vs_duckdb and vs_chdb benches via the established `#[path] mod` pattern, so the registry/spec contract can't drift between the two split binaries. Validated: `cargo test -p cayenne maintained_aggregate` (20 lib + 2 integration tests) green; both maintained-filtered benches compile under duckdb-bench/chdb-bench; fmt clean. * test(chbench): add IVM (predicate-aware maintained-aggregate) SF-1000 dispatch pod New accelerated spicepod postgres-cayenne[file]-cdc-tuned-memory-ivm-sf1000.yaml: the SF-1000 cdc-tuned MEMORY (in-memory tier) baseline plus maintained_aggregates on order_line — the heaviest CH-benCH table — for BOTH of its single-table analytical queries: - q1: SUM/AVG(ol_quantity, ol_amount) + COUNT GROUP BY ol_number, WHERE ol_delivery_d > '2007-01-02'. - q6: global SUM(ol_amount) WHERE ol_delivery_d in a window AND ol_quantity BETWEEN 1 AND 100000. Each view's group-by, aggregate set/order, and filter match the query's plan, so the optimizer serves it from the incrementally-maintained view (O(groups)) rather than re-scanning the 60M+/SF-row order_line. The other 20 queries join/correlate across tables (not yet maintainable), so no views are added for them. The Delivery txn flips ol_delivery_d filter-match; the per-PK index keeps each view exact. SF-1000 dispatch only (tools/testoperator/dispatch/chbench/sf1000/) — the manual workflow_dispatch tier; scheduled crons top out at SF100. * fix(cayenne): validate maintained-aggregate filter is Boolean; rename config field to filter_sql Addresses the two Copilot review threads on PR spiceai#11458 (both: a non-Boolean maintained-aggregate filter was accepted and only failed later during maintenance with an internal error) plus a config-naming cleanup: - Validate the filter is a Boolean predicate, fail-fast with a clear error: - MaintainedAggregateView::try_new (cayenne) type-checks spec.filter against the schema (guards every caller — config, tests, benches, future programmatic use). - parse_maintained_aggregate_filter (runtime) rejects a non-Boolean predicate at config time with InvalidConfiguration. Tests: a non-Boolean filter is rejected at view construction (cayenne) and at config parse (runtime). - Rename the spicepod config field `filter` -> `filter_sql`, matching the existing `retention_sql` convention and signalling the value is a SQL string. The cayenne-internal MaintainedAggregateSpec.filter stays `filter` (it holds a parsed PhysicalExpr, not SQL). Updated the IVM SF-1000 pod YAML to filter_sql. Validated: cargo test -p spicepod maintained (7/7), cargo test -p cayenne maintained_aggregate (lib 20 + integration 3), fmt clean. Runtime compile is re-validated by CI's full feature matrix. * docs(cayenne): clarify batch_for_spec Ok(None) fallback vs Err contract Addresses PR review: the # Errors docstring conflated the Ok(None) fallback (stale registry / no matching view / output-schema mismatch) with a true error. Add a # Returns section and narrow # Errors to the only real error case (a matched view failing to build its arrays). Doc-only; no behavior change. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(cluster): shared Ballista job state with scheduler failover (spiceai#11436) * feat(cluster): shared Ballista job state with scheduler failover Distribute Ballista job state across schedulers so that killing the scheduler driving a distributed query lets another scheduler resume it to completion. Schedulers become effectively stateless; consumers and executors are unaffected. * SharedJobState: an object-store-backed JobState. Persists each job's execution graph (via the ballista serializer) plus a small compare-and-set governed metadata document (status, owning scheduler instance id, fencing epoch). Any scheduler can read a job's status; try_acquire_job transfers ownership via CAS and bumps the epoch. * A recovery loop in the scheduler registry periodically re-drives running async queries whose owning scheduler instance is no longer in the live set. * resume_distributed / JobExecutor::resume re-drive an orphaned query, resuming the recovered execution graph instead of replanning. * SharedJobState is selected when runtime.scheduler.state_location is configured; otherwise job state stays in memory. * The async job store keys the driving scheduler by its per-process instance id so a same-address restart is correctly detected as orphaned. Pairs with the companion ballista change (execution graph serialization + recover_job), pinned via the ballista-* git rev in Cargo.toml. Claude-Session: https://claude.ai/code/session_01L4JKXq2AwTvdN2f9MJisNk * chore: repin ballista to rebased spiceai-54 base Bumps the ballista-* git rev to the version rebased onto the maintained spiceai-54 line and drops the local development patch override. * fix(cluster): harden shared job-state ownership transfer - save_job CASes ownership metadata before persisting the graph blob and yields if another scheduler owns the job, preventing a lost owner from clobbering shared state; the local entry is evicted on yield so status reads fall through to the new owner. - recovery treats a concurrent set_job_running as a no-op instead of an error. - surface (rather than discard) errors when deleting a job graph or persisting a failed unscheduled job, and warn when falling back to in-memory job state. * style: rustfmt the shared job-state ownership-transfer changes * chore: repin spice-rs to the merged lazy-Flight commit (#77) * fix(cluster): handle UpdateResult::NotFound in shared job-state OCC writes save_job errors if the job metadata no longer exists (rather than persisting an orphaned graph blob); try_acquire_job declines the claim on any non-Ok result. * fix(cluster): delay first recovery sweep until after peer discovery * refactor(cluster): address review — reuse backoff/task-tracking, enum submit mode, scheduler naming * chore(deps): bump ballista to Any-supertrait refactor (spiceai/datafusion-ballista#56) * test(cluster): construct job_executor in registry runner test * style(cluster): rustfmt registry runner test fix * harden(cluster): address review feedback on shared job state - is_terminal: treat undecodable persisted status as terminal (refuse takeover) - submit_job: claim OCC metadata before writing the graph blob; roll back on failure - remove_job: delete metadata before graph to avoid dangling metadata - resume: never short-circuit on cached results (would orphan the recovered job) - app-wait loop: stay unbounded but break on shutdown signal * chore(deps): pin ballista to merged spiceai-54 * fix(cluster): keep job graph when meta delete fails in remove_job Addresses review: remove_job must skip deleting the graph blob if the metadata delete fails, otherwise metadata can dangle to a missing graph. ObjectState::delete treats NotFound as success, so an error here is a real failure and we keep the graph. * fix(cayenne): correct ebs_upload_concurrency_cap test expectation The IMDS-baseline case asserted Some(7) but the implementation floors: floor(625 MiB/s / 90 MiB/s per-stream) = floor(6.94) = 6. The floor is the deliberate, documented invariant — a cap must never round UP past the bandwidth ceiling (7 streams would demand 630 MiB/s > 625). Fix the assertion to Some(6) and correct the comment's arithmetic. * fix(cayenne): address Copilot review on adaptive CDC tuning - autotune: ebs_upload_concurrency_cap applies the IMDS-baseline cap only to Unknown storage, not confirmed LocalSsd/Tmpfs (which parallelize well and the doc says are left uncapped); add regression coverage that fast local media stay uncapped even when an EBS baseline is known. - tuning: binding_constraint mirrors the controller's slow-tier earlier-drain gate (MEM_PRESSURE_OK - SLOW_TIER_MEM_DRAIN_OFFSET) for confirmed_slow_tier(), so a slow-tier goal stuck behind the shifted gate is no longer misclassified; add a test for the shifted vs standard gate. - builder: qualify the coordinated_mem_tier_budget floating-ceiling comment with the documented remainder >= floor precondition (clamp returns floor > remainder when the floor wins). * docs(cayenne): goal_slo_infeasible covers gating, not only clamped actuators The infeasible-SLO tracker increments whenever an eligible goal violation persists and the controller makes no move (decide_with_goals returns None) past warmup/dwell — which includes resource gating (memory/CPU pressure), not only every actuator clamped at its bound. Reword the goal_stuck_ticks and goal_slo_infeasible docs, the telemetry field doc, and the cayenne_goal_slo_infeasible metric description to match (bounds OR gating). * fix(cayenne): avoid u64 overflow in the low-disk percent_free log `available * 100` could overflow u64 before the divide on a very large (>184 PB) volume, logging a wrong percentage in the low-free-space warning. Use saturating_mul — a purely-informational percentage in a warning log. * perf(cayenne): parallelize per-shard CDC validation across shards validate_and_append_sharded ran each shard's on-conflict validation in a sequential loop (only the append step fanned out), so the insert-heavy validate cost was not parallelized at N>1. Extract validate_one_shard and fan the shards out via std::thread::scope — the N independent shard validations (disjoint keys, no shared state) run concurrently, so the apply blocks for max(shard) validate CPU instead of sum(shard). Empty shards skip the spawn (mirrors the append step's has_rows skip). Sharded LWW-equivalence tests unchanged-green. * fix(cayenne): encode-budget cap shrinks in place, no resize over-subscription cap_global_encode_concurrency swapped in a fresh Semaphore, leaving permits already held against the old one running alongside a brand-new full-size budget — aggregate concurrency could transiently reach old_held + new_cap, over-subscribing the shared EBS pipe the cap protects. Shrink the live semaphore in place via forget_permits, record the residual held permits as an owed deficit drained on the acquire path, and read an atomic live total so a clamped request can't stall against a shrunk semaphore. Single-sourced as EncodeBudget::shrink_to (prod + test both call it). Regression test cap_under_held_permits_never_oversubscribes. * style(cayenne): rustfmt write_budget.rs to fix Rust Lint cargo fmt --check flagged the forget_or_owe call at write_budget.rs:225; apply the standard multi-line wrapping to green the Rust Lint check. * refactor(cayenne): encapsulate EWMA smoothing in a tested Ewma struct Per review (bjchambers): extract the scattered alpha*x + (1-alpha)*slot smoothing and the bare-f64 EwmaInner slots into an Ewma { alpha, value: Option<f64> } with update/value. Each signal carries its own alpha (the fast cliff EWMAs use EWMA_ALPHA_FAST), and self-seeding on the first sample removes the parallel io_samples/publish_samples priming counters — value() == None now encodes "unseeded", replacing the (samples > 0).then_some(..) gating. Adds direct unit tests; behavior preserved (the existing tuning tests still pass). * docs(cayenne): align infeasible-SLO warning text with bounds-or-gating Follow-up to the goal_slo_infeasible doc/metric rewording: the operator warning still said "every relevant actuator is at its limit", but the tracker can reach infeasible via resource gating (memory/CPU) too. Reword to "no further tuning adjustment is available (actuator bounds or resource gating)"; the constraint field already names the actual binding resource. * test(search): pin target_partitions so rerank EXPLAIN snapshot is deterministic The rerank filter-pushdown EXPLAIN snapshot embedded `RepartitionExec: RoundRobinBatch(N)` where N = the host CPU count, so it failed on CI runners whose core count differs from the machine that recorded it (snapshot had 10; runner has 12). Pin target_partitions=1 in the test session: DataFusion then omits the incidental post-filter round-robin repartition, making the plan deterministic across runners. The filter-pushdown assertion (FilterExec below RerankExec) and query results are unchanged; snapshot re-recorded. Unblocks Build and Test for spiceai#11463 (failure was unrelated to the Cayenne change). * chore(telemetry): name cayenne storage write-throughput gauges *_mibps The two new calibration-probe gauges measure MiB/s but were named *_write_mbps, which reads as megabits/s. Rename them (and their backing statics) to cayenne_data_storage_write_mibps / cayenne_metastore_storage_write_mibps to match the MiB/s unit and avoid dashboard/operator confusion. Metric-name-only change; the internal/serialized backing field names are left untouched. * test(cayenne): satisfy clippy::unwrap_used in Ewma tests CI Rust Lint denies clippy::unwrap_used in test code (only expect_used is allowed back), but the new Ewma tests called .unwrap() on Option<f64>. Switch to .unwrap_or(f64::NAN) so a None still fails the tolerance assertion. Verified with the exact CI clippy flag set (lib + tests gates, --no-deps) on cayenne. * fix(cayenne): binding_constraint binds on memory at exactly the gate The controller's mem_ok is `p < mem_gate`, so it blocks buffer growth at `p >= mem_gate`. binding_constraint used a strict `>`, misclassifying the binding resource (as CPU/storage) at exactly `p == mem_gate`. Use `>=` to mirror the controller's effective gate. Adds a boundary test at exactly the shifted slow-tier gate; verified with the exact CI clippy flag set. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Phillip LeBlanc <879445+phillipleblanc@users.noreply.github.com> Co-authored-by: claudespice <claude@spice.ai> Co-authored-by: Jeadie <jeadie@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
github-actions Bot
pushed a commit
to spiceai/spiceai
that referenced
this pull request
Jun 29, 2026
… IMDS, I/O-cliff fast path, infeasible-SLO feedback (#11463) * feat(cayenne): storage-aware adaptive CDC tuning — calibration probe, IMDS, I/O-cliff fast path, infeasible-SLO feedback Make the closed-loop CDC auto-tuner environment- and EBS-aware. The tuner previously classified storage by device identity (a 4-value enum) and reacted to I/O latency with a slow additive crawl; this measures real storage performance, reacts decisively to burst-credit cliffs, and surfaces when an SLO is infeasible on the hardware. Environment detection - Calibration probe at registration (off-runtime, memoized per volume, fail-open) measures real write throughput, so the slow-tier bias is continuous instead of a device-identity class — and it is the only storage signal a cdc_durability:memory table that never spills produces. Cloud-agnostic. - EC2 instance probe via aws_config::imds (tight timeout, opt out with SPICE_DISABLE_IMDS): T-family burstable CPU + EBS-optimized baseline bandwidth. - Wire the detected storage class + measured throughput onto the Cayenne config (was left at Unknown, so the loop applied the EBS bias even on local NVMe). Closed-loop control - Continuous slow-tier bias (tier_scale): a fast io2 volume no longer gets the same commit-amortization pressure as slow gp3. - I/O-cliff fast path: a fast/slow latency-EWMA divergence (EBS burst-credit depletion / instance EBS pipe saturating) triggers one decisive multiplicative backoff instead of an additive crawl — the I/O analogue of the critical-memory fast path; applies in both legacy and goal modes. - Infeasible-SLO feedback: when a goal stays violated with every relevant lever clamped, warn once naming the binding constraint and latch a telemetry gauge. - Earlier/smaller drain on a confirmed slow tier (EBS/measured-slow; Unknown keeps the standard thresholds). Burstable-CPU gate withholds CPU-stealing moves sooner. Resource coordination - Process-global encode budget bounded by the instance EBS write bandwidth. - Conservative floating mem-tier ceiling for query-light deployments (host/8 -> host/6), preserving the #11449 no-overcommit invariant exactly. - Low-free-space startup warning on the data/spill volume. Telemetry gauges for measured throughput + goal-infeasible. Unit tests across all of the above; scoped cargo check, targeted tests (cayenne tuning 73/0, runtime storage/imds/autotune/builder 54/0), and scoped clippy are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: cargo fmt Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review — per-volume probe cache key + imds intra-doc link - storage.rs: extract a shared probe_dir() and key the calibration-probe cache on the canonicalized *probed directory* (the parent when handed a file path), so two files on the same volume dedupe to a single probe (the cache key now matches the dir actually probed). - imds.rs: link the in-scope [`Client`] in the module docs instead of `aws_config::imds::Client`, so the intra-doc link resolves (no rustdoc warning). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review round 2 (Copilot) - autotune: ebs_upload_concurrency_cap floors (not rounds) the stream count, so a cap never rounds up past the bandwidth ceiling. - storage: probe cache stores a per-key shared OnceCell so concurrent registrations on one volume coalesce on a single probe (no duplicate 8 MiB writes). - cayenne/mod: warn_if_low_disk is async and runs the canonicalize + mount enumeration via spawn_blocking (off the Tokio worker); dedup key is canonicalized so equivalent paths (trailing slash, symlinks) collapse to one check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review round 3 (Copilot) - write_budget: make cap_global_encode_concurrency atomic — check-and-replace under a single write lock (re-read total, replace only while it still exceeds the cap), so concurrent caps can't race and 'tightest wins, never raises' holds. - storage: move probe_dir (is_dir) + canonicalize into spawn_blocking, so probe_storage_perf_async does no blocking filesystem I/O on the runtime thread. - docs: correct the infeasible-SLO signal wording — it is intentionally NOT a permanent latch; the gauge reflects current state and self-clears if the SLO becomes reachable again, and the warning fires once per infeasible episode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): CI Rust Lint (pedantic doc_markdown) + PR review round 4 - Backtick doc-comment identifiers flagged by clippy::doc_markdown (-Dpedantic): NVMe (tuning.rs, autotune.rs) and IMDSv2/IMDSv1 (imds.rs) — the Rust Lint failure. - storage: move static PROBE_SEQ before statements (clippy::items_after_statements); open the probe temp file with create_new (O_EXCL) so it never truncates/clobbers a pre-existing or attacker-planted file (fails open instead). - cayenne/mod: correct warn_if_low_disk docs — the dedup key is the canonicalized path, not the mount, so 'once per canonicalized path' (distinct dirs on the same mount each warn once). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): CI Rust Lint on test code (--tests pedantic) The CI lint runs clippy --tests too (not just --lib): fix the two test-code lints it caught: - tuning.rs: use .expect() instead of .unwrap() in a test (clippy::unwrap_used; tests allow expect_used but not unwrap_used). - datafusion/builder.rs: drop a '+ 0' in the no-overcommit assertion (identity_op). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cayenne): address PR review — binding_constraint message accuracy - 'memory-bound' no longer claims buffers are 'shrinking' (shrinking only starts at MEM_PRESSURE_HIGH); reworded to 'at/over the RAM budget — can't grow buffers'. - Reserve the 'EBS' wording for the confirmed Ebs class; a slow/undetected (Unknown) tier now reports 'slow or undetected tier' instead of misnaming it EBS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(deps): bump vortex to spiceai-54 HEAD (flat reader subrange decode fix) (#11466) Bumps vortex/vortex-* from 729249dd to ab4f0b17 (spiceai-54 HEAD), bringing in "Fix flat reader subrange decode reuse" plus the rebase of the spiceai-54 perf work (intra-file decode parallelism, per-ExecutionCtx ArrayKernels snapshot) onto the Vortex 0.75.0 base. Net source delta vs the prior pin is internal-only (flat/reader.rs, arc_swap_map.rs, optimizer/kernels.rs) with no public API change, so no Spice-side code changes are required. The spiceai-54 tip (e09ec32c) was briefly un-buildable — the rebase dropped ArcSwapMap::load_full and a HashMap import still used by the kernel snapshot; restored upstream in spiceai/vortex#70, folded into the ab4f0b17 HEAD pinned here. Verified: cargo check -p cayenne -p vortex-datafusion clean; Cargo.lock regenerated (git rev only, dependency tree unchanged). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cluster): route Ballista shuffle/temp to the data PVC (#11454) * fix(cluster): route Ballista shuffle/temp to the data PVC Cluster-bench executors wrote Ballista shuffle output to the default work_dir (env::temp_dir() = /tmp), a small EmptyDir whose sizeLimit a SF10+ shuffle exceeds, getting the pod evicted mid-query and livelocking the run. The dispatch workflow already exports SPIDAPTER_SHUFFLE_LOCATION and SPIDAPTER_QUERY_TEMP_DIRECTORY (pointing at the executor's /data PVC) but nothing consumed them. - spidapter: inject SPIDAPTER_SHUFFLE_LOCATION into runtime.params and SPIDAPTER_QUERY_TEMP_DIRECTORY into runtime.query.temp_directory when generating the cluster app spicepod (mirrors SCHEDULER_STATE_LOCATION). - runtime: create the configured shuffle work_dir (create_dir_all) instead of only warning, since Ballista uses work_dir as-is without creating it. * style: rustfmt the spidapter shuffle-location injection * fix(cluster): drive Flight TLS handshakes concurrently (fix shuffle-fetch connection resets) (#11456) * fix(cluster): drive TLS handshakes concurrently to avoid shuffle-fetch resets The Flight server's tls_incoming accepted one connection, awaited its TLS handshake inline, then accepted the next — serializing acceptance. Under a distributed-join shuffle (each reducer opens up to 64 fetch connections per peer, no client-side pooling) the listener accept queue overflowed and peers saw 'connection reset by peer', failing every multi-stage query while single-stage queries (no shuffle) passed. Drive up to 1024 handshakes concurrently via buffer_unordered so acceptance keeps draining the backlog. * style: rustfmt the spidapter shuffle-location injection * fix(search): default Elasticsearch kNN candidate pool to 1000 instead of 10 (fixes #11264) (#11467) * fix(search): default Elasticsearch kNN candidate pool to 1000 instead of 10 (fixes #11264) The Elasticsearch vector index hardcoded the kNN candidate pool to k=10 in query_table_provider. The search pipeline places Filter/Sort above the index scan, so DataFusion cannot push the requested top-K below them and the scan receives limit=None — silently capping every filtered/sorted vector search at 10 hits regardless of the requested limit. Use a shared 1000 default matching the sibling DuckDB vector backend (DEFAULT_DUCKDB_VECTOR_SEARCH_LIMIT). Kept at 1000 because the kNN exec requests num_candidates = k*2 and Elasticsearch rejects num_candidates above 10,000, so a larger default would fail the search request outright. * fix(search): simplify ES kNN default to a named constant Replace the regression test and compile-time assertions with a plain DEFAULT_ELASTICSEARCH_VECTOR_SEARCH_LIMIT = 1000 constant used as the kNN candidate-pool fallback, matching the DuckDB vector backend. --------- Co-authored-by: Jeadie <jeadie@users.noreply.github.com> * feat(acceleration): add on_schema_change drop_and_recreate policy (#11462) * feat(acceleration): add on_schema_change drop_and_recreate policy Widening schema evolution (append_new_columns/sync_all_columns, #11261) adopts lossless changes in place but leaves non-widening changes block-equivalent. This adds the `drop_and_recreate` policy that recreates the accelerated table when a change cannot be applied in place (dropped columns, narrowing/incompatible type changes, new non-nullable columns) -- but only under refresh_mode: full on a drop-capable engine (duckdb/sqlite/turso/cayenne), where a full refresh re-fetches every row so the drop is lossless. Widening still applies in place; in append/changes modes an incompatible change is rejected and the table preserved. A single recreates_on_schema_mismatch() helper is the source of truth for the recreate decision, consulted by handle_schema_difference and by both the initial-load and reload schema-mismatch gates so they cannot drift. Reuses the existing snapshot -> drop_table -> clear-checkpoint machinery. Adds schema_evolution_applied{action=recreate} metric + task_history events for the evolution lifecycle. Unit tests + CSV integration tests (duckdb/sqlite/cayenne). Closes #10008 * fix: drop now-unused Mode import in init/dataset.rs The file_update/drop_and_recreate gate now goes through schema_evolution::recreates_on_schema_mismatch, so the direct Mode::FileUpdate comparison (and its import) are no longer needed. * fix: address PR review comments + rustfmt - Recreate path now clears cached logical plans (clear_cached_plans), matching the in-place evolution path, so stale plans don't survive a destructive schema change. - Clarify recreates_on_schema_mismatch doc + the init/dataset.rs gate comment to reflect the exact conditions (file_update requires refreshes enabled; drop_and_recreate requires refresh_mode: full on a recreate-capable engine). - rustfmt. * fix: DuckDB drop_table handles view layout; backtick refresh_mode in doc - DuckDB drop_table now dispatches on the object type (view vs base table) and drops every internal __data_{name}_{ts} table, so drop_and_recreate works in mode: file (full refresh) where DuckDB stores the accelerated object as a view. Previously DROP TABLE errored ('object is of type View'). Reuses the same layout helpers as evolve_table_schema; the file_update base-table path is unchanged. - Backtick `refresh_mode` in the DropAndRecreate doc (clippy::doc_markdown) and re-sync the generated spicepod.schema.json description. * fix(clippy): merge identical drop_and_recreate/sync_all_columns match arm clippy::match_same_arms — the CDC SchemaEvolutionPolicy mapping had separate SyncAllColumns and DropAndRecreate arms with identical bodies; merge them into one or-pattern (mirrors the cayenne mapping). * refactor: engine_supports_recreate delegates to engine_supports_in_place_evolution Avoids duplicating the engine match list (they are the same set today: the four engines with a real drop_table are exactly those with evolve_table_schema). Single source of truth; doc notes to split if a future engine gains one capability but not the other. * fix: align recreate metric/event action label; correct Phase 2 test comment - task_history recreate event now uses action="recreate", matching the SCHEMA_EVOLUTION_APPLIED metric label, so metrics and task_history rows correlate. - Reword the drop_and_recreate Phase 2 comment: the assertion checks the observable outcome (column added, rows preserved); in-place-vs-recreate is unit-tested, not asserted here. * feat(cayenne): predicate-aware maintained aggregates serve filtered analytical queries from the CDC delta (#11458) * feat(cayenne): predicate-aware maintained aggregates serve filtered analytical queries from the CDC delta The maintained-aggregate registry (#11389) serves grouped SUM/COUNT/AVG in O(groups) from the CDC delta, but only when the plan is a clean unfiltered aggregate directly over the Cayenne scan: a FilterExec between the AggregateExec and the scan defeated the optimizer rewrite, so every analytical query with a WHERE fell back to an O(rows) re-scan. This makes the maintained view predicate-aware. It maintains the aggregate over only the rows its filter selects, and the optimizer serves a query carrying the identical predicate from that state. A non-matching row is treated as absent (not indexed, not accumulated), so all retraction logic is reused unchanged; an UPDATE that flips a row's filter status always retracts the prior contribution first. Exact and deletion-safe via the existing per-PK contribution index (not a sketch). The serve gate requires the query's filter to equal the view's exactly, so a filtered view never answers an unfiltered query, and vice versa. - maintained_aggregate.rs: optional filter on the spec and view; mask-aware apply_insert_batch; filter-equality in the serve match; batch_for_spec serve hook; filter threaded through batch_for_aggregate_with_output. - optimizer_rules.rs: descend a single FilterExec during plan walk and capture its predicate for the serve. - tests/maintained_aggregate_filter_test.rs: full CDC-lifecycle correctness (insert / update-out-of-predicate / update-into-predicate / delete vs a from-scratch filtered recompute) plus the serve-gate guard; optimizer routing tests cover the positive (filtered query routes) and negative (unfiltered query over a filtered view is refused) paths. - benches: vs_duckdb_ and vs_chdb_maintained_filtered_groupby as separate binaries (the two bundled engines abort if driven in one process). Measured on the q1/q6 shape (~16 groups, result asserted equal before timing): Cayenne serve is flat ~3.1 us (O(groups)); DuckDB 382 us -> 639 us and chDB 2.04 ms -> 2.36 ms across 100k -> 1M rows (O(rows)). That is 117x/205x faster than DuckDB and 634x/740x faster than chDB, and the gap widens with table size. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(cayenne): user-definable filter on maintained_aggregates config (fire filtered queries end-to-end) Adds an optional `filter` SQL predicate to the spicepod `maintained_aggregates` config and parses it into a physical expression over the dataset schema, so a configured maintained view can be declared WITH a `WHERE` and serve a live filtered analytical query (e.g. CH-benCH q1/q6) from the incrementally-maintained state — the end-to-end enabler for the predicate-aware serving added earlier in this PR. - spicepod: `MaintainedAggregate.filter: Option<String>` (serde-default, so the config stays backward compatible) + deserialize test. - runtime: thread the table schema into `maintained_aggregate_specs_for_cayenne` and parse the filter via `parse_sql_expr` + `create_physical_expr` over the schema (so the predicate's column indices match both the CDC batches the view is maintained from and a filtered query's `FilterExec`); a parse/bind/plan failure surfaces as an `InvalidConfiguration` error naming the filter. - tests: filter round-trips from YAML; config filter parses onto the spec; a filter referencing an unknown column is rejected. Also merges origin/trunk (incl. the Cayenne provider type-move refactor) into the branch; the maintained-aggregate lever recompiles cleanly against it. Note: full auto-derivation of the filter from the observed recurring workload is a larger follow-up; this lands the user-definable path, which is the concrete piece that makes a configured q1/q6 serve from the maintained view. * refactor(cayenne): simplify filtered maintained-aggregate apply, share the bench fixture, document the serve-match boundary /simplify cleanup of the predicate-aware maintained-aggregate change; no behavior change. - maintained_aggregate.rs: collapse `apply_insert_batch`'s indexed/unindexed branches into one shared tail — compute the PK once, then a single match-gate + group insert — instead of duplicating the post-filter path (`continue`, group-key, insert) in both arms. - Document the serve-match boundary on `matches_query` and the runtime filter parser: the match is structural `PhysicalExpr` equality over the scan-output schema, so it silently does NOT fire across a projection / cast / type-coercion between the scan and the filter (e.g. `SchemaCastScanExec`, or `Utf8View`-over-`Utf8`) and falls back to a re-scan. Name-based predicate normalization is the follow-up. - benches: extract the duplicated Cayenne-side fixture (schema, spec, row generator, registry load, serve decode) into `maintained_filtered_helpers/common.rs`, shared by the vs_duckdb and vs_chdb benches via the established `#[path] mod` pattern, so the registry/spec contract can't drift between the two split binaries. Validated: `cargo test -p cayenne maintained_aggregate` (20 lib + 2 integration tests) green; both maintained-filtered benches compile under duckdb-bench/chdb-bench; fmt clean. * test(chbench): add IVM (predicate-aware maintained-aggregate) SF-1000 dispatch pod New accelerated spicepod postgres-cayenne[file]-cdc-tuned-memory-ivm-sf1000.yaml: the SF-1000 cdc-tuned MEMORY (in-memory tier) baseline plus maintained_aggregates on order_line — the heaviest CH-benCH table — for BOTH of its single-table analytical queries: - q1: SUM/AVG(ol_quantity, ol_amount) + COUNT GROUP BY ol_number, WHERE ol_delivery_d > '2007-01-02'. - q6: global SUM(ol_amount) WHERE ol_delivery_d in a window AND ol_quantity BETWEEN 1 AND 100000. Each view's group-by, aggregate set/order, and filter match the query's plan, so the optimizer serves it from the incrementally-maintained view (O(groups)) rather than re-scanning the 60M+/SF-row order_line. The other 20 queries join/correlate across tables (not yet maintainable), so no views are added for them. The Delivery txn flips ol_delivery_d filter-match; the per-PK index keeps each view exact. SF-1000 dispatch only (tools/testoperator/dispatch/chbench/sf1000/) — the manual workflow_dispatch tier; scheduled crons top out at SF100. * fix(cayenne): validate maintained-aggregate filter is Boolean; rename config field to filter_sql Addresses the two Copilot review threads on PR #11458 (both: a non-Boolean maintained-aggregate filter was accepted and only failed later during maintenance with an internal error) plus a config-naming cleanup: - Validate the filter is a Boolean predicate, fail-fast with a clear error: - MaintainedAggregateView::try_new (cayenne) type-checks spec.filter against the schema (guards every caller — config, tests, benches, future programmatic use). - parse_maintained_aggregate_filter (runtime) rejects a non-Boolean predicate at config time with InvalidConfiguration. Tests: a non-Boolean filter is rejected at view construction (cayenne) and at config parse (runtime). - Rename the spicepod config field `filter` -> `filter_sql`, matching the existing `retention_sql` convention and signalling the value is a SQL string. The cayenne-internal MaintainedAggregateSpec.filter stays `filter` (it holds a parsed PhysicalExpr, not SQL). Updated the IVM SF-1000 pod YAML to filter_sql. Validated: cargo test -p spicepod maintained (7/7), cargo test -p cayenne maintained_aggregate (lib 20 + integration 3), fmt clean. Runtime compile is re-validated by CI's full feature matrix. * docs(cayenne): clarify batch_for_spec Ok(None) fallback vs Err contract Addresses PR review: the # Errors docstring conflated the Ok(None) fallback (stale registry / no matching view / output-schema mismatch) with a true error. Add a # Returns section and narrow # Errors to the only real error case (a matched view failing to build its arrays). Doc-only; no behavior change. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(cluster): shared Ballista job state with scheduler failover (#11436) * feat(cluster): shared Ballista job state with scheduler failover Distribute Ballista job state across schedulers so that killing the scheduler driving a distributed query lets another scheduler resume it to completion. Schedulers become effectively stateless; consumers and executors are unaffected. * SharedJobState: an object-store-backed JobState. Persists each job's execution graph (via the ballista serializer) plus a small compare-and-set governed metadata document (status, owning scheduler instance id, fencing epoch). Any scheduler can read a job's status; try_acquire_job transfers ownership via CAS and bumps the epoch. * A recovery loop in the scheduler registry periodically re-drives running async queries whose owning scheduler instance is no longer in the live set. * resume_distributed / JobExecutor::resume re-drive an orphaned query, resuming the recovered execution graph instead of replanning. * SharedJobState is selected when runtime.scheduler.state_location is configured; otherwise job state stays in memory. * The async job store keys the driving scheduler by its per-process instance id so a same-address restart is correctly detected as orphaned. Pairs with the companion ballista change (execution graph serialization + recover_job), pinned via the ballista-* git rev in Cargo.toml. Claude-Session: https://claude.ai/code/session_01L4JKXq2AwTvdN2f9MJisNk * chore: repin ballista to rebased spiceai-54 base Bumps the ballista-* git rev to the version rebased onto the maintained spiceai-54 line and drops the local development patch override. * fix(cluster): harden shared job-state ownership transfer - save_job CASes ownership metadata before persisting the graph blob and yields if another scheduler owns the job, preventing a lost owner from clobbering shared state; the local entry is evicted on yield so status reads fall through to the new owner. - recovery treats a concurrent set_job_running as a no-op instead of an error. - surface (rather than discard) errors when deleting a job graph or persisting a failed unscheduled job, and warn when falling back to in-memory job state. * style: rustfmt the shared job-state ownership-transfer changes * chore: repin spice-rs to the merged lazy-Flight commit (#77) * fix(cluster): handle UpdateResult::NotFound in shared job-state OCC writes save_job errors if the job metadata no longer exists (rather than persisting an orphaned graph blob); try_acquire_job declines the claim on any non-Ok result. * fix(cluster): delay first recovery sweep until after peer discovery * refactor(cluster): address review — reuse backoff/task-tracking, enum submit mode, scheduler naming * chore(deps): bump ballista to Any-supertrait refactor (spiceai/datafusion-ballista#56) * test(cluster): construct job_executor in registry runner test * style(cluster): rustfmt registry runner test fix * harden(cluster): address review feedback on shared job state - is_terminal: treat undecodable persisted status as terminal (refuse takeover) - submit_job: claim OCC metadata before writing the graph blob; roll back on failure - remove_job: delete metadata before graph to avoid dangling metadata - resume: never short-circuit on cached results (would orphan the recovered job) - app-wait loop: stay unbounded but break on shutdown signal * chore(deps): pin ballista to merged spiceai-54 * fix(cluster): keep job graph when meta delete fails in remove_job Addresses review: remove_job must skip deleting the graph blob if the metadata delete fails, otherwise metadata can dangle to a missing graph. ObjectState::delete treats NotFound as success, so an error here is a real failure and we keep the graph. * fix(cayenne): correct ebs_upload_concurrency_cap test expectation The IMDS-baseline case asserted Some(7) but the implementation floors: floor(625 MiB/s / 90 MiB/s per-stream) = floor(6.94) = 6. The floor is the deliberate, documented invariant — a cap must never round UP past the bandwidth ceiling (7 streams would demand 630 MiB/s > 625). Fix the assertion to Some(6) and correct the comment's arithmetic. * fix(cayenne): address Copilot review on adaptive CDC tuning - autotune: ebs_upload_concurrency_cap applies the IMDS-baseline cap only to Unknown storage, not confirmed LocalSsd/Tmpfs (which parallelize well and the doc says are left uncapped); add regression coverage that fast local media stay uncapped even when an EBS baseline is known. - tuning: binding_constraint mirrors the controller's slow-tier earlier-drain gate (MEM_PRESSURE_OK - SLOW_TIER_MEM_DRAIN_OFFSET) for confirmed_slow_tier(), so a slow-tier goal stuck behind the shifted gate is no longer misclassified; add a test for the shifted vs standard gate. - builder: qualify the coordinated_mem_tier_budget floating-ceiling comment with the documented remainder >= floor precondition (clamp returns floor > remainder when the floor wins). * docs(cayenne): goal_slo_infeasible covers gating, not only clamped actuators The infeasible-SLO tracker increments whenever an eligible goal violation persists and the controller makes no move (decide_with_goals returns None) past warmup/dwell — which includes resource gating (memory/CPU pressure), not only every actuator clamped at its bound. Reword the goal_stuck_ticks and goal_slo_infeasible docs, the telemetry field doc, and the cayenne_goal_slo_infeasible metric description to match (bounds OR gating). * fix(cayenne): avoid u64 overflow in the low-disk percent_free log `available * 100` could overflow u64 before the divide on a very large (>184 PB) volume, logging a wrong percentage in the low-free-space warning. Use saturating_mul — a purely-informational percentage in a warning log. * perf(cayenne): parallelize per-shard CDC validation across shards validate_and_append_sharded ran each shard's on-conflict validation in a sequential loop (only the append step fanned out), so the insert-heavy validate cost was not parallelized at N>1. Extract validate_one_shard and fan the shards out via std::thread::scope — the N independent shard validations (disjoint keys, no shared state) run concurrently, so the apply blocks for max(shard) validate CPU instead of sum(shard). Empty shards skip the spawn (mirrors the append step's has_rows skip). Sharded LWW-equivalence tests unchanged-green. * fix(cayenne): encode-budget cap shrinks in place, no resize over-subscription cap_global_encode_concurrency swapped in a fresh Semaphore, leaving permits already held against the old one running alongside a brand-new full-size budget — aggregate concurrency could transiently reach old_held + new_cap, over-subscribing the shared EBS pipe the cap protects. Shrink the live semaphore in place via forget_permits, record the residual held permits as an owed deficit drained on the acquire path, and read an atomic live total so a clamped request can't stall against a shrunk semaphore. Single-sourced as EncodeBudget::shrink_to (prod + test both call it). Regression test cap_under_held_permits_never_oversubscribes. * style(cayenne): rustfmt write_budget.rs to fix Rust Lint cargo fmt --check flagged the forget_or_owe call at write_budget.rs:225; apply the standard multi-line wrapping to green the Rust Lint check. * refactor(cayenne): encapsulate EWMA smoothing in a tested Ewma struct Per review (bjchambers): extract the scattered alpha*x + (1-alpha)*slot smoothing and the bare-f64 EwmaInner slots into an Ewma { alpha, value: Option<f64> } with update/value. Each signal carries its own alpha (the fast cliff EWMAs use EWMA_ALPHA_FAST), and self-seeding on the first sample removes the parallel io_samples/publish_samples priming counters — value() == None now encodes "unseeded", replacing the (samples > 0).then_some(..) gating. Adds direct unit tests; behavior preserved (the existing tuning tests still pass). * docs(cayenne): align infeasible-SLO warning text with bounds-or-gating Follow-up to the goal_slo_infeasible doc/metric rewording: the operator warning still said "every relevant actuator is at its limit", but the tracker can reach infeasible via resource gating (memory/CPU) too. Reword to "no further tuning adjustment is available (actuator bounds or resource gating)"; the constraint field already names the actual binding resource. * test(search): pin target_partitions so rerank EXPLAIN snapshot is deterministic The rerank filter-pushdown EXPLAIN snapshot embedded `RepartitionExec: RoundRobinBatch(N)` where N = the host CPU count, so it failed on CI runners whose core count differs from the machine that recorded it (snapshot had 10; runner has 12). Pin target_partitions=1 in the test session: DataFusion then omits the incidental post-filter round-robin repartition, making the plan deterministic across runners. The filter-pushdown assertion (FilterExec below RerankExec) and query results are unchanged; snapshot re-recorded. Unblocks Build and Test for #11463 (failure was unrelated to the Cayenne change). * chore(telemetry): name cayenne storage write-throughput gauges *_mibps The two new calibration-probe gauges measure MiB/s but were named *_write_mbps, which reads as megabits/s. Rename them (and their backing statics) to cayenne_data_storage_write_mibps / cayenne_metastore_storage_write_mibps to match the MiB/s unit and avoid dashboard/operator confusion. Metric-name-only change; the internal/serialized backing field names are left untouched. * test(cayenne): satisfy clippy::unwrap_used in Ewma tests CI Rust Lint denies clippy::unwrap_used in test code (only expect_used is allowed back), but the new Ewma tests called .unwrap() on Option<f64>. Switch to .unwrap_or(f64::NAN) so a None still fails the tolerance assertion. Verified with the exact CI clippy flag set (lib + tests gates, --no-deps) on cayenne. * fix(cayenne): binding_constraint binds on memory at exactly the gate The controller's mem_ok is `p < mem_gate`, so it blocks buffer growth at `p >= mem_gate`. binding_constraint used a strict `>`, misclassifying the binding resource (as CPU/storage) at exactly `p == mem_gate`. Use `>=` to mirror the controller's effective gate. Adds a boundary test at exactly the shifted slow-tier gate; verified with the exact CI clippy flag set. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Phillip LeBlanc <879445+phillipleblanc@users.noreply.github.com> Co-authored-by: claudespice <claude@spice.ai> Co-authored-by: Jeadie <jeadie@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The current
spiceai-54HEAD (e09ec32c) does not compile. The rebase thatproduced it (the
spiceai-54perf work re-applied on top of the 0.75.0 base)dropped two symbols that the per-
ExecutionCtxArrayKernelssnapshot stilluses, while keeping their callers:
ArcSwapMap::load_fullwas removed fromarc_swap_map.rs, butArrayKernels::snapshot()(optimizer/kernels.rs:206) still calls it.use vortex_utils::aliases::hash_map::HashMap;import was removed fromkernels.rs, but theKernelSnapshotstruct field (kernels.rs:218) stillnames
HashMap.Result (any consumer that enables the optimizer, e.g. building
vortex-array):Fix
executor.rs(unchanged from the prior good commit) still consumessnapshot()/KernelSnapshot, so the snapshot machinery is meant to stay —the removals were collateral from the rebase, not an intentional revert. This
restores the two dropped lines. Both files become byte-identical to the
prior good commit
729249dd(blob hashes567e025ande16e59a), so this isa pure un-break with no behavioral change.
🤖 Generated with Claude Code