refactor(query-engine): execute native range query DAGs

[milindsrivastava1997]

Native query DAG executor

Plan and decisions

Issue #729 replaces range-query execution with a small native DAG while preserving public PromQL behavior. The DAG owns dependency ordering and node-once execution; SimpleEngine supplies storage, window composition, key resolution, estimation, top-k, and formatting.

The cutover is intentionally scoped to individual native query arms. Complete arithmetic/native-query DAG execution is deferred to #743. Capability misses remain fallback-eligible; an accepted native execution failure is local and must never forward to Prometheus.

What changed

• Added an explicit range-query plan with store-read, window composition, key resolution, estimate, top-k, and formatting stages.
• Validated DAG dependencies and execute each node once in dependency order.
• Preserved execution errors through the query engine and HTTP server, distinguishing local failures from unsupported queries.
• Added real-precompute E2E legacy-versus-DAG characterizations for leaf, sparse, keyed, top-k/tie, DeltaSet replay, and failure paths.
• Added Docker differential suites for off-grid rate, sparse native DAG behavior, and native aggregations.

Validation

• cargo test --manifest-path asap-query-engine/Cargo.toml --features native_query_legacy_test_support — passed (635 unit tests and 12 E2E tests).
• Docker differential matrix is being rerun from a clean stack with a unique fixture timestamp.

Commit guide

Architecture decision record: grilling Q&A (verbatim)

Design choices decided

This records the design discussion for future implementation reference. Questions and answers below are preserved as asked and answered.

1. Question: “First question: what should count as the TDD acceptance seam? My recommendation: use the existing real precompute pipeline—remote-write ingestion → precompute windows → SimpleMapStore → public handle_query_promql / handle_range_query_promql—as the E2E test harness. It exercises real data production and public query behavior without adding an HTTP query-server lifecycle that is unrelated to this refactor. The first tracer test should be a range query whose output is correct only if the DAG executor is actually used; after the cutover it goes green. Do you want that seam, or do you require the query HTTP endpoint in the acceptance tests too?”

   Answer: “Yes, that makes sense. We can also have legacy vs new pipeline tests using the handle_query_promql and handle_range_query_promql interfaces. We can also use the e2e promql compliance suite”

2. Question: “One constraint needs a decision. Issue 729 requires invalid DAGs to return actionable errors, but a client cannot submit a DAG—the compiler builds it internally from a valid PromQL request. Therefore an E2E test cannot naturally produce a malformed DAG. My recommendation: keep the TDD acceptance tests E2E as agreed, but add one narrow internal validation test for malformed graph structure (for example, a missing input reference). Do not add a test-only HTTP/config switch just to inject invalid plans; that would enlarge the production interface for a state users cannot create. Is that exception for graph validation acceptable?”

   Answer: “makes sense”

3. Question: “Next question: should legacy-vs-DAG differential comparison be a temporary development aid or remain as a permanent test mechanism? My recommendation: temporary only. During the cutover, retain the old executor behind an internal test-only selector and run the same public handle_query_promql / handle_range_query_promql calls against both modes. Once the E2E cases and PromQL compliance suite pass, delete the legacy selector and old executor. Keeping two production-capable pipelines permanently would create a shallow, drifting abstraction and double the behavior we must maintain. Do you agree that the legacy path should disappear with this PR?”

   Answer: “yes, temporaty”

4. Question: “For this work, I recommend changing how each individual query is executed by the new DAG, but leaving the existing +, -, *, / combination code alone. The combined query still benefits because each side uses the new executor. Should we keep it that way for issue 729?”

   Answer: “what's the alternative?”

   Clarification: The alternative is a DAG representing the whole combined query, including scalar constants, arithmetic, PromQL label matching, timestamp alignment, and fallback behavior.

   Decision: “okay fine, let's keep the scope small”

   A follow-up was created and assigned: refactor(query-engine): execute complete native-query DAGs #743, refactor(query-engine): execute complete native-query DAGs.

5. Question: “Because refactor(query-engine): execute native query DAGs #729 is explicitly behavior-preserving, every correct public E2E query already passes through the legacy pipeline today. A new E2E correctness test cannot go red before the DAG executor exists. My recommendation: use characterization-driven TDD for this refactor: (1) add public-interface E2E cases that document the required behavior; (2) during the cutover, run those same cases against the temporary legacy and DAG modes and assert identical results; (3) make DAG mode the default, then delete legacy mode; (4) add one narrow internal test that confirms the default path rejects an invalid DAG loudly. This preserves E2E as the source of correctness without inventing a fake user-visible behavior just to force a red test. Do you want to use this form of TDD for refactor(query-engine): execute native query DAGs #729?”

   Answer: “yeah that makes sense”

6. Question: “what should the first tracer scenario be? My recommendation: an E2E tumbling sum_over_time range query over two label groups, with remote-write ingestion and a query through handle_range_query_promql; also compare its value at the final timestamp through handle_query_promql. It exercises the complete normal DAG path while staying simple enough to diagnose. After that, we add the riskier cases one at a time: sliding exact cover, separate key aggregation, DeltaSet key replay, top-k, then fallback/error behavior. Do you want that as the first tracer?”

   Answer: “do we need to add them one at a time? or can we just add them all in one go?”

   Decision: Add the full E2E characterization matrix up front; implement and validate the DAG executor in small vertical slices; run the smallest relevant scenario after each slice and the full matrix before deleting legacy.

7. Question: “how should the temporary legacy-vs-DAG selector work? My recommendation: keep it internal to the crate and only usable by test code. Test fixtures can construct a SimpleEngine in either mode, but invoke only the real public query methods. There will be no YAML option, environment variable, or HTTP parameter that lets users select legacy execution. Do you agree with that test-only selector?”

   Answer: “yes”

8. Question: “Think of it as two jobs. The DAG executor’s only job is to follow the arrows: read data → build windows → find keys → calculate values → top-k → format result. It makes sure each box runs in the right order, and only once if multiple later boxes need its result. SimpleEngine’s job is what each box actually does: how to read buckets from the store; how to merge a window; how to get keys from an accumulator; how to calculate a statistic. So the DAG executor does not need to understand sketches, PromQL, or storage. It just runs the graph. SimpleEngine provides the query-specific work. The outside world still just calls handle_query_promql or handle_range_query_promql as before. Does that separation sound right?”

   Answer: “yes”

9. Question: “The current public Rust methods on SimpleEngine use Option for every non-success case. The HTTP server interprets None as ‘unsupported,’ so it may forward the request to the fallback backend. To avoid breaking callers, we could leave those methods unchanged and add richer internal methods used only by the HTTP server and DAG cutover. But that means direct Rust callers still cannot distinguish unsupported from failed execution. Alternatively, change these methods to return Result<Option<...>, QueryExecutionError>: Ok(Some(result)) for local success; Ok(None) for unsupported and fallback allowed; Err(error) for execution/invalid-plan failure and fallback forbidden. That is the cleaner design, but it is an API change.”

   Answer: “Sure, make them return Result, that's ok”

10. Question: “how should malformed PromQL itself behave? My recommendation: preserve today’s behavior—treat parse failures as Ok(None), so the configured Prometheus fallback can parse and answer them as it does now. Reserve Err(QueryExecutionError) for a query that was accepted for native execution but failed while building or executing its DAG (including an invalid plan or store/operator failure). Do you agree?”

    Answer: “cool”

11. Question: “what should an execution failure look like at the HTTP boundary? My recommendation: return a local HTTP 500 response with a clear message such as native query execution failed: invalid query DAG: ...; do not call the fallback backend. The E2E regression test should configure a fallback mock, trigger a test-only malformed plan, assert the local error, and assert the mock received no request. Does that response contract work for you?”

    Answer: “ok”

12. Question: “should this be the required E2E matrix for the cutover? [tumbling instant/range; sliding exact cover; separate value/key aggregations; DeltaSet key replay; top-k across groups and timestamps; malformed test-only DAG with fallback configured; PromQL differential suite]. My recommendation is yes: add any missing cases to the real precompute fixture, compare legacy and DAG modes through the public methods during development, then run this matrix plus compliance in DAG-only mode before deleting legacy.”

    Answer: “any other scenario we can tests”

    Decision: Also cover self-keyed values, empty/missing data, expected native capability misses, store/operator failures, top-k ties, binary-query arms, SQL, and duplicate buckets. The agreed failure taxonomy is: unsupported/cannot execute natively permits fallback; invalid DAG or operational failure returns a local error with no fallback.

13. Question: “Use a reusable E2E fixture, not raw InferenceConfig in every test. InferenceConfig is one input: it tells query serving which PromQL query maps to which precomputed aggregation. But a real scenario needs four inputs: aggregation configs; query config/schema; remote-write samples; and an instant or range query request. The fixture can derive StreamingConfig and InferenceConfig from a concise case definition. My recommendation is to make NativeDagScenario the sole fixture input type. It keeps each test readable in domain terms while preventing test setup from duplicating config wiring. Does that fixture shape work for you?”

    Answer: “Nativedagscenario makes sense but what does ‘Only special cases add inputs:’ mean”

    Clarification: Every test starts with NativeDagScenario. Capability-miss and store/operator-failure tests add a recording fallback mock; invalid-DAG tests add a test-only malformed-plan override. These are harness controls, not production configuration fields.

    Answer: “ok”

14. Question: “The real precompute tests are integration tests under asap-query-engine/tests/. To let them create both a legacy-mode and DAG-mode engine, I recommend a narrowly scoped Cargo feature such as test-support. It exposes only the test fixture constructor/selector; normal builds cannot enable behavior selection accidentally, and no runtime config is added. Do you prefer that, or should the legacy-vs-DAG comparisons live inside the crate’s test module, where test-only internals are available without a feature?”

    Answer: “does it matter”

    Decision: Use the temporary test-support feature: it keeps differential tests with existing E2E tests, avoids duplicated fixture infrastructure, and is explicit in the test command.

15. Question: “because you chose a temporary legacy path, the test-support feature and legacy-vs-DAG differential tests are temporary too. They help us reach confidence during the refactor, then are removed before the PR lands. The final PR keeps only DAG execution in normal builds; the E2E behavior matrix in DAG-only mode; the PromQL differential suite; and the narrow malformed-graph validation test. That preserves your ‘no permanent duplicate pipeline’ decision. Is that the intended end state?”

    Answer: “ok”

16. Direction: “for error design, btw, feel free to make as many enums as you want.”

    Decision: Use explicit error enums rather than overloading Option or a generic string, with enough variants to distinguish unsupported/capability misses from invalid plans and operational failures.

[milindsrivastava1997]

Differential classification

The Docker runner is now isolated per invocation (unique Compose project and a past fixture base timestamp). The isolated off-grid run completes without lifecycle or remote-write failures; it reports a real parity mismatch: Prometheus returns 1.0, while ASAP returns 0.85/0.8 at off-grid evaluation points.

Evidence from the feature-enabled public E2E legacy-vs-DAG matrix:

• Sparse sum(checkout_up): legacy and DAG match. Docker mismatch predates DAG staging.
• Tumbling sum(data): legacy and DAG match. Docker mismatch predates DAG staging.
• Keyed count(data) by (job): legacy and DAG match. Docker mismatch predates DAG staging.
• Self-keyed topk(2, data), including tie behavior: legacy and DAG match. Docker mismatch predates DAG staging.
• Off-grid rate(...): isolated Docker mismatch reproduced, but it has no legacy-vs-DAG differential fixture yet. It remains unclassified and must not be attributed to the DAG cutover until that fixture exists.

The full make run-all command was started with the new isolation scheme. This host terminal detached after it began the second case, so it did not yield a single complete new all-case transcript; prior report artifacts plus the isolated off-grid rerun provide the classification above.

[milindsrivastava1997]

Automated code review: 5 findings (1 high, 2 medium, 2 low). See inline comments.

🤖 Generated with Claude Code