From 2f70f7bcd9fa666bb8da0fe1855e1ed05731a749 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Mon, 12 Oct 2026 22:53:53 -0400 Subject: [PATCH 01/12] feat(promql-compliance): let a suite supply fixed planner output Add an optional suite-level planner_output_dir holding streaming_config.yaml and inference_config.yaml. When set, the runner validates both files at load time, skips generating the planner input, and adds a Compose override that parks the planner behind an inactive profile and mounts the directory where the query engine reads planner output. This lets the harness exercise engine configs the planner does not produce yet. Co-Authored-By: Claude Opus 5.5 --- promql-compliance/ARCHITECTURE.md | 4 +- promql-compliance/HOW_TO.md | 19 ++++ promql-compliance/runner/config.go | 48 +++++++++- promql-compliance/runner/config_test.go | 119 ++++++++++++++++++++++++ promql-compliance/runner/run.go | 51 +++++++++- promql-compliance/runner/run_test.go | 80 ++++++++++++++++ 6 files changed, 316 insertions(+), 5 deletions(-) diff --git a/promql-compliance/ARCHITECTURE.md b/promql-compliance/ARCHITECTURE.md index e7da232a..23998fad 100644 --- a/promql-compliance/ARCHITECTURE.md +++ b/promql-compliance/ARCHITECTURE.md @@ -54,7 +54,9 @@ The runner performs these stages: are relative to this timestamp. 3. Generate temporary planner and engine configuration. 4. Start Prometheus and wait for its health check. -5. Run the planner. Its output is placed in a named shared volume. +5. Run the planner. Its output is placed in a named shared volume. A suite + with `planner_output_dir` skips the planner and mounts that directory + instead. 6. Start the ASAPQuery query engine after the planner completes. 7. Send the same encoded remote-write batches to Prometheus and ASAPQuery. 8. Wait for both query APIs to be ready and for the first probe query to return diff --git a/promql-compliance/HOW_TO.md b/promql-compliance/HOW_TO.md index 71bacc33..e05602c3 100644 --- a/promql-compliance/HOW_TO.md +++ b/promql-compliance/HOW_TO.md @@ -128,6 +128,25 @@ To change the five-minute cap or one-second ingestion assumption, change the runner’s generated-config logic in `runner/run.go`. Do not edit the generated configuration: it is temporary and is deleted after the run. +## Use fixed planner output + +By default the planner generates the engine's streaming and inference +configs from the suite. To test configs the planner does not produce, such as +roll-ups from a finer grouping, check the configs in next to the suite and +point the suite at their directory: + +```yaml +name: rollup +planner_output_dir: rollup-planner-output +``` + +The path is relative to the suite file. The directory must contain +`streaming_config.yaml` and `inference_config.yaml` in the planner's output +format; the runner fails at load time if either is missing. The planner +service then does not run, and the query engine reads the directory in place +of the planner's output volume. With already-running targets the field has no +effect. `suites/rollup.yaml` is an example. + ## Configure tolerance Set a suite-wide default: diff --git a/promql-compliance/runner/config.go b/promql-compliance/runner/config.go index a0857636..0c197a40 100644 --- a/promql-compliance/runner/config.go +++ b/promql-compliance/runner/config.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "os" + "path/filepath" "time" "gopkg.in/yaml.v3" @@ -15,6 +16,10 @@ const ( instantOrderDescending = "descending" orderGroupingBy = "by" orderGroupingWithout = "without" + + // File names the planner writes and the query engine reads. + plannerStreamingConfigFile = "streaming_config.yaml" + plannerInferenceConfigFile = "inference_config.yaml" ) // Suite is a data-independent collection of PromQL cases. All timestamps are @@ -23,6 +28,11 @@ type Suite struct { Name string `yaml:"name" json:"name"` ComparisonDefaults ComparisonPolicy `yaml:"comparison_defaults" json:"comparisonDefaults"` Queries []QueryCase `yaml:"queries" json:"queries"` + // PlannerOutputDir, when set, holds fixed streaming and inference configs + // that the query engine uses instead of running the planner. It is + // relative to the suite file; LoadSuiteFile resolves it to an absolute + // path. + PlannerOutputDir string `yaml:"planner_output_dir" json:"plannerOutputDir,omitempty"` } type QueryCase struct { @@ -123,13 +133,47 @@ func LoadSuite(contents []byte) (Suite, error) { return suite, nil } -// LoadSuiteFile reads a query suite from disk. +// LoadSuiteFile reads a query suite from disk and resolves its +// planner_output_dir against the suite file's directory. func LoadSuiteFile(path string) (Suite, error) { contents, err := os.ReadFile(path) if err != nil { return Suite{}, fmt.Errorf("read query suite %q: %w", path, err) } - return LoadSuite(contents) + suite, err := LoadSuite(contents) + if err != nil { + return Suite{}, err + } + if suite.PlannerOutputDir != "" { + resolved, err := resolvePlannerOutputDir(filepath.Dir(path), suite.PlannerOutputDir) + if err != nil { + return Suite{}, fmt.Errorf("query suite %q: %w", suite.Name, err) + } + suite.PlannerOutputDir = resolved + } + return suite, nil +} + +func resolvePlannerOutputDir(suiteDirectory, configured string) (string, error) { + directory := configured + if !filepath.IsAbs(directory) { + directory = filepath.Join(suiteDirectory, directory) + } + directory, err := filepath.Abs(directory) + if err != nil { + return "", fmt.Errorf("resolve planner_output_dir %q: %w", configured, err) + } + for _, name := range []string{plannerStreamingConfigFile, plannerInferenceConfigFile} { + file := filepath.Join(directory, name) + info, err := os.Stat(file) + if err != nil { + return "", fmt.Errorf("planner_output_dir %q: %w", configured, err) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("planner_output_dir %q: %s is not a regular file", configured, file) + } + } + return directory, nil } func (q QueryCase) InstantTimes(base time.Time) []time.Time { diff --git a/promql-compliance/runner/config_test.go b/promql-compliance/runner/config_test.go index d38d8a2d..8aa0a234 100644 --- a/promql-compliance/runner/config_test.go +++ b/promql-compliance/runner/config_test.go @@ -1,6 +1,10 @@ package runner import ( + "fmt" + "os" + "path/filepath" + "strings" "testing" "time" ) @@ -171,3 +175,118 @@ queries: t.Fatal("LoadSuite accepted an invalid instant vector order direction") } } + +const plannerOutputSuite = `name: fixed-output +planner_output_dir: %s +queries: + - name: up + expr: up + instant_offsets_seconds: [60] +` + +// writePlannerOutputSuite writes a suite under root/suites whose +// planner_output_dir is configured, plus the named files under +// root/suites/fixed. +func writePlannerOutputSuite(t *testing.T, configured string, files ...string) (string, string) { + t.Helper() + root := t.TempDir() + suiteDirectory := filepath.Join(root, "suites") + outputDirectory := filepath.Join(suiteDirectory, "fixed") + if err := os.MkdirAll(outputDirectory, 0o755); err != nil { + t.Fatalf("create planner output directory: %v", err) + } + for _, name := range files { + if err := os.WriteFile(filepath.Join(outputDirectory, name), []byte("{}\n"), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + suitePath := filepath.Join(suiteDirectory, "suite.yaml") + if err := os.WriteFile(suitePath, []byte(fmt.Sprintf(plannerOutputSuite, configured)), 0o600); err != nil { + t.Fatalf("write suite: %v", err) + } + return suitePath, outputDirectory +} + +func TestLoadSuiteFileResolvesPlannerOutputDirAgainstSuiteFile(t *testing.T) { + suitePath, outputDirectory := writePlannerOutputSuite(t, "fixed", plannerStreamingConfigFile, plannerInferenceConfigFile) + + // Load through a relative suite path so resolution cannot depend on the + // caller's working directory matching the suite's directory. + workingDirectory, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + relativeSuitePath, err := filepath.Rel(workingDirectory, suitePath) + if err != nil { + t.Fatalf("Rel: %v", err) + } + suite, err := LoadSuiteFile(relativeSuitePath) + if err != nil { + t.Fatalf("LoadSuiteFile: %v", err) + } + if got, want := suite.PlannerOutputDir, outputDirectory; got != want { + t.Fatalf("planner output dir = %q, want %q", got, want) + } +} + +func TestLoadSuiteFileAcceptsAbsolutePlannerOutputDir(t *testing.T) { + outputDirectory := t.TempDir() + for _, name := range []string{plannerStreamingConfigFile, plannerInferenceConfigFile} { + if err := os.WriteFile(filepath.Join(outputDirectory, name), []byte("{}\n"), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + suitePath, _ := writePlannerOutputSuite(t, outputDirectory) + + suite, err := LoadSuiteFile(suitePath) + if err != nil { + t.Fatalf("LoadSuiteFile: %v", err) + } + if got, want := suite.PlannerOutputDir, outputDirectory; got != want { + t.Fatalf("planner output dir = %q, want %q", got, want) + } +} + +func TestLoadSuiteFileLeavesPlannerOutputDirUnsetByDefault(t *testing.T) { + suite, err := LoadSuiteFile("../suites/temporal.yaml") + if err != nil { + t.Fatalf("LoadSuiteFile: %v", err) + } + if suite.PlannerOutputDir != "" { + t.Fatalf("planner output dir = %q, want empty", suite.PlannerOutputDir) + } +} + +func TestLoadSuiteFileRejectsIncompletePlannerOutputDir(t *testing.T) { + for _, test := range []struct { + name string + configured string + files []string + missing string + }{ + {"missing inference config", "fixed", []string{plannerStreamingConfigFile}, plannerInferenceConfigFile}, + {"missing streaming config", "fixed", []string{plannerInferenceConfigFile}, plannerStreamingConfigFile}, + {"missing directory", "absent", nil, plannerStreamingConfigFile}, + } { + t.Run(test.name, func(t *testing.T) { + suitePath, _ := writePlannerOutputSuite(t, test.configured, test.files...) + _, err := LoadSuiteFile(suitePath) + if err == nil { + t.Fatal("LoadSuiteFile accepted a planner_output_dir without both config files") + } + if !strings.Contains(err.Error(), test.missing) { + t.Fatalf("error %q does not name the missing file %q", err, test.missing) + } + }) + } +} + +func TestLoadSuiteFileRejectsPlannerOutputDirectoryInPlaceOfFile(t *testing.T) { + suitePath, outputDirectory := writePlannerOutputSuite(t, "fixed", plannerStreamingConfigFile) + if err := os.Mkdir(filepath.Join(outputDirectory, plannerInferenceConfigFile), 0o755); err != nil { + t.Fatalf("create directory: %v", err) + } + if _, err := LoadSuiteFile(suitePath); err == nil { + t.Fatal("LoadSuiteFile accepted a directory as a planner config file") + } +} diff --git a/promql-compliance/runner/run.go b/promql-compliance/runner/run.go index c229a4eb..595e4af3 100644 --- a/promql-compliance/runner/run.go +++ b/promql-compliance/runner/run.go @@ -27,8 +27,27 @@ const ( dataReadyTimeout = 3 * time.Minute readinessPollInterval = time.Second composeShutdownTimeout = time.Minute + + plannerOutputMountPath = "/asap-planner-output" + fixedPlannerOutputComposeFile = "docker-compose.fixed-planner-output.yml" + fixedPlannerOutputDirEnv = "ASAP_FIXED_PLANNER_OUTPUT_DIR" ) +// fixedPlannerOutputCompose overrides the base Compose file for suites with a +// planner_output_dir: the planner sits behind a never-activated profile so it +// does not run, and the query engine mounts the fixed directory where the +// planner's output volume would be. +const fixedPlannerOutputCompose = `services: + planner: + profiles: ["generated-planner-output"] + queryengine: + depends_on: !override + prometheus: + condition: service_healthy + volumes: + - "${` + fixedPlannerOutputDirEnv + `:?` + fixedPlannerOutputDirEnv + ` is required}:` + plannerOutputMountPath + `:ro" +` + type RunOptions struct { DatasetPath string SuitePath string @@ -79,6 +98,13 @@ func Run(ctx context.Context, options RunOptions) (Report, error) { project: options.ComposeProject, env: []string{"ASAP_TEST_CONFIG_DIR=" + runDirectory}, } + if suite.PlannerOutputDir != "" { + lifecycle.files = append( + append([]string(nil), options.ComposeFiles...), + filepath.Join(runDirectory, fixedPlannerOutputComposeFile), + ) + lifecycle.env = append(lifecycle.env, fixedPlannerOutputDirEnv+"="+suite.PlannerOutputDir) + } if len(options.ComposeFiles) > 0 { if err := lifecycle.Start(ctx); err != nil { return Report{}, err @@ -345,7 +371,24 @@ type plannerCleanup struct { Policy string `yaml:"policy"` } +// writeGeneratedConfigs writes the engine config plus either the planner input +// or, for a suite with fixed planner output, the Compose override that skips +// the planner. The planner input is not generated in the fixed case, so its +// timing derivation cannot reject queries the planner never sees. func writeGeneratedConfigs(directory string, fixture seeder.Fixture, suite Suite) error { + if err := writeEngineConfig(directory); err != nil { + return err + } + if suite.PlannerOutputDir != "" { + if err := os.WriteFile(filepath.Join(directory, fixedPlannerOutputComposeFile), []byte(fixedPlannerOutputCompose), 0o600); err != nil { + return fmt.Errorf("write fixed planner output compose override: %w", err) + } + return nil + } + return writePlannerConfig(directory, fixture, suite) +} + +func writePlannerConfig(directory string, fixture seeder.Fixture, suite Suite) error { metrics := make(map[string]map[string]struct{}) for _, series := range fixture.Series { if metrics[series.Metric] == nil { @@ -399,6 +442,10 @@ func writeGeneratedConfigs(directory string, fixture seeder.Fixture, suite Suite if err := os.WriteFile(filepath.Join(directory, "controller-config.yaml"), contents, 0o600); err != nil { return fmt.Errorf("write generated planner config: %w", err) } + return nil +} + +func writeEngineConfig(directory string) error { engineConfig := []byte(`output_dir: "/app/outputs" log_level: "INFO" data_ingestion_interval_ms: 1000 @@ -414,8 +461,8 @@ store: ingest: type: "http_remote_write" port: 9091 -inference_config: "/asap-planner-output/inference_config.yaml" -streaming_config: "/asap-planner-output/streaming_config.yaml" +inference_config: "` + plannerOutputMountPath + `/` + plannerInferenceConfigFile + `" +streaming_config: "` + plannerOutputMountPath + `/` + plannerStreamingConfigFile + `" `) if err := os.WriteFile(filepath.Join(directory, "engine_config.yaml"), engineConfig, 0o600); err != nil { return fmt.Errorf("write generated engine config: %w", err) diff --git a/promql-compliance/runner/run_test.go b/promql-compliance/runner/run_test.go index 825efc8e..a7bcc188 100644 --- a/promql-compliance/runner/run_test.go +++ b/promql-compliance/runner/run_test.go @@ -159,3 +159,83 @@ func TestGeneratedPlannerGroupsUseCompatibleQueryTiming(t *testing.T) { t.Fatalf("short-rate range_duration_ms = %v, want 60000", planner.QueryGroups[1].RangeDurationMS) } } + +func TestGeneratedConfigsWithFixedPlannerOutputSkipPlanner(t *testing.T) { + directory := t.TempDir() + fixture := seeder.Fixture{ + Name: "single-series", + Series: []seeder.FixtureSeries{{ + Metric: "up", + Labels: map[string]string{"job": "test"}, + Samples: []seeder.FixtureSample{{OffsetSeconds: 0, Value: 1}}, + }}, + } + suite := Suite{ + Name: "fixed-output", + Queries: []QueryCase{{Name: "up", Expr: "up"}}, + PlannerOutputDir: "/fixed/planner/output", + } + + if err := writeGeneratedConfigs(directory, fixture, suite); err != nil { + t.Fatalf("writeGeneratedConfigs: %v", err) + } + + if _, err := os.Stat(filepath.Join(directory, "controller-config.yaml")); !os.IsNotExist(err) { + t.Fatalf("planner config was generated for a suite with fixed planner output (stat err: %v)", err) + } + + overrideContents, err := os.ReadFile(filepath.Join(directory, fixedPlannerOutputComposeFile)) + if err != nil { + t.Fatalf("read compose override: %v", err) + } + var override struct { + Services struct { + Planner struct { + Profiles []string `yaml:"profiles"` + } `yaml:"planner"` + QueryEngine struct { + DependsOn yaml.Node `yaml:"depends_on"` + Volumes []string `yaml:"volumes"` + } `yaml:"queryengine"` + } `yaml:"services"` + } + if err := yaml.Unmarshal(overrideContents, &override); err != nil { + t.Fatalf("parse compose override: %v", err) + } + if len(override.Services.Planner.Profiles) == 0 { + t.Fatal("compose override leaves the planner service active") + } + dependsOn := override.Services.QueryEngine.DependsOn + if dependsOn.Tag != "!override" { + t.Fatalf("queryengine depends_on tag = %q, want !override so the planner dependency is dropped", dependsOn.Tag) + } + var dependencies map[string]any + if err := dependsOn.Decode(&dependencies); err != nil { + t.Fatalf("decode queryengine depends_on: %v", err) + } + if _, ok := dependencies["planner"]; ok { + t.Fatal("queryengine still depends on the planner") + } + wantVolume := "${" + fixedPlannerOutputDirEnv + ":?" + fixedPlannerOutputDirEnv + " is required}:" + plannerOutputMountPath + ":ro" + if got := override.Services.QueryEngine.Volumes; len(got) != 1 || got[0] != wantVolume { + t.Fatalf("queryengine volumes = %v, want [%s]", got, wantVolume) + } + + engineContents, err := os.ReadFile(filepath.Join(directory, "engine_config.yaml")) + if err != nil { + t.Fatalf("read engine config: %v", err) + } + var engine struct { + InferenceConfig string `yaml:"inference_config"` + StreamingConfig string `yaml:"streaming_config"` + } + if err := yaml.Unmarshal(engineContents, &engine); err != nil { + t.Fatalf("parse engine config: %v", err) + } + if got, want := engine.InferenceConfig, plannerOutputMountPath+"/"+plannerInferenceConfigFile; got != want { + t.Fatalf("engine inference_config = %q, want %q", got, want) + } + if got, want := engine.StreamingConfig, plannerOutputMountPath+"/"+plannerStreamingConfigFile; got != want { + t.Fatalf("engine streaming_config = %q, want %q", got, want) + } +} From 901cf843c2fbfdf1bbb9517a459acd85f7ad1a76 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Mon, 12 Oct 2026 22:53:55 -0400 Subject: [PATCH 02/12] test(promql-compliance): add roll-up suite with fixed planner output The rollup suite reads KLL, DDSketch, Sum and MinMax aggregations grouped by (service, endpoint), over tumbling and sliding windows, through queries grouped by (service, endpoint), by (service) and with no grouping. The dataset includes an endpoint that starts mid-range. Co-Authored-By: Claude Opus 5.5 --- promql-compliance/datasets/rollup.yaml | 569 ++++++++++++++++++ promql-compliance/runner/Makefile | 3 +- .../checked_in_fixture_validation_test.go | 47 ++ .../inference_config.yaml | 117 ++++ .../streaming_config.yaml | 113 ++++ promql-compliance/suites/rollup.yaml | 125 ++++ 6 files changed, 973 insertions(+), 1 deletion(-) create mode 100644 promql-compliance/datasets/rollup.yaml create mode 100644 promql-compliance/suites/rollup-planner-output/inference_config.yaml create mode 100644 promql-compliance/suites/rollup-planner-output/streaming_config.yaml create mode 100644 promql-compliance/suites/rollup.yaml diff --git a/promql-compliance/datasets/rollup.yaml b/promql-compliance/datasets/rollup.yaml new file mode 100644 index 00000000..9091e645 --- /dev/null +++ b/promql-compliance/datasets/rollup.yaml @@ -0,0 +1,569 @@ +name: rollup +series: + # rollup_latency{service, endpoint}: 2 services x 10 endpoints at 60s + # cadence, plus endpoint e-late on service a that starts at offset 600 so + # roll-ups see a subpopulation appear mid-range. + # + # With i = endpoint*2 + service (e-late is i = 20), value = 1000 + 2*i + + # 10*(t/60). Within one service adjacent values are 4 apart (under 0.5%), + # which bounds the gap between ASAPQuery's nearest-rank quantile and + # Prometheus' interpolated one well inside the suite's sketch tolerance. + # + # Samples run to offset 1260, past the last evaluated offset (1200). + - metric: rollup_latency + labels: + service: a + endpoint: e00 + samples: + - {offset_seconds: 0, value: 1000} + - {offset_seconds: 60, value: 1010} + - {offset_seconds: 120, value: 1020} + - {offset_seconds: 180, value: 1030} + - {offset_seconds: 240, value: 1040} + - {offset_seconds: 300, value: 1050} + - {offset_seconds: 360, value: 1060} + - {offset_seconds: 420, value: 1070} + - {offset_seconds: 480, value: 1080} + - {offset_seconds: 540, value: 1090} + - {offset_seconds: 600, value: 1100} + - {offset_seconds: 660, value: 1110} + - {offset_seconds: 720, value: 1120} + - {offset_seconds: 780, value: 1130} + - {offset_seconds: 840, value: 1140} + - {offset_seconds: 900, value: 1150} + - {offset_seconds: 960, value: 1160} + - {offset_seconds: 1020, value: 1170} + - {offset_seconds: 1080, value: 1180} + - {offset_seconds: 1140, value: 1190} + - {offset_seconds: 1200, value: 1200} + - {offset_seconds: 1260, value: 1210} + - metric: rollup_latency + labels: + service: b + endpoint: e00 + samples: + - {offset_seconds: 0, value: 1002} + - {offset_seconds: 60, value: 1012} + - {offset_seconds: 120, value: 1022} + - {offset_seconds: 180, value: 1032} + - {offset_seconds: 240, value: 1042} + - {offset_seconds: 300, value: 1052} + - {offset_seconds: 360, value: 1062} + - {offset_seconds: 420, value: 1072} + - {offset_seconds: 480, value: 1082} + - {offset_seconds: 540, value: 1092} + - {offset_seconds: 600, value: 1102} + - {offset_seconds: 660, value: 1112} + - {offset_seconds: 720, value: 1122} + - {offset_seconds: 780, value: 1132} + - {offset_seconds: 840, value: 1142} + - {offset_seconds: 900, value: 1152} + - {offset_seconds: 960, value: 1162} + - {offset_seconds: 1020, value: 1172} + - {offset_seconds: 1080, value: 1182} + - {offset_seconds: 1140, value: 1192} + - {offset_seconds: 1200, value: 1202} + - {offset_seconds: 1260, value: 1212} + - metric: rollup_latency + labels: + service: a + endpoint: e01 + samples: + - {offset_seconds: 0, value: 1004} + - {offset_seconds: 60, value: 1014} + - {offset_seconds: 120, value: 1024} + - {offset_seconds: 180, value: 1034} + - {offset_seconds: 240, value: 1044} + - {offset_seconds: 300, value: 1054} + - {offset_seconds: 360, value: 1064} + - {offset_seconds: 420, value: 1074} + - {offset_seconds: 480, value: 1084} + - {offset_seconds: 540, value: 1094} + - {offset_seconds: 600, value: 1104} + - {offset_seconds: 660, value: 1114} + - {offset_seconds: 720, value: 1124} + - {offset_seconds: 780, value: 1134} + - {offset_seconds: 840, value: 1144} + - {offset_seconds: 900, value: 1154} + - {offset_seconds: 960, value: 1164} + - {offset_seconds: 1020, value: 1174} + - {offset_seconds: 1080, value: 1184} + - {offset_seconds: 1140, value: 1194} + - {offset_seconds: 1200, value: 1204} + - {offset_seconds: 1260, value: 1214} + - metric: rollup_latency + labels: + service: b + endpoint: e01 + samples: + - {offset_seconds: 0, value: 1006} + - {offset_seconds: 60, value: 1016} + - {offset_seconds: 120, value: 1026} + - {offset_seconds: 180, value: 1036} + - {offset_seconds: 240, value: 1046} + - {offset_seconds: 300, value: 1056} + - {offset_seconds: 360, value: 1066} + - {offset_seconds: 420, value: 1076} + - {offset_seconds: 480, value: 1086} + - {offset_seconds: 540, value: 1096} + - {offset_seconds: 600, value: 1106} + - {offset_seconds: 660, value: 1116} + - {offset_seconds: 720, value: 1126} + - {offset_seconds: 780, value: 1136} + - {offset_seconds: 840, value: 1146} + - {offset_seconds: 900, value: 1156} + - {offset_seconds: 960, value: 1166} + - {offset_seconds: 1020, value: 1176} + - {offset_seconds: 1080, value: 1186} + - {offset_seconds: 1140, value: 1196} + - {offset_seconds: 1200, value: 1206} + - {offset_seconds: 1260, value: 1216} + - metric: rollup_latency + labels: + service: a + endpoint: e02 + samples: + - {offset_seconds: 0, value: 1008} + - {offset_seconds: 60, value: 1018} + - {offset_seconds: 120, value: 1028} + - {offset_seconds: 180, value: 1038} + - {offset_seconds: 240, value: 1048} + - {offset_seconds: 300, value: 1058} + - {offset_seconds: 360, value: 1068} + - {offset_seconds: 420, value: 1078} + - {offset_seconds: 480, value: 1088} + - {offset_seconds: 540, value: 1098} + - {offset_seconds: 600, value: 1108} + - {offset_seconds: 660, value: 1118} + - {offset_seconds: 720, value: 1128} + - {offset_seconds: 780, value: 1138} + - {offset_seconds: 840, value: 1148} + - {offset_seconds: 900, value: 1158} + - {offset_seconds: 960, value: 1168} + - {offset_seconds: 1020, value: 1178} + - {offset_seconds: 1080, value: 1188} + - {offset_seconds: 1140, value: 1198} + - {offset_seconds: 1200, value: 1208} + - {offset_seconds: 1260, value: 1218} + - metric: rollup_latency + labels: + service: b + endpoint: e02 + samples: + - {offset_seconds: 0, value: 1010} + - {offset_seconds: 60, value: 1020} + - {offset_seconds: 120, value: 1030} + - {offset_seconds: 180, value: 1040} + - {offset_seconds: 240, value: 1050} + - {offset_seconds: 300, value: 1060} + - {offset_seconds: 360, value: 1070} + - {offset_seconds: 420, value: 1080} + - {offset_seconds: 480, value: 1090} + - {offset_seconds: 540, value: 1100} + - {offset_seconds: 600, value: 1110} + - {offset_seconds: 660, value: 1120} + - {offset_seconds: 720, value: 1130} + - {offset_seconds: 780, value: 1140} + - {offset_seconds: 840, value: 1150} + - {offset_seconds: 900, value: 1160} + - {offset_seconds: 960, value: 1170} + - {offset_seconds: 1020, value: 1180} + - {offset_seconds: 1080, value: 1190} + - {offset_seconds: 1140, value: 1200} + - {offset_seconds: 1200, value: 1210} + - {offset_seconds: 1260, value: 1220} + - metric: rollup_latency + labels: + service: a + endpoint: e03 + samples: + - {offset_seconds: 0, value: 1012} + - {offset_seconds: 60, value: 1022} + - {offset_seconds: 120, value: 1032} + - {offset_seconds: 180, value: 1042} + - {offset_seconds: 240, value: 1052} + - {offset_seconds: 300, value: 1062} + - {offset_seconds: 360, value: 1072} + - {offset_seconds: 420, value: 1082} + - {offset_seconds: 480, value: 1092} + - {offset_seconds: 540, value: 1102} + - {offset_seconds: 600, value: 1112} + - {offset_seconds: 660, value: 1122} + - {offset_seconds: 720, value: 1132} + - {offset_seconds: 780, value: 1142} + - {offset_seconds: 840, value: 1152} + - {offset_seconds: 900, value: 1162} + - {offset_seconds: 960, value: 1172} + - {offset_seconds: 1020, value: 1182} + - {offset_seconds: 1080, value: 1192} + - {offset_seconds: 1140, value: 1202} + - {offset_seconds: 1200, value: 1212} + - {offset_seconds: 1260, value: 1222} + - metric: rollup_latency + labels: + service: b + endpoint: e03 + samples: + - {offset_seconds: 0, value: 1014} + - {offset_seconds: 60, value: 1024} + - {offset_seconds: 120, value: 1034} + - {offset_seconds: 180, value: 1044} + - {offset_seconds: 240, value: 1054} + - {offset_seconds: 300, value: 1064} + - {offset_seconds: 360, value: 1074} + - {offset_seconds: 420, value: 1084} + - {offset_seconds: 480, value: 1094} + - {offset_seconds: 540, value: 1104} + - {offset_seconds: 600, value: 1114} + - {offset_seconds: 660, value: 1124} + - {offset_seconds: 720, value: 1134} + - {offset_seconds: 780, value: 1144} + - {offset_seconds: 840, value: 1154} + - {offset_seconds: 900, value: 1164} + - {offset_seconds: 960, value: 1174} + - {offset_seconds: 1020, value: 1184} + - {offset_seconds: 1080, value: 1194} + - {offset_seconds: 1140, value: 1204} + - {offset_seconds: 1200, value: 1214} + - {offset_seconds: 1260, value: 1224} + - metric: rollup_latency + labels: + service: a + endpoint: e04 + samples: + - {offset_seconds: 0, value: 1016} + - {offset_seconds: 60, value: 1026} + - {offset_seconds: 120, value: 1036} + - {offset_seconds: 180, value: 1046} + - {offset_seconds: 240, value: 1056} + - {offset_seconds: 300, value: 1066} + - {offset_seconds: 360, value: 1076} + - {offset_seconds: 420, value: 1086} + - {offset_seconds: 480, value: 1096} + - {offset_seconds: 540, value: 1106} + - {offset_seconds: 600, value: 1116} + - {offset_seconds: 660, value: 1126} + - {offset_seconds: 720, value: 1136} + - {offset_seconds: 780, value: 1146} + - {offset_seconds: 840, value: 1156} + - {offset_seconds: 900, value: 1166} + - {offset_seconds: 960, value: 1176} + - {offset_seconds: 1020, value: 1186} + - {offset_seconds: 1080, value: 1196} + - {offset_seconds: 1140, value: 1206} + - {offset_seconds: 1200, value: 1216} + - {offset_seconds: 1260, value: 1226} + - metric: rollup_latency + labels: + service: b + endpoint: e04 + samples: + - {offset_seconds: 0, value: 1018} + - {offset_seconds: 60, value: 1028} + - {offset_seconds: 120, value: 1038} + - {offset_seconds: 180, value: 1048} + - {offset_seconds: 240, value: 1058} + - {offset_seconds: 300, value: 1068} + - {offset_seconds: 360, value: 1078} + - {offset_seconds: 420, value: 1088} + - {offset_seconds: 480, value: 1098} + - {offset_seconds: 540, value: 1108} + - {offset_seconds: 600, value: 1118} + - {offset_seconds: 660, value: 1128} + - {offset_seconds: 720, value: 1138} + - {offset_seconds: 780, value: 1148} + - {offset_seconds: 840, value: 1158} + - {offset_seconds: 900, value: 1168} + - {offset_seconds: 960, value: 1178} + - {offset_seconds: 1020, value: 1188} + - {offset_seconds: 1080, value: 1198} + - {offset_seconds: 1140, value: 1208} + - {offset_seconds: 1200, value: 1218} + - {offset_seconds: 1260, value: 1228} + - metric: rollup_latency + labels: + service: a + endpoint: e05 + samples: + - {offset_seconds: 0, value: 1020} + - {offset_seconds: 60, value: 1030} + - {offset_seconds: 120, value: 1040} + - {offset_seconds: 180, value: 1050} + - {offset_seconds: 240, value: 1060} + - {offset_seconds: 300, value: 1070} + - {offset_seconds: 360, value: 1080} + - {offset_seconds: 420, value: 1090} + - {offset_seconds: 480, value: 1100} + - {offset_seconds: 540, value: 1110} + - {offset_seconds: 600, value: 1120} + - {offset_seconds: 660, value: 1130} + - {offset_seconds: 720, value: 1140} + - {offset_seconds: 780, value: 1150} + - {offset_seconds: 840, value: 1160} + - {offset_seconds: 900, value: 1170} + - {offset_seconds: 960, value: 1180} + - {offset_seconds: 1020, value: 1190} + - {offset_seconds: 1080, value: 1200} + - {offset_seconds: 1140, value: 1210} + - {offset_seconds: 1200, value: 1220} + - {offset_seconds: 1260, value: 1230} + - metric: rollup_latency + labels: + service: b + endpoint: e05 + samples: + - {offset_seconds: 0, value: 1022} + - {offset_seconds: 60, value: 1032} + - {offset_seconds: 120, value: 1042} + - {offset_seconds: 180, value: 1052} + - {offset_seconds: 240, value: 1062} + - {offset_seconds: 300, value: 1072} + - {offset_seconds: 360, value: 1082} + - {offset_seconds: 420, value: 1092} + - {offset_seconds: 480, value: 1102} + - {offset_seconds: 540, value: 1112} + - {offset_seconds: 600, value: 1122} + - {offset_seconds: 660, value: 1132} + - {offset_seconds: 720, value: 1142} + - {offset_seconds: 780, value: 1152} + - {offset_seconds: 840, value: 1162} + - {offset_seconds: 900, value: 1172} + - {offset_seconds: 960, value: 1182} + - {offset_seconds: 1020, value: 1192} + - {offset_seconds: 1080, value: 1202} + - {offset_seconds: 1140, value: 1212} + - {offset_seconds: 1200, value: 1222} + - {offset_seconds: 1260, value: 1232} + - metric: rollup_latency + labels: + service: a + endpoint: e06 + samples: + - {offset_seconds: 0, value: 1024} + - {offset_seconds: 60, value: 1034} + - {offset_seconds: 120, value: 1044} + - {offset_seconds: 180, value: 1054} + - {offset_seconds: 240, value: 1064} + - {offset_seconds: 300, value: 1074} + - {offset_seconds: 360, value: 1084} + - {offset_seconds: 420, value: 1094} + - {offset_seconds: 480, value: 1104} + - {offset_seconds: 540, value: 1114} + - {offset_seconds: 600, value: 1124} + - {offset_seconds: 660, value: 1134} + - {offset_seconds: 720, value: 1144} + - {offset_seconds: 780, value: 1154} + - {offset_seconds: 840, value: 1164} + - {offset_seconds: 900, value: 1174} + - {offset_seconds: 960, value: 1184} + - {offset_seconds: 1020, value: 1194} + - {offset_seconds: 1080, value: 1204} + - {offset_seconds: 1140, value: 1214} + - {offset_seconds: 1200, value: 1224} + - {offset_seconds: 1260, value: 1234} + - metric: rollup_latency + labels: + service: b + endpoint: e06 + samples: + - {offset_seconds: 0, value: 1026} + - {offset_seconds: 60, value: 1036} + - {offset_seconds: 120, value: 1046} + - {offset_seconds: 180, value: 1056} + - {offset_seconds: 240, value: 1066} + - {offset_seconds: 300, value: 1076} + - {offset_seconds: 360, value: 1086} + - {offset_seconds: 420, value: 1096} + - {offset_seconds: 480, value: 1106} + - {offset_seconds: 540, value: 1116} + - {offset_seconds: 600, value: 1126} + - {offset_seconds: 660, value: 1136} + - {offset_seconds: 720, value: 1146} + - {offset_seconds: 780, value: 1156} + - {offset_seconds: 840, value: 1166} + - {offset_seconds: 900, value: 1176} + - {offset_seconds: 960, value: 1186} + - {offset_seconds: 1020, value: 1196} + - {offset_seconds: 1080, value: 1206} + - {offset_seconds: 1140, value: 1216} + - {offset_seconds: 1200, value: 1226} + - {offset_seconds: 1260, value: 1236} + - metric: rollup_latency + labels: + service: a + endpoint: e07 + samples: + - {offset_seconds: 0, value: 1028} + - {offset_seconds: 60, value: 1038} + - {offset_seconds: 120, value: 1048} + - {offset_seconds: 180, value: 1058} + - {offset_seconds: 240, value: 1068} + - {offset_seconds: 300, value: 1078} + - {offset_seconds: 360, value: 1088} + - {offset_seconds: 420, value: 1098} + - {offset_seconds: 480, value: 1108} + - {offset_seconds: 540, value: 1118} + - {offset_seconds: 600, value: 1128} + - {offset_seconds: 660, value: 1138} + - {offset_seconds: 720, value: 1148} + - {offset_seconds: 780, value: 1158} + - {offset_seconds: 840, value: 1168} + - {offset_seconds: 900, value: 1178} + - {offset_seconds: 960, value: 1188} + - {offset_seconds: 1020, value: 1198} + - {offset_seconds: 1080, value: 1208} + - {offset_seconds: 1140, value: 1218} + - {offset_seconds: 1200, value: 1228} + - {offset_seconds: 1260, value: 1238} + - metric: rollup_latency + labels: + service: b + endpoint: e07 + samples: + - {offset_seconds: 0, value: 1030} + - {offset_seconds: 60, value: 1040} + - {offset_seconds: 120, value: 1050} + - {offset_seconds: 180, value: 1060} + - {offset_seconds: 240, value: 1070} + - {offset_seconds: 300, value: 1080} + - {offset_seconds: 360, value: 1090} + - {offset_seconds: 420, value: 1100} + - {offset_seconds: 480, value: 1110} + - {offset_seconds: 540, value: 1120} + - {offset_seconds: 600, value: 1130} + - {offset_seconds: 660, value: 1140} + - {offset_seconds: 720, value: 1150} + - {offset_seconds: 780, value: 1160} + - {offset_seconds: 840, value: 1170} + - {offset_seconds: 900, value: 1180} + - {offset_seconds: 960, value: 1190} + - {offset_seconds: 1020, value: 1200} + - {offset_seconds: 1080, value: 1210} + - {offset_seconds: 1140, value: 1220} + - {offset_seconds: 1200, value: 1230} + - {offset_seconds: 1260, value: 1240} + - metric: rollup_latency + labels: + service: a + endpoint: e08 + samples: + - {offset_seconds: 0, value: 1032} + - {offset_seconds: 60, value: 1042} + - {offset_seconds: 120, value: 1052} + - {offset_seconds: 180, value: 1062} + - {offset_seconds: 240, value: 1072} + - {offset_seconds: 300, value: 1082} + - {offset_seconds: 360, value: 1092} + - {offset_seconds: 420, value: 1102} + - {offset_seconds: 480, value: 1112} + - {offset_seconds: 540, value: 1122} + - {offset_seconds: 600, value: 1132} + - {offset_seconds: 660, value: 1142} + - {offset_seconds: 720, value: 1152} + - {offset_seconds: 780, value: 1162} + - {offset_seconds: 840, value: 1172} + - {offset_seconds: 900, value: 1182} + - {offset_seconds: 960, value: 1192} + - {offset_seconds: 1020, value: 1202} + - {offset_seconds: 1080, value: 1212} + - {offset_seconds: 1140, value: 1222} + - {offset_seconds: 1200, value: 1232} + - {offset_seconds: 1260, value: 1242} + - metric: rollup_latency + labels: + service: b + endpoint: e08 + samples: + - {offset_seconds: 0, value: 1034} + - {offset_seconds: 60, value: 1044} + - {offset_seconds: 120, value: 1054} + - {offset_seconds: 180, value: 1064} + - {offset_seconds: 240, value: 1074} + - {offset_seconds: 300, value: 1084} + - {offset_seconds: 360, value: 1094} + - {offset_seconds: 420, value: 1104} + - {offset_seconds: 480, value: 1114} + - {offset_seconds: 540, value: 1124} + - {offset_seconds: 600, value: 1134} + - {offset_seconds: 660, value: 1144} + - {offset_seconds: 720, value: 1154} + - {offset_seconds: 780, value: 1164} + - {offset_seconds: 840, value: 1174} + - {offset_seconds: 900, value: 1184} + - {offset_seconds: 960, value: 1194} + - {offset_seconds: 1020, value: 1204} + - {offset_seconds: 1080, value: 1214} + - {offset_seconds: 1140, value: 1224} + - {offset_seconds: 1200, value: 1234} + - {offset_seconds: 1260, value: 1244} + - metric: rollup_latency + labels: + service: a + endpoint: e09 + samples: + - {offset_seconds: 0, value: 1036} + - {offset_seconds: 60, value: 1046} + - {offset_seconds: 120, value: 1056} + - {offset_seconds: 180, value: 1066} + - {offset_seconds: 240, value: 1076} + - {offset_seconds: 300, value: 1086} + - {offset_seconds: 360, value: 1096} + - {offset_seconds: 420, value: 1106} + - {offset_seconds: 480, value: 1116} + - {offset_seconds: 540, value: 1126} + - {offset_seconds: 600, value: 1136} + - {offset_seconds: 660, value: 1146} + - {offset_seconds: 720, value: 1156} + - {offset_seconds: 780, value: 1166} + - {offset_seconds: 840, value: 1176} + - {offset_seconds: 900, value: 1186} + - {offset_seconds: 960, value: 1196} + - {offset_seconds: 1020, value: 1206} + - {offset_seconds: 1080, value: 1216} + - {offset_seconds: 1140, value: 1226} + - {offset_seconds: 1200, value: 1236} + - {offset_seconds: 1260, value: 1246} + - metric: rollup_latency + labels: + service: b + endpoint: e09 + samples: + - {offset_seconds: 0, value: 1038} + - {offset_seconds: 60, value: 1048} + - {offset_seconds: 120, value: 1058} + - {offset_seconds: 180, value: 1068} + - {offset_seconds: 240, value: 1078} + - {offset_seconds: 300, value: 1088} + - {offset_seconds: 360, value: 1098} + - {offset_seconds: 420, value: 1108} + - {offset_seconds: 480, value: 1118} + - {offset_seconds: 540, value: 1128} + - {offset_seconds: 600, value: 1138} + - {offset_seconds: 660, value: 1148} + - {offset_seconds: 720, value: 1158} + - {offset_seconds: 780, value: 1168} + - {offset_seconds: 840, value: 1178} + - {offset_seconds: 900, value: 1188} + - {offset_seconds: 960, value: 1198} + - {offset_seconds: 1020, value: 1208} + - {offset_seconds: 1080, value: 1218} + - {offset_seconds: 1140, value: 1228} + - {offset_seconds: 1200, value: 1238} + - {offset_seconds: 1260, value: 1248} + - metric: rollup_latency + labels: + service: a + endpoint: e-late + samples: + - {offset_seconds: 600, value: 1140} + - {offset_seconds: 660, value: 1150} + - {offset_seconds: 720, value: 1160} + - {offset_seconds: 780, value: 1170} + - {offset_seconds: 840, value: 1180} + - {offset_seconds: 900, value: 1190} + - {offset_seconds: 960, value: 1200} + - {offset_seconds: 1020, value: 1210} + - {offset_seconds: 1080, value: 1220} + - {offset_seconds: 1140, value: 1230} + - {offset_seconds: 1200, value: 1240} + - {offset_seconds: 1260, value: 1250} diff --git a/promql-compliance/runner/Makefile b/promql-compliance/runner/Makefile index 0461ac2e..3f501175 100644 --- a/promql-compliance/runner/Makefile +++ b/promql-compliance/runner/Makefile @@ -13,7 +13,8 @@ CI_CASES := \ aggregations-native-dag:../datasets/aggregations.yaml:../suites/native-dag-aggregations.yaml \ aggregations:../datasets/aggregations.yaml:../suites/aggregations.yaml \ topk-ordering:../datasets/topk-ordering.yaml:../suites/topk-ordering.yaml \ - quantiles:../datasets/quantiles.yaml:../suites/quantiles.yaml + quantiles:../datasets/quantiles.yaml:../suites/quantiles.yaml \ + rollup:../datasets/rollup.yaml:../suites/rollup.yaml # Tracks planner coverage (#738); most queries are expected to fail. NON_CI_CASES := \ diff --git a/promql-compliance/runner/checked_in_fixture_validation_test.go b/promql-compliance/runner/checked_in_fixture_validation_test.go index dea0a89c..8eb38c00 100644 --- a/promql-compliance/runner/checked_in_fixture_validation_test.go +++ b/promql-compliance/runner/checked_in_fixture_validation_test.go @@ -1,10 +1,13 @@ package runner import ( + "os" + "path/filepath" "testing" "github.com/ProjectASAP/ASAPQuery/promql-compliance/seeder" promqlparser "github.com/prometheus/prometheus/promql/parser" + "gopkg.in/yaml.v3" ) func TestCheckedInFixturesAndSuites(t *testing.T) { @@ -80,3 +83,47 @@ func TestCheckedInNativeDagSuites(t *testing.T) { } } } + +// Every rollup suite query must have a query config in the fixed planner +// output; a mismatched string would make the engine reject it rather than +// exercise a roll-up. +func TestCheckedInRollupSuiteMatchesFixedPlannerOutput(t *testing.T) { + if _, err := seeder.LoadFixture("../datasets/rollup.yaml"); err != nil { + t.Fatalf("LoadFixture: %v", err) + } + suite, err := LoadSuiteFile("../suites/rollup.yaml") + if err != nil { + t.Fatalf("LoadSuiteFile: %v", err) + } + if suite.PlannerOutputDir == "" { + t.Fatal("rollup suite has no planner_output_dir") + } + contents, err := os.ReadFile(filepath.Join(suite.PlannerOutputDir, plannerInferenceConfigFile)) + if err != nil { + t.Fatalf("read inference config: %v", err) + } + var inference struct { + Queries []struct { + Query string `yaml:"query"` + } `yaml:"queries"` + } + if err := yaml.Unmarshal(contents, &inference); err != nil { + t.Fatalf("parse inference config: %v", err) + } + configured := make(map[string]struct{}, len(inference.Queries)) + for _, query := range inference.Queries { + configured[query.Query] = struct{}{} + } + parser := promqlparser.NewParser(promqlparser.Options{}) + for _, query := range suite.Queries { + if _, err := parser.ParseExpr(query.Expr); err != nil { + t.Fatalf("parse %q: %v", query.Expr, err) + } + if _, ok := configured[query.Expr]; !ok { + t.Errorf("query %q has no query config in the fixed planner output", query.Expr) + } + } + if got, want := len(inference.Queries), len(suite.Queries); got != want { + t.Errorf("fixed planner output has %d query configs, suite has %d queries", got, want) + } +} diff --git a/promql-compliance/suites/rollup-planner-output/inference_config.yaml b/promql-compliance/suites/rollup-planner-output/inference_config.yaml new file mode 100644 index 00000000..6a7f7006 --- /dev/null +++ b/promql-compliance/suites/rollup-planner-output/inference_config.yaml @@ -0,0 +1,117 @@ +# Hand-written in the planner's output format. Queries grouped by a subset of +# an aggregation's grouping labels point at that aggregation, so the engine +# answers them by rolling the per-(service, endpoint) aggregates up. +# no_cleanup: several queries read the same windows, so a per-query read +# count threshold would evict windows other queries still need. +cleanup_policy: + name: no_cleanup +metrics: + rollup_latency: + - endpoint + - service +queries: +- aggregations: + - aggregation_id: 1 + query: quantile by (service, endpoint) (0.5, rollup_latency) + planned_subquery: quantile by (service, endpoint) (0.5, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 1 + query: quantile by (service) (0.5, rollup_latency) + planned_subquery: quantile by (service) (0.5, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 1 + query: quantile(0.5, rollup_latency) + planned_subquery: quantile(0.5, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 1 + query: quantile by (service, endpoint) (0.9, rollup_latency) + planned_subquery: quantile by (service, endpoint) (0.9, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 1 + query: quantile by (service) (0.9, rollup_latency) + planned_subquery: quantile by (service) (0.9, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 1 + query: quantile(0.9, rollup_latency) + planned_subquery: quantile(0.9, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 2 + query: quantile by (service, endpoint) (0.75, rollup_latency) + planned_subquery: quantile by (service, endpoint) (0.75, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 2 + query: quantile by (service) (0.75, rollup_latency) + planned_subquery: quantile by (service) (0.75, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 2 + query: quantile(0.75, rollup_latency) + planned_subquery: quantile(0.75, rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 3 + query: sum by (service, endpoint) (rollup_latency) + planned_subquery: sum by (service, endpoint) (rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 3 + query: sum by (service) (rollup_latency) + planned_subquery: sum by (service) (rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 3 + query: sum(rollup_latency) + planned_subquery: sum(rollup_latency) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 4 + query: sum by (service, endpoint) (sum_over_time(rollup_latency[5m])) + planned_subquery: sum by (service, endpoint) (sum_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 4 + query: sum by (service) (sum_over_time(rollup_latency[5m])) + planned_subquery: sum by (service) (sum_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 4 + query: sum(sum_over_time(rollup_latency[5m])) + planned_subquery: sum(sum_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 5 + query: min by (service, endpoint) (min_over_time(rollup_latency[5m])) + planned_subquery: min by (service, endpoint) (min_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 5 + query: min by (service) (min_over_time(rollup_latency[5m])) + planned_subquery: min by (service) (min_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 5 + query: min(min_over_time(rollup_latency[5m])) + planned_subquery: min(min_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 6 + query: max by (service, endpoint) (max_over_time(rollup_latency[5m])) + planned_subquery: max by (service, endpoint) (max_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 6 + query: max by (service) (max_over_time(rollup_latency[5m])) + planned_subquery: max by (service) (max_over_time(rollup_latency[5m])) + query_time_aggregations: [] +- aggregations: + - aggregation_id: 6 + query: max(max_over_time(rollup_latency[5m])) + planned_subquery: max(max_over_time(rollup_latency[5m])) + query_time_aggregations: [] diff --git a/promql-compliance/suites/rollup-planner-output/streaming_config.yaml b/promql-compliance/suites/rollup-planner-output/streaming_config.yaml new file mode 100644 index 00000000..afb0dc73 --- /dev/null +++ b/promql-compliance/suites/rollup-planner-output/streaming_config.yaml @@ -0,0 +1,113 @@ +# Hand-written in the planner's output format. Every aggregation is a +# single-subpopulation type grouped by (service, endpoint); the tumbling +# window sizes match what the planner picks for each query shape, and ids 4 +# and 6 are sliding to cover sliding-window roll-ups. +aggregations: +- aggregationId: 1 + aggregationSubType: '' + aggregationType: DatasketchesKLL + labels: + aggregated: [] + grouping: + - endpoint + - service + rollup: [] + metric: rollup_latency + parameters: + K: 500 + slideIntervalMs: 1000 + spatialFilter: '' + table_name: null + value_column: null + windowSizeMs: 1000 + windowType: tumbling +- aggregationId: 2 + aggregationSubType: '' + aggregationType: DDSketch + labels: + aggregated: [] + grouping: + - endpoint + - service + rollup: [] + metric: rollup_latency + parameters: + alpha: 0.01 + slideIntervalMs: 1000 + spatialFilter: '' + table_name: null + value_column: null + windowSizeMs: 1000 + windowType: tumbling +- aggregationId: 3 + aggregationSubType: sum + aggregationType: Sum + labels: + aggregated: [] + grouping: + - endpoint + - service + rollup: [] + metric: rollup_latency + parameters: {} + slideIntervalMs: 1000 + spatialFilter: '' + table_name: null + value_column: null + windowSizeMs: 1000 + windowType: tumbling +- aggregationId: 4 + aggregationSubType: sum + aggregationType: Sum + labels: + aggregated: [] + grouping: + - endpoint + - service + rollup: [] + metric: rollup_latency + parameters: {} + slideIntervalMs: 60000 + spatialFilter: '' + table_name: null + value_column: null + windowSizeMs: 300000 + windowType: sliding +- aggregationId: 5 + aggregationSubType: min + aggregationType: MinMax + labels: + aggregated: [] + grouping: + - endpoint + - service + rollup: [] + metric: rollup_latency + parameters: {} + slideIntervalMs: 60000 + spatialFilter: '' + table_name: null + value_column: null + windowSizeMs: 60000 + windowType: tumbling +- aggregationId: 6 + aggregationSubType: max + aggregationType: MinMax + labels: + aggregated: [] + grouping: + - endpoint + - service + rollup: [] + metric: rollup_latency + parameters: {} + slideIntervalMs: 60000 + spatialFilter: '' + table_name: null + value_column: null + windowSizeMs: 300000 + windowType: sliding +metrics: + rollup_latency: + - endpoint + - service diff --git a/promql-compliance/suites/rollup.yaml b/promql-compliance/suites/rollup.yaml new file mode 100644 index 00000000..e595cca3 --- /dev/null +++ b/promql-compliance/suites/rollup.yaml @@ -0,0 +1,125 @@ +name: rollup +# Every aggregation in the fixed planner output is grouped by (service, +# endpoint). Each query appears grouped by (service, endpoint), which reads +# the aggregation as-is, then by (service) and with no grouping, which roll +# the per-endpoint aggregates up. The first query is the readiness probe, so +# it is a non-rolled-up one. +planner_output_dir: rollup-planner-output +comparison_defaults: + value_tolerance: + relative: 0.01 + absolute: 0.000001 + +queries: + # DatasketchesKLL, 1s tumbling. + - name: kll-quantile-0.5-by-service-endpoint + expr: "quantile by (service, endpoint) (0.5, rollup_latency)" + instant_offsets_seconds: &evaluation_offsets [300, 600, 900, 1200] + range: &evaluation_range + start_offset_seconds: 300 + end_offset_seconds: 1200 + step_seconds: 60 + - name: kll-quantile-0.5-by-service + expr: "quantile by (service) (0.5, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: kll-quantile-0.5-all + expr: "quantile(0.5, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: kll-quantile-0.9-by-service-endpoint + expr: "quantile by (service, endpoint) (0.9, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: kll-quantile-0.9-by-service + expr: "quantile by (service) (0.9, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + - name: kll-quantile-0.9-all + expr: "quantile(0.9, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + # DDSketch (alpha 0.01), 1s tumbling. Its relative error adds to the + # nearest-rank gap, hence the wider tolerance. + - name: ddsketch-quantile-0.75-by-service-endpoint + expr: "quantile by (service, endpoint) (0.75, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: &ddsketch_tolerance + value_tolerance: + relative: 0.02 + - name: ddsketch-quantile-0.75-by-service + expr: "quantile by (service) (0.75, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *ddsketch_tolerance + - name: ddsketch-quantile-0.75-all + expr: "quantile(0.75, rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *ddsketch_tolerance + # Sum and MinMax merge exactly, so they are compared tightly. Sum is 1s + # tumbling for the instant-vector sum and 5m/1m sliding for sum_over_time; + # min is 1m tumbling and max is 5m/1m sliding. + - name: sum-by-service-endpoint + expr: "sum by (service, endpoint) (rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: &exact + value_tolerance: + relative: 0 + - name: sum-by-service + expr: "sum by (service) (rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: sum-all + expr: "sum(rollup_latency)" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: sum-sum-over-time-by-service-endpoint + expr: "sum by (service, endpoint) (sum_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: sum-sum-over-time-by-service + expr: "sum by (service) (sum_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: sum-sum-over-time-all + expr: "sum(sum_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: min-min-over-time-by-service-endpoint + expr: "min by (service, endpoint) (min_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: min-min-over-time-by-service + expr: "min by (service) (min_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: min-min-over-time-all + expr: "min(min_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: max-max-over-time-by-service-endpoint + expr: "max by (service, endpoint) (max_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: max-max-over-time-by-service + expr: "max by (service) (max_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact + - name: max-max-over-time-all + expr: "max(max_over_time(rollup_latency[5m]))" + instant_offsets_seconds: *evaluation_offsets + range: *evaluation_range + comparison: *exact From 76830c3ff54152edd2974ec844ba81c8798f3a05 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Mon, 12 Oct 2026 22:51:08 -0400 Subject: [PATCH 03/12] feat(planner): MILP roll-ups for KLL, DDSketch and HLL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A MILP deployment grouped by G can serve a query grouped by Q ⊊ G. For single-subpopulation sketches (DatasketchesKLL, DDSketch, HLL) the engine merges across G-keys at query time, so the planner now writes the query_config pointing at the G-grouped aggregation instead of refusing. aggregation_config takes its labels from the deployment's grouping rather than the first served item, so a shared deployment is written at G whichever query comes first. Other families' roll-ups stay RollupUnsupported, and a query grouping outside the deployment's is a new GroupingOutsideDeployment error. Co-Authored-By: Claude Opus 5.5 --- asap-planner-rs/src/optimizer/milp_output.rs | 229 ++++++++++++++++--- asap-planner-rs/src/optimizer/saved_plan.rs | 5 +- 2 files changed, 198 insertions(+), 36 deletions(-) diff --git a/asap-planner-rs/src/optimizer/milp_output.rs b/asap-planner-rs/src/optimizer/milp_output.rs index 1b282d6b..2a32f0f4 100644 --- a/asap-planner-rs/src/optimizer/milp_output.rs +++ b/asap-planner-rs/src/optimizer/milp_output.rs @@ -8,7 +8,7 @@ use indexmap::IndexMap; use promql_utilities::data_model::KeyByLabelNames; use promql_utilities::query_logics::enums::AggregationType; use rqe_optimizer::milp::MilpSolution; -use rqe_optimizer::{Capability, Deployment}; +use rqe_optimizer::{Capability, Deployment, LabelSet, Raqe}; use serde_json::Value; use thiserror::Error; @@ -48,10 +48,20 @@ pub enum MilpOutputError { #[error("query {0:?} is served by two deployments; a query string can name only one")] QueryOnTwoDeployments(String), #[error( - "query {query:?} is grouped by {raqe:?} but served by a deployment grouped by \ - {deployment:?}: a roll-up, which the engine can't read yet" + "query {query:?} is grouped by {raqe:?} but served by a {aggregation_type} deployment \ + grouped by {deployment:?}: the engine rolls up only DatasketchesKLL, DDSketch and HLL" )] RollupUnsupported { + query: String, + aggregation_type: AggregationType, + raqe: Vec, + deployment: Vec, + }, + #[error( + "query {query:?} is grouped by {raqe:?}, which is not a subset of its \ + deployment's grouping {deployment:?}" + )] + GroupingOutsideDeployment { query: String, raqe: Vec, deployment: Vec, @@ -117,21 +127,6 @@ pub fn deployments_to_planner_output( "one deployment per Raqe" ); reject_unwritable_queries(config)?; - // sketch-bench lets a deployment serve a RAQE grouped by a subset of its - // grouping (a roll-up, sketch-bench#190). The engine reads a deployment - // only at its own grouping, and `aggregation_config` takes the labels - // from the first RAQE served, so refuse a roll-up rather than write - // configs at the wrong grouping. - for (raqe, &index) in workload.raqes.iter().zip(served_by) { - let deployment = &deployments[index]; - if raqe.grouping_labels != deployment.grouping_labels { - return Err(MilpOutputError::RollupUnsupported { - query: raqe.id.clone(), - raqe: raqe.grouping_labels.iter().cloned().collect(), - deployment: deployment.grouping_labels.iter().cloned().collect(), - }); - } - } // A SQL aggregation names its table and value column, and its metric is // the table. @@ -169,6 +164,11 @@ pub fn deployments_to_planner_output( for (raqe, &index) in served_by.iter().enumerate() { let item = workload.raqe_items[raqe]; let key = &item_keys[&(index, item)]; + check_rollup( + &workload.raqes[raqe], + &deployments[index], + aggregations[key].aggregation_type, + )?; for query in &workload.items[item].query_strings { match queries.entry(query.clone()) { Entry::Vacant(entry) => { @@ -212,9 +212,42 @@ pub fn deployments_to_planner_output( })) } +/// A Raqe grouped by a strict subset of its deployment's grouping is a +/// roll-up. The engine merges single-subpopulation sketches across the +/// deployment's groups at query time, so those need only a query_config +/// naming the aggregation. +fn check_rollup( + raqe: &Raqe, + deployment: &Deployment, + aggregation_type: AggregationType, +) -> Result<(), MilpOutputError> { + let (fine, coarse) = (&deployment.grouping_labels, &raqe.grouping_labels); + if coarse == fine { + return Ok(()); + } + let labels = |set: &LabelSet| set.iter().cloned().collect(); + if !coarse.is_subset(fine) { + return Err(MilpOutputError::GroupingOutsideDeployment { + query: raqe.id.clone(), + raqe: labels(coarse), + deployment: labels(fine), + }); + } + match aggregation_type { + AggregationType::DatasketchesKLL | AggregationType::DDSketch | AggregationType::HLL => { + Ok(()) + } + _ => Err(MilpOutputError::RollupUnsupported { + query: raqe.id.clone(), + aggregation_type, + raqe: labels(coarse), + deployment: labels(fine), + }), + } +} + /// The aggregation `deployment` runs for `item`, one of the optimizer items -/// it serves. Items share the deployment's capability and grouping -/// (`plan_to_planner_output` refuses roll-ups); `item` decides the labels. +/// it serves. fn aggregation_config( deployment: &Deployment, item: &OptimizerItem, @@ -283,7 +316,8 @@ fn aggregation_config( } }; - let (grouping_labels, aggregated_labels) = subpopulation_labels(item, aggregation_type); + let (grouping_labels, aggregated_labels) = + subpopulation_labels(deployment, item, aggregation_type); Ok(IntermediateAggConfig { aggregation_type, @@ -309,20 +343,33 @@ fn aggregation_config( }) } -/// The (grouping, aggregated) labels `item`'s query needs from an aggregation -/// of `aggregation_type`. +/// The (grouping, aggregated) labels `item`'s query needs from `deployment`. +/// A roll-up query is grouped by a subset of the deployment's grouping, so a +/// non-top-k aggregation is always built at the deployment's grouping. A +/// top-k deployment is grouped by its `topk by` buckets, and its heap is +/// keyed by the item's own labels (all labels, or a topk-of-sum's inner +/// `sum by` labels), which can differ between items on one deployment. fn subpopulation_labels( + deployment: &Deployment, item: &OptimizerItem, aggregation_type: AggregationType, ) -> (KeyByLabelNames, KeyByLabelNames) { let requirements = &item.requirements; + let deployment_labels = + KeyByLabelNames::new(deployment.grouping_labels.iter().cloned().collect()); + let (subpopulation_labels, topk_by_labels) = match deployment.capability { + Capability::TopKByValue | Capability::TopKByCount => { + (&requirements.grouping_labels, Some(&deployment_labels)) + } + _ => (&deployment_labels, None), + }; let mut grouping_labels = KeyByLabelNames::empty(); let mut aggregated_labels = KeyByLabelNames::empty(); set_subpopulation_labels( requirements.statistics[0], aggregation_type, - &requirements.grouping_labels, - requirements.topk_by_labels.as_ref(), + subpopulation_labels, + topk_by_labels, &mut KeyByLabelNames::empty(), &mut grouping_labels, &mut aggregated_labels, @@ -823,22 +870,78 @@ mod tests { ); } + fn instance_job() -> KeyByLabelNames { + KeyByLabelNames::new(vec!["instance".into(), "job".into()]) + } + + /// The solution with its only deployment grouped by (instance, job). + fn widen_to_instance_job(solution: &mut MilpSolution) { + let [planned] = solution.deployments.as_mut_slice() else { + panic!("expected one deployment, got {:?}", solution.deployments); + }; + planned.deployment.grouping_labels.insert("instance".into()); + } + + /// A sketch `by (job)` served from a deployment by (instance, job) is + /// written at the deployment's grouping; the engine merges across + /// `instance` at query time. #[test] - fn rollup_is_refused() { - // A deployment by (instance, job) serving a query by (job): the - // engine would build the aggregation at the query's grouping. + fn sketch_rollup_is_deployed_at_the_deployment_grouping() { + let query = "quantile by (job) (0.99, http_requests_total)"; + for (variant, capability, params, aggregation_type) in [ + ( + "kll-percall", + Capability::Quantile, + json!({"k": 200}), + AggregationType::DatasketchesKLL, + ), + ( + "dd", + Capability::Quantile, + json!({"alpha": 0.01}), + AggregationType::DDSketch, + ), + ( + "hll", + Capability::Cardinality, + json!({"lg_k": 12}), + AggregationType::HLL, + ), + ] { + let (config, workload, mut solution) = plan(&group(query, 0.99)); + assert_eq!( + workload.raqes[0].grouping_labels, + LabelSet::from(["job".into()]) + ); + widen_to_instance_job(&mut solution); + let deployment = &mut solution.deployments[0].deployment; + deployment.config.sketch = variant.into(); + deployment.config.sketch_config["params"] = params; + deployment.capability = capability; + + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + assert_eq!(output.streaming_aggregation_count(), 1, "{variant}"); + let a = aggregation_for(&output, query); + assert_eq!(a.aggregation_type, aggregation_type, "{variant}"); + assert_eq!(a.grouping_labels, instance_job(), "{variant}"); + assert!(a.aggregated_labels.is_empty(), "{variant}"); + } + } + + /// Exact accumulators are keyed types the engine doesn't roll up. + #[test] + fn exact_sum_rollup_is_refused() { let query = "sum by (job) (http_requests_total)"; let (config, workload, mut solution) = plan(&group(query, 0.99)); - solution.deployments[0] - .deployment - .grouping_labels - .insert("instance".into()); + widen_to_instance_job(&mut solution); let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) .err() .unwrap(); assert!( - matches!(&err, MilpOutputError::RollupUnsupported { query: q, raqe, deployment } - if q.contains(query) && raqe == &["job"] && deployment == &["instance", "job"]), + matches!(&err, MilpOutputError::RollupUnsupported { + query: q, aggregation_type: AggregationType::MultipleSum, raqe, deployment + } if q.contains(query) && raqe == &["job"] && deployment == &["instance", "job"]), "{err}" ); } @@ -871,6 +974,64 @@ mod tests { } } + #[test] + fn grouping_outside_the_deployment_is_refused() { + let query = "quantile by (job) (0.99, http_requests_total)"; + let (config, workload, mut solution) = plan(&group(query, 0.99)); + let grouping = &mut solution.deployments[0].deployment.grouping_labels; + *grouping = LabelSet::from(["instance".into()]); + let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) + .err() + .unwrap(); + assert!( + matches!(&err, MilpOutputError::GroupingOutsideDeployment { raqe, deployment, .. } + if raqe == &["job"] && deployment == &["instance"]), + "{err}" + ); + } + + /// A deployment serving an exact-grouping query and a roll-up is one + /// aggregation at the deployment's grouping, whichever query comes first. + #[test] + fn shared_deployment_takes_its_own_grouping_in_either_order() { + let fine = "quantile by (instance, job) (0.99, http_requests_total)"; + let coarse = "quantile by (job) (0.99, http_requests_total)"; + for reverse in [false, true] { + let (config, mut workload, mut solution) = + plan(&(group(fine, 0.99) + &group(coarse, 0.99))); + // Raqes are sorted by query string; reversing the index-aligned + // lists puts the other query first. + if reverse { + workload.raqes.reverse(); + workload.raqe_items.reverse(); + solution.raqes.reverse(); + } + let fine_raqe = workload + .raqes + .iter() + .position(|r| r.id.contains(fine)) + .unwrap(); + let shared = solution.deployments[solution.raqes[fine_raqe].deployment].clone(); + solution.deployments = vec![shared]; + for planned in &mut solution.raqes { + planned.deployment = 0; + } + + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + assert_eq!(output.streaming_aggregation_count(), 1); + for query in [fine, coarse] { + let a = aggregation_for(&output, query); + assert_eq!(a.aggregation_type, AggregationType::DatasketchesKLL); + assert_eq!( + a.grouping_labels, + instance_job(), + "{query}, reverse {reverse}" + ); + } + } + } + #[test] fn query_split_across_deployments_is_an_error() { // The same query string at two cadences is two items; if they land on diff --git a/asap-planner-rs/src/optimizer/saved_plan.rs b/asap-planner-rs/src/optimizer/saved_plan.rs index 4a75e352..2b50235d 100644 --- a/asap-planner-rs/src/optimizer/saved_plan.rs +++ b/asap-planner-rs/src/optimizer/saved_plan.rs @@ -402,12 +402,13 @@ mod tests { assert!(err.contains("0 plans match"), "{err}"); } + /// The exact sum is a keyed MultipleSum, which the engine doesn't roll up. #[test] - fn refuses_a_roll_up() { + fn refuses_an_exact_sum_roll_up() { let mut plan = plans(shared_p50()); plan["plans"][0]["chosen"][0]["deployment_grouping"] = json!(["job", "label_0"]); let err = refusal(plan, None); - assert!(err.contains("roll-up"), "{err}"); + assert!(err.contains("rolls up only"), "{err}"); } #[test] From 2dc11925c0f9b8e2d054f2cd81a0a78c0289c75b Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Mon, 12 Oct 2026 22:58:53 -0400 Subject: [PATCH 04/12] feat(planner): MILP roll-ups for exact sum, count, min and max The MILP emits exact-sum/min/max as keyed MultipleSum/MultipleMinMax, which the engine doesn't roll up itself. Regrouping their final values is exact, so a roll-up from grouping G to Q now writes a query_config whose planned_subquery is the query with its outer aggregation rewritten to `by (G)` (on the AST, which also covers `without`) and whose query_time_aggregations is one Sum/Min/Max stage `by (Q)`, or `all` when Q is empty. Count regroups with Sum. exact-increase and cms-heap roll-ups stay RollupUnsupported. Adds an end-to-end engine test that such a config over a MultipleSum keyed by (instance, job) answers `sum by (job)` exactly. Co-Authored-By: Claude Opus 5.5 --- asap-planner-rs/src/optimizer/milp.rs | 2 +- asap-planner-rs/src/optimizer/milp_output.rs | 253 +++++++++++++++--- asap-planner-rs/src/optimizer/saved_plan.rs | 10 +- .../tests/e2e_precompute_equivalence.rs | 105 ++++++++ 4 files changed, 335 insertions(+), 35 deletions(-) diff --git a/asap-planner-rs/src/optimizer/milp.rs b/asap-planner-rs/src/optimizer/milp.rs index 07165935..a8e12c3f 100644 --- a/asap-planner-rs/src/optimizer/milp.rs +++ b/asap-planner-rs/src/optimizer/milp.rs @@ -602,7 +602,7 @@ metrics: names.iter().map(|s| s.to_string()).collect() } - fn cost(sketch: &str, accuracy: &[(&str, f64)]) -> AtomicCostEntry { + pub(crate) fn cost(sketch: &str, accuracy: &[(&str, f64)]) -> AtomicCostEntry { AtomicCostEntry { sketch: sketch.into(), sketch_config: serde_json::json!({"algorithm": sketch, "params": {}}), diff --git a/asap-planner-rs/src/optimizer/milp_output.rs b/asap-planner-rs/src/optimizer/milp_output.rs index 2a32f0f4..a62e05c7 100644 --- a/asap-planner-rs/src/optimizer/milp_output.rs +++ b/asap-planner-rs/src/optimizer/milp_output.rs @@ -3,8 +3,13 @@ use std::collections::HashMap; use asap_types::enums::{CleanupPolicy, WindowType}; +use asap_types::query_config::{ + QueryTimeAggregation, QueryTimeAggregationOperator, QueryTimeGrouping, QueryTimeGroupingMode, +}; use indexmap::map::Entry; use indexmap::IndexMap; +use promql_parser::label::Labels; +use promql_parser::parser::{Expr, LabelModifier}; use promql_utilities::data_model::KeyByLabelNames; use promql_utilities::query_logics::enums::AggregationType; use rqe_optimizer::milp::MilpSolution; @@ -49,7 +54,8 @@ pub enum MilpOutputError { QueryOnTwoDeployments(String), #[error( "query {query:?} is grouped by {raqe:?} but served by a {aggregation_type} deployment \ - grouped by {deployment:?}: the engine rolls up only DatasketchesKLL, DDSketch and HLL" + grouped by {deployment:?}: roll-ups need DatasketchesKLL, DDSketch, HLL, or a \ + MultipleSum or MultipleMinMax answering sum, count, min or max" )] RollupUnsupported { query: String, @@ -66,6 +72,8 @@ pub enum MilpOutputError { raqe: Vec, deployment: Vec, }, + #[error("query {0:?} is a roll-up but not an aggregation whose grouping can be rewritten")] + UnregroupableQuery(String), #[error(transparent)] Generator(#[from] ControllerError), } @@ -164,18 +172,16 @@ pub fn deployments_to_planner_output( for (raqe, &index) in served_by.iter().enumerate() { let item = workload.raqe_items[raqe]; let key = &item_keys[&(index, item)]; - check_rollup( - &workload.raqes[raqe], - &deployments[index], - aggregations[key].aggregation_type, - )?; for query in &workload.items[item].query_strings { match queries.entry(query.clone()) { Entry::Vacant(entry) => { - entry.insert(QueryPlanEntry::fully_planned( - query.clone(), - vec![(key.clone(), None)], - )); + entry.insert(query_plan_entry( + query, + key, + &workload.raqes[raqe], + &deployments[index], + aggregations[key].aggregation_type, + )?); } Entry::Occupied(entry) if entry.get().aggregation_keys[0].0 == *key => {} Entry::Occupied(_) => { @@ -212,18 +218,27 @@ pub fn deployments_to_planner_output( })) } +/// How `query` (an occurrence of `raqe`) reads the aggregation `key`. +/// /// A Raqe grouped by a strict subset of its deployment's grouping is a /// roll-up. The engine merges single-subpopulation sketches across the -/// deployment's groups at query time, so those need only a query_config -/// naming the aggregation. -fn check_rollup( +/// deployment's groups itself, so those name the aggregation as is. Exact +/// keyed accumulators are read at the deployment's grouping and regrouped +/// at query time, which is exact for sum, count, min and max. +fn query_plan_entry( + query: &str, + key: &str, raqe: &Raqe, deployment: &Deployment, aggregation_type: AggregationType, -) -> Result<(), MilpOutputError> { +) -> Result { + let aggregation_keys = vec![(key.to_string(), None)]; let (fine, coarse) = (&deployment.grouping_labels, &raqe.grouping_labels); if coarse == fine { - return Ok(()); + return Ok(QueryPlanEntry::fully_planned( + query.to_string(), + aggregation_keys, + )); } let labels = |set: &LabelSet| set.iter().cloned().collect(); if !coarse.is_subset(fine) { @@ -233,17 +248,65 @@ fn check_rollup( deployment: labels(fine), }); } - match aggregation_type { - AggregationType::DatasketchesKLL | AggregationType::DDSketch | AggregationType::HLL => { - Ok(()) + let operator = match (aggregation_type, deployment.capability) { + ( + AggregationType::DatasketchesKLL | AggregationType::DDSketch | AggregationType::HLL, + _, + ) => { + return Ok(QueryPlanEntry::fully_planned( + query.to_string(), + aggregation_keys, + )) } - _ => Err(MilpOutputError::RollupUnsupported { - query: raqe.id.clone(), - aggregation_type, - raqe: labels(coarse), - deployment: labels(fine), - }), - } + // A coarse count is the sum of the fine counts. + (AggregationType::MultipleSum, Capability::Sum | Capability::Count) => { + QueryTimeAggregationOperator::Sum + } + (AggregationType::MultipleMinMax, Capability::Min) => QueryTimeAggregationOperator::Min, + (AggregationType::MultipleMinMax, Capability::Max) => QueryTimeAggregationOperator::Max, + _ => { + return Err(MilpOutputError::RollupUnsupported { + query: raqe.id.clone(), + aggregation_type, + raqe: labels(coarse), + deployment: labels(fine), + }) + } + }; + let grouping = if coarse.is_empty() { + QueryTimeGrouping { + mode: QueryTimeGroupingMode::All, + labels: Vec::new(), + } + } else { + QueryTimeGrouping { + mode: QueryTimeGroupingMode::By, + labels: labels(coarse), + } + }; + Ok(QueryPlanEntry { + aggregation_keys, + planned_subquery: regrouped(query, fine)?, + query_time_aggregations: vec![QueryTimeAggregation { + operator, + grouping, + parameter: None, + }], + }) +} + +/// `query` with its outer aggregation grouped `by (grouping)`. Replacing a +/// `without` clause is also right: the fine output is then `grouping`. +fn regrouped(query: &str, grouping: &LabelSet) -> Result { + let unregroupable = || MilpOutputError::UnregroupableQuery(query.to_string()); + let mut expr = promql_parser::parser::parse(query).map_err(|_| unregroupable())?; + let Expr::Aggregate(aggregate) = &mut expr else { + return Err(unregroupable()); + }; + aggregate.modifier = Some(LabelModifier::Include(Labels::new( + grouping.iter().map(String::as_str).collect(), + ))); + Ok(expr.to_string()) } /// The aggregation `deployment` runs for `item`, one of the optimizer items @@ -400,6 +463,7 @@ mod tests { use asap_types::aggregation_config::AggregationConfig; use asap_types::enums::QueryLanguage; + use asap_types::query_config::QueryConfig; use rqe_optimizer::milp::Objective; use rqe_optimizer::{AtomicCostEntry, WorkloadFacts}; use serde_json::json; @@ -407,7 +471,7 @@ mod tests { use super::*; use crate::config::input::{ControllerConfig, SQLControllerConfig}; use crate::optimizer::milp::tests::{ - config, costs, facts, group, table_accuracy, FACTS, SCRAPE_MS, + config, cost, costs, facts, group, table_accuracy, FACTS, SCRAPE_MS, }; use crate::optimizer::milp::{build_milp_workload, solve_milp}; use crate::optimizer::workload_facts::parse_workload_facts; @@ -421,6 +485,8 @@ mod tests { fn solve(config: MilpConfig, facts: &WorkloadFacts) -> (MilpWorkload, MilpSolution) { let workload = build_milp_workload(config, facts, SCRAPE_MS).unwrap(); let mut costs = costs(); + costs.push(cost("exact-min", &[("relative_error", 0.0)])); + costs.push(cost("exact-max", &[("relative_error", 0.0)])); for cost in &mut costs { cost.sketch_config["params"] = match cost.sketch.as_str() { "kll-percall" => json!({"k": 200}), @@ -879,7 +945,7 @@ mod tests { let [planned] = solution.deployments.as_mut_slice() else { panic!("expected one deployment, got {:?}", solution.deployments); }; - planned.deployment.grouping_labels.insert("instance".into()); + planned.deployment.grouping_labels = LabelSet::from(["instance".into(), "job".into()]); } /// A sketch `by (job)` served from a deployment by (instance, job) is @@ -929,18 +995,145 @@ mod tests { } } - /// Exact accumulators are keyed types the engine doesn't roll up. + /// The query_config each query gets, parsed back the way the engine reads it. + fn query_config_for(output: &PlannerOutput, query: &str) -> QueryConfig { + output + .to_inference_config(QueryLanguage::promql) + .unwrap() + .query_configs + .into_iter() + .find(|q| q.query == query) + .unwrap_or_else(|| panic!("no query_config for {query}")) + } + + /// An exact keyed accumulator is read at the deployment's grouping and + /// regrouped to the query's at query time. + #[test] + fn exact_rollup_regroups_at_query_time() { + for (query, planned_subquery, operator, aggregation_type, sub_type) in [ + ( + "sum by (job) (http_requests_total)", + "sum by (instance, job) (http_requests_total)", + QueryTimeAggregationOperator::Sum, + AggregationType::MultipleSum, + "sum", + ), + ( + "sum by (job) (count_over_time(http_requests_total[1m]))", + "sum by (instance, job) (count_over_time(http_requests_total[1m]))", + QueryTimeAggregationOperator::Sum, + AggregationType::MultipleSum, + "count", + ), + ( + "min by (job) (http_requests_total)", + "min by (instance, job) (http_requests_total)", + QueryTimeAggregationOperator::Min, + AggregationType::MultipleMinMax, + "min", + ), + ( + "max by (job) (http_requests_total)", + "max by (instance, job) (http_requests_total)", + QueryTimeAggregationOperator::Max, + AggregationType::MultipleMinMax, + "max", + ), + ( + "sum without (instance) (http_requests_total)", + "sum by (instance, job) (http_requests_total)", + QueryTimeAggregationOperator::Sum, + AggregationType::MultipleSum, + "sum", + ), + ] { + let (config, workload, mut solution) = plan(&group(query, 0.99)); + widen_to_instance_job(&mut solution); + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + + let a = aggregation_for(&output, query); + assert_eq!(a.aggregation_type, aggregation_type, "{query}"); + assert_eq!(a.aggregation_sub_type, sub_type, "{query}"); + assert_eq!(a.aggregated_labels, instance_job(), "{query}"); + assert!(a.grouping_labels.is_empty(), "{query}"); + + let query_config = query_config_for(&output, query); + assert_eq!(query_config.planned_subquery, planned_subquery); + let [stage] = query_config.query_time_aggregations.as_slice() else { + panic!("{query}: {:?}", query_config.query_time_aggregations); + }; + assert_eq!( + std::mem::discriminant(&stage.operator), + std::mem::discriminant(&operator), + "{query}: {stage:?}" + ); + assert!( + matches!(stage.grouping.mode, QueryTimeGroupingMode::By), + "{query}: {stage:?}" + ); + assert_eq!(stage.grouping.labels, ["job"], "{query}"); + assert!(stage.parameter.is_none(), "{query}"); + stage.validate().unwrap(); + } + } + + /// A query with no output labels regroups over everything. #[test] - fn exact_sum_rollup_is_refused() { + fn exact_rollup_to_no_labels_regroups_all() { + for query in [ + "sum(http_requests_total)", + "sum by () (http_requests_total)", + ] { + let (config, workload, mut solution) = plan(&group(query, 0.99)); + widen_to_instance_job(&mut solution); + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + let query_config = query_config_for(&output, query); + assert_eq!( + query_config.planned_subquery, + "sum by (instance, job) (http_requests_total)" + ); + let [stage] = query_config.query_time_aggregations.as_slice() else { + panic!("{query}: {:?}", query_config.query_time_aggregations); + }; + assert!( + matches!(stage.grouping.mode, QueryTimeGroupingMode::All), + "{query}: {stage:?}" + ); + assert!(stage.grouping.labels.is_empty(), "{query}"); + stage.validate().unwrap(); + } + } + + /// An exact-grouping query keeps its own string as the planned subquery. + #[test] + fn exact_query_at_its_own_grouping_has_no_query_time_stage() { + let query = "sum by (job) (http_requests_total)"; + let (config, workload, solution) = plan(&group(query, 0.99)); + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + let query_config = query_config_for(&output, query); + assert_eq!(query_config.planned_subquery, query); + assert!(query_config.query_time_aggregations.is_empty()); + } + + /// An increase deployment's capability doesn't say which outer + /// aggregation would regroup it, so its roll-ups stay refused. + #[test] + fn exact_increase_rollup_is_refused() { let query = "sum by (job) (http_requests_total)"; let (config, workload, mut solution) = plan(&group(query, 0.99)); widen_to_instance_job(&mut solution); + let deployment = &mut solution.deployments[0].deployment; + deployment.config.sketch = "exact-increase".into(); + deployment.capability = Capability::RateOrIncrease; let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) .err() .unwrap(); assert!( matches!(&err, MilpOutputError::RollupUnsupported { - query: q, aggregation_type: AggregationType::MultipleSum, raqe, deployment + query: q, aggregation_type: AggregationType::MultipleIncrease, raqe, deployment } if q.contains(query) && raqe == &["job"] && deployment == &["instance", "job"]), "{err}" ); diff --git a/asap-planner-rs/src/optimizer/saved_plan.rs b/asap-planner-rs/src/optimizer/saved_plan.rs index 2b50235d..d1f8c3eb 100644 --- a/asap-planner-rs/src/optimizer/saved_plan.rs +++ b/asap-planner-rs/src/optimizer/saved_plan.rs @@ -402,13 +402,15 @@ mod tests { assert!(err.contains("0 plans match"), "{err}"); } - /// The exact sum is a keyed MultipleSum, which the engine doesn't roll up. + /// The exact sum is a keyed MultipleSum, so the roll-up reads it at the + /// deployment's grouping and sums by `job` at query time. #[test] - fn refuses_an_exact_sum_roll_up() { + fn writes_an_exact_sum_roll_up_as_a_query_time_sum() { let mut plan = plans(shared_p50()); plan["plans"][0]["chosen"][0]["deployment_grouping"] = json!(["job", "label_0"]); - let err = refusal(plan, None); - assert!(err.contains("rolls up only"), "{err}"); + let yaml = run(plan, None).unwrap().to_inference_yaml_string().unwrap(); + assert!(yaml.contains("sum by (job, label_0) (data)"), "{yaml}"); + assert!(yaml.contains("operator: sum"), "{yaml}"); } #[test] diff --git a/asap-query-engine/tests/e2e_precompute_equivalence.rs b/asap-query-engine/tests/e2e_precompute_equivalence.rs index 2a7ec9a1..f4a7c942 100644 --- a/asap-query-engine/tests/e2e_precompute_equivalence.rs +++ b/asap-query-engine/tests/e2e_precompute_equivalence.rs @@ -983,6 +983,111 @@ async fn e2e_nested_aggregation_operator_matrix_executes_instant_and_range() { } } +/// The MILP planner's exact roll-up: `sum by (job)` served from a MultipleSum +/// keyed by (instance, job), read at that grouping and summed by `job` at +/// query time. +#[tokio::test] +async fn e2e_exact_rollup_regroups_a_keyed_sum_at_query_time() { + use asap_types::query_config::{ + QueryTimeAggregationOperator, QueryTimeGrouping, QueryTimeGroupingMode, + }; + + let metric = "exact_rollup_requests"; + let query = "sum by (job) (exact_rollup_requests)"; + let scenario = NativeDagScenario { + port: 19429, + metric, + query, + aggregation_configs: vec![make_agg_config_full( + 29, + metric, + AggregationType::MultipleSum, + "sum", + 1_000, + 0, + vec![], + vec!["instance", "job"], + )], + schema_labels: vec!["instance".to_string(), "job".to_string()], + samples: vec![ + make_timeseries(metric, vec![("job", "api"), ("instance", "a")], 1_000, 2.0), + make_timeseries(metric, vec![("job", "api"), ("instance", "b")], 1_000, 3.0), + make_timeseries( + metric, + vec![("job", "worker"), ("instance", "a")], + 1_000, + 9.0, + ), + make_timeseries(metric, vec![("job", "api"), ("instance", "a")], 3_000, 0.0), + make_timeseries( + metric, + vec![("job", "worker"), ("instance", "a")], + 3_000, + 0.0, + ), + ], + evaluation_time_seconds: 1.0, + base_interval_ms: 1_000, + }; + let (engine, query) = scenario + .build_engine_with_plan( + "sum by (instance, job) (exact_rollup_requests)", + vec![QueryTimeAggregation { + operator: QueryTimeAggregationOperator::Sum, + grouping: QueryTimeGrouping { + mode: QueryTimeGroupingMode::By, + labels: vec!["job".to_string()], + }, + parameter: None, + }], + ) + .await; + let expected = vec![ + (vec!["api".to_string()], 5.0), + (vec!["worker".to_string()], 9.0), + ]; + + let Some((_, instant)) = engine + .handle_query_promql(query.clone(), 1.0) + .expect("roll-up should execute locally") + else { + panic!("roll-up returned no local result"); + }; + let QueryResult::Vector(instant) = instant else { + panic!("instant query should return a vector"); + }; + let mut instant_values: Vec<(Vec, f64)> = instant + .values + .into_iter() + .map(|value| (value.labels.labels, value.value)) + .collect(); + instant_values.sort_by(|left, right| left.0.cmp(&right.0)); + assert_eq!(instant_values, expected, "instant"); + + let Some((_, range)) = engine + .handle_range_query_promql(query, 1.0, 2.0, 1.0) + .expect("roll-up range should execute locally") + else { + panic!("roll-up range returned no local result"); + }; + let QueryResult::Matrix(range) = range else { + panic!("range query should return a matrix"); + }; + let mut range_values: Vec<(Vec, f64)> = range + .values + .into_iter() + .map(|value| { + let sample = value + .samples + .last() + .expect("range result should have a final sample"); + (value.labels.labels, sample.value) + }) + .collect(); + range_values.sort_by(|left, right| left.0.cmp(&right.0)); + assert_eq!(range_values, expected, "range"); +} + #[tokio::test] async fn e2e_sliding_precompute_outputs_compose_a_wider_query() { let port = 19402u16; From 2aa41059e2bcc42b325df3adeb0016871fb784d3 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 09:32:26 -0400 Subject: [PATCH 05/12] test(query-engine): move the exact roll-up e2e test off a shared port It and the topk-of-sum e2e test both bound port 19429, so running them in parallel failed one. Co-Authored-By: Claude Opus 5.5 --- asap-query-engine/tests/e2e_precompute_equivalence.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asap-query-engine/tests/e2e_precompute_equivalence.rs b/asap-query-engine/tests/e2e_precompute_equivalence.rs index f4a7c942..ef83d7d9 100644 --- a/asap-query-engine/tests/e2e_precompute_equivalence.rs +++ b/asap-query-engine/tests/e2e_precompute_equivalence.rs @@ -995,7 +995,7 @@ async fn e2e_exact_rollup_regroups_a_keyed_sum_at_query_time() { let metric = "exact_rollup_requests"; let query = "sum by (job) (exact_rollup_requests)"; let scenario = NativeDagScenario { - port: 19429, + port: 19430, metric, query, aggregation_configs: vec![make_agg_config_full( From 0ac55d7a39d6ea196e14c5da408d1fa11a102499 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 09:53:15 -0400 Subject: [PATCH 06/12] refactor(planner): build exact roll-up stages from the query's aggregation Find the outer aggregation through parentheses, build its query-time stage with aggregation_stage (count read as a sum of counts), and pick regroupable deployments by capability alone. Co-Authored-By: Claude Opus 5.5 --- asap-planner-rs/src/optimizer/milp_output.rs | 211 ++++++++++++------- asap-planner-rs/src/planner/promql.rs | 5 +- 2 files changed, 132 insertions(+), 84 deletions(-) diff --git a/asap-planner-rs/src/optimizer/milp_output.rs b/asap-planner-rs/src/optimizer/milp_output.rs index a62e05c7..837a3aca 100644 --- a/asap-planner-rs/src/optimizer/milp_output.rs +++ b/asap-planner-rs/src/optimizer/milp_output.rs @@ -3,13 +3,11 @@ use std::collections::HashMap; use asap_types::enums::{CleanupPolicy, WindowType}; -use asap_types::query_config::{ - QueryTimeAggregation, QueryTimeAggregationOperator, QueryTimeGrouping, QueryTimeGroupingMode, -}; +use asap_types::query_config::QueryTimeAggregationOperator; use indexmap::map::Entry; use indexmap::IndexMap; use promql_parser::label::Labels; -use promql_parser::parser::{Expr, LabelModifier}; +use promql_parser::parser::{AggregateExpr, Expr, LabelModifier}; use promql_utilities::data_model::KeyByLabelNames; use promql_utilities::query_logics::enums::AggregationType; use rqe_optimizer::milp::MilpSolution; @@ -21,6 +19,7 @@ use crate::error::ControllerError; use crate::generator::{GeneratorOutput, QueryPlanEntry}; use crate::planner::agg_config::IntermediateAggConfig; use crate::planner::labels::set_subpopulation_labels; +use crate::planner::promql::{aggregation_stage, strip_parens}; use crate::planner_output::PlannerOutput; use crate::promql::generator::{build_inference_yaml, build_streaming_yaml}; use crate::sql::generator::{build_sql_inference_yaml, build_sql_streaming_yaml}; @@ -248,22 +247,17 @@ fn query_plan_entry( deployment: labels(fine), }); } - let operator = match (aggregation_type, deployment.capability) { - ( - AggregationType::DatasketchesKLL | AggregationType::DDSketch | AggregationType::HLL, - _, - ) => { - return Ok(QueryPlanEntry::fully_planned( - query.to_string(), - aggregation_keys, - )) - } - // A coarse count is the sum of the fine counts. - (AggregationType::MultipleSum, Capability::Sum | Capability::Count) => { - QueryTimeAggregationOperator::Sum - } - (AggregationType::MultipleMinMax, Capability::Min) => QueryTimeAggregationOperator::Min, - (AggregationType::MultipleMinMax, Capability::Max) => QueryTimeAggregationOperator::Max, + if matches!( + aggregation_type, + AggregationType::DatasketchesKLL | AggregationType::DDSketch | AggregationType::HLL + ) { + return Ok(QueryPlanEntry::fully_planned( + query.to_string(), + aggregation_keys, + )); + } + match deployment.capability { + Capability::Sum | Capability::Count | Capability::Min | Capability::Max => {} _ => { return Err(MilpOutputError::RollupUnsupported { query: raqe.id.clone(), @@ -272,41 +266,37 @@ fn query_plan_entry( deployment: labels(fine), }) } - }; - let grouping = if coarse.is_empty() { - QueryTimeGrouping { - mode: QueryTimeGroupingMode::All, - labels: Vec::new(), - } - } else { - QueryTimeGrouping { - mode: QueryTimeGroupingMode::By, - labels: labels(coarse), - } - }; + } + let unregroupable = || MilpOutputError::UnregroupableQuery(query.to_string()); + let aggregate = outer_aggregate(query).ok_or_else(unregroupable)?; + let mut stage = aggregation_stage(&aggregate).ok_or_else(unregroupable)?; + // A coarse count is the sum of the fine counts. + if matches!(stage.operator, QueryTimeAggregationOperator::Count) { + stage.operator = QueryTimeAggregationOperator::Sum; + } Ok(QueryPlanEntry { aggregation_keys, - planned_subquery: regrouped(query, fine)?, - query_time_aggregations: vec![QueryTimeAggregation { - operator, - grouping, - parameter: None, - }], + planned_subquery: regrouped(aggregate, fine), + query_time_aggregations: vec![stage], }) } -/// `query` with its outer aggregation grouped `by (grouping)`. Replacing a -/// `without` clause is also right: the fine output is then `grouping`. -fn regrouped(query: &str, grouping: &LabelSet) -> Result { - let unregroupable = || MilpOutputError::UnregroupableQuery(query.to_string()); - let mut expr = promql_parser::parser::parse(query).map_err(|_| unregroupable())?; - let Expr::Aggregate(aggregate) = &mut expr else { - return Err(unregroupable()); - }; +/// The aggregation `query` applies last, inside any parentheses. +fn outer_aggregate(query: &str) -> Option { + let expr = promql_parser::parser::parse(query).ok()?; + match strip_parens(&expr) { + Expr::Aggregate(aggregate) => Some(aggregate.clone()), + _ => None, + } +} + +/// `aggregate` grouped `by (grouping)`. Replacing a `without` clause is also +/// right: the fine output is then `grouping`. +fn regrouped(mut aggregate: AggregateExpr, grouping: &LabelSet) -> String { aggregate.modifier = Some(LabelModifier::Include(Labels::new( grouping.iter().map(String::as_str).collect(), ))); - Ok(expr.to_string()) + Expr::Aggregate(aggregate).to_string() } /// The aggregation `deployment` runs for `item`, one of the optimizer items @@ -1010,39 +1000,53 @@ mod tests { /// regrouped to the query's at query time. #[test] fn exact_rollup_regroups_at_query_time() { - for (query, planned_subquery, operator, aggregation_type, sub_type) in [ + let by_job = json!({"mode": "by", "labels": ["job"]}); + for (query, planned_subquery, operator, grouping, aggregation_type, sub_type) in [ ( "sum by (job) (http_requests_total)", "sum by (instance, job) (http_requests_total)", - QueryTimeAggregationOperator::Sum, + "sum", + &by_job, AggregationType::MultipleSum, "sum", ), ( "sum by (job) (count_over_time(http_requests_total[1m]))", "sum by (instance, job) (count_over_time(http_requests_total[1m]))", - QueryTimeAggregationOperator::Sum, + "sum", + &by_job, + AggregationType::MultipleSum, + "count", + ), + ( + "count by (job) (http_requests_total)", + "count by (instance, job) (http_requests_total)", + "sum", + &by_job, AggregationType::MultipleSum, "count", ), ( "min by (job) (http_requests_total)", "min by (instance, job) (http_requests_total)", - QueryTimeAggregationOperator::Min, + "min", + &by_job, AggregationType::MultipleMinMax, "min", ), ( "max by (job) (http_requests_total)", "max by (instance, job) (http_requests_total)", - QueryTimeAggregationOperator::Max, + "max", + &by_job, AggregationType::MultipleMinMax, "max", ), ( "sum without (instance) (http_requests_total)", "sum by (instance, job) (http_requests_total)", - QueryTimeAggregationOperator::Sum, + "sum", + &json!({"mode": "without", "labels": ["instance"]}), AggregationType::MultipleSum, "sum", ), @@ -1064,20 +1068,43 @@ mod tests { panic!("{query}: {:?}", query_config.query_time_aggregations); }; assert_eq!( - std::mem::discriminant(&stage.operator), - std::mem::discriminant(&operator), - "{query}: {stage:?}" - ); - assert!( - matches!(stage.grouping.mode, QueryTimeGroupingMode::By), - "{query}: {stage:?}" + serde_json::to_value(stage).unwrap(), + json!({"operator": operator, "grouping": grouping, "parameter": null}), + "{query}" ); - assert_eq!(stage.grouping.labels, ["job"], "{query}"); - assert!(stage.parameter.is_none(), "{query}"); stage.validate().unwrap(); } } + /// Parentheses around the outer aggregation don't hide it. The extractor + /// rejects such a query, so the item's query string is swapped in. + #[test] + fn exact_rollup_regroups_a_parenthesized_aggregation() { + let query = "(sum by (job) (http_requests_total))"; + let (config, mut workload, mut solution) = + plan(&group("sum by (job) (http_requests_total)", 0.99)); + widen_to_instance_job(&mut solution); + workload.items[0].query_strings = vec![query.to_string()]; + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + let query_config = query_config_for(&output, query); + assert_eq!( + query_config.planned_subquery, + "sum by (instance, job) (http_requests_total)" + ); + let [stage] = query_config.query_time_aggregations.as_slice() else { + panic!("{:?}", query_config.query_time_aggregations); + }; + assert_eq!( + serde_json::to_value(stage).unwrap(), + json!({ + "operator": "sum", + "grouping": {"mode": "by", "labels": ["job"]}, + "parameter": null, + }) + ); + } + /// A query with no output labels regroups over everything. #[test] fn exact_rollup_to_no_labels_regroups_all() { @@ -1097,11 +1124,15 @@ mod tests { let [stage] = query_config.query_time_aggregations.as_slice() else { panic!("{query}: {:?}", query_config.query_time_aggregations); }; - assert!( - matches!(stage.grouping.mode, QueryTimeGroupingMode::All), - "{query}: {stage:?}" + assert_eq!( + serde_json::to_value(stage).unwrap(), + json!({ + "operator": "sum", + "grouping": {"mode": "all", "labels": []}, + "parameter": null, + }), + "{query}" ); - assert!(stage.grouping.labels.is_empty(), "{query}"); stage.validate().unwrap(); } } @@ -1119,24 +1150,40 @@ mod tests { } /// An increase deployment's capability doesn't say which outer - /// aggregation would regroup it, so its roll-ups stay refused. + /// aggregation would regroup it, and a top-k heap can't be regrouped, so + /// their roll-ups stay refused. #[test] - fn exact_increase_rollup_is_refused() { + fn increase_and_topk_rollups_are_refused() { let query = "sum by (job) (http_requests_total)"; - let (config, workload, mut solution) = plan(&group(query, 0.99)); - widen_to_instance_job(&mut solution); - let deployment = &mut solution.deployments[0].deployment; - deployment.config.sketch = "exact-increase".into(); - deployment.capability = Capability::RateOrIncrease; - let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) - .err() - .unwrap(); - assert!( - matches!(&err, MilpOutputError::RollupUnsupported { - query: q, aggregation_type: AggregationType::MultipleIncrease, raqe, deployment - } if q.contains(query) && raqe == &["job"] && deployment == &["instance", "job"]), - "{err}" - ); + let (config, workload, solution) = plan(&group(query, 0.99)); + let mut increase = solution.deployments[0].deployment.clone(); + increase.config.sketch = "exact-increase".into(); + increase.capability = Capability::RateOrIncrease; + let (_, _, topk) = plan(&group( + "topk(5, sum_over_time(http_requests_total[1m]))", + 0.99, + )); + let heap = topk.deployments[0].deployment.clone(); + for (source, expected) in [ + (increase, AggregationType::MultipleIncrease), + (heap, AggregationType::CountMinSketchWithHeap), + ] { + let mut solution = solution.clone(); + widen_to_instance_job(&mut solution); + let deployment = &mut solution.deployments[0].deployment; + deployment.config = source.config; + deployment.capability = source.capability; + let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) + .err() + .unwrap(); + assert!( + matches!(&err, MilpOutputError::RollupUnsupported { + query: q, aggregation_type, raqe, deployment + } if q.contains(query) && *aggregation_type == expected + && raqe == &["job"] && deployment == &["instance", "job"]), + "{err}" + ); + } } /// Both are global top-k Raqes on one metric, so the solver may give them diff --git a/asap-planner-rs/src/planner/promql.rs b/asap-planner-rs/src/planner/promql.rs index 744cbbb9..38ebb451 100644 --- a/asap-planner-rs/src/planner/promql.rs +++ b/asap-planner-rs/src/planner/promql.rs @@ -38,7 +38,8 @@ pub struct NestedAggregationPlan { pub query_time_aggregations: Vec, } -fn aggregation_stage( +/// The query-time stage that evaluates `aggregate` over its input series. +pub(crate) fn aggregation_stage( aggregate: &promql_parser::parser::AggregateExpr, ) -> Option { let operator = match aggregate.op.to_string().as_str() { @@ -114,7 +115,7 @@ fn expr_to_binary_arm(expr: &promql_parser::parser::Expr) -> BinaryArm { } /// Recursively remove outer `Paren` wrappers from an expression. -fn strip_parens(expr: &promql_parser::parser::Expr) -> &promql_parser::parser::Expr { +pub(crate) fn strip_parens(expr: &promql_parser::parser::Expr) -> &promql_parser::parser::Expr { if let promql_parser::parser::Expr::Paren(paren) = expr { strip_parens(&paren.expr) } else { From 51406006ca3bfb9719ae490687de6e5375fb29c5 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 09:56:33 -0400 Subject: [PATCH 07/12] fix(planner): refuse exact roll-ups for binary-query leaves and SQL An exact roll-up needs a query-time stage. The engine runs a binary query's arms without one, and the SQL path doesn't run them at all, so those queries now get RollupUnsupported with the reason, instead of a plan the engine can't answer or a misleading UnregroupableQuery. Sketch roll-ups need no stage and stay allowed for binary-query leaves. Co-Authored-By: Claude Opus 5.5 --- asap-planner-rs/src/optimizer/milp_output.rs | 174 ++++++++++++++++--- 1 file changed, 147 insertions(+), 27 deletions(-) diff --git a/asap-planner-rs/src/optimizer/milp_output.rs b/asap-planner-rs/src/optimizer/milp_output.rs index 837a3aca..c1b26b5e 100644 --- a/asap-planner-rs/src/optimizer/milp_output.rs +++ b/asap-planner-rs/src/optimizer/milp_output.rs @@ -1,6 +1,6 @@ //! Turns a solved MILP plan into the planner's streaming and inference YAML. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use asap_types::enums::{CleanupPolicy, WindowType}; use asap_types::query_config::QueryTimeAggregationOperator; @@ -19,12 +19,12 @@ use crate::error::ControllerError; use crate::generator::{GeneratorOutput, QueryPlanEntry}; use crate::planner::agg_config::IntermediateAggConfig; use crate::planner::labels::set_subpopulation_labels; -use crate::planner::promql::{aggregation_stage, strip_parens}; +use crate::planner::promql::{aggregation_stage, parse_binary_arms, strip_parens}; use crate::planner_output::PlannerOutput; use crate::promql::generator::{build_inference_yaml, build_streaming_yaml}; use crate::sql::generator::{build_sql_inference_yaml, build_sql_streaming_yaml}; -use super::aqe_extractor::contains_avg; +use super::aqe_extractor::{contains_avg, decompose_to_leaves}; use super::milp::{MilpConfig, MilpWorkload}; use super::solution::OptimizerItem; @@ -53,14 +53,14 @@ pub enum MilpOutputError { QueryOnTwoDeployments(String), #[error( "query {query:?} is grouped by {raqe:?} but served by a {aggregation_type} deployment \ - grouped by {deployment:?}: roll-ups need DatasketchesKLL, DDSketch, HLL, or a \ - MultipleSum or MultipleMinMax answering sum, count, min or max" + grouped by {deployment:?}: {reason}" )] RollupUnsupported { query: String, aggregation_type: AggregationType, raqe: Vec, deployment: Vec, + reason: RollupBlocker, }, #[error( "query {query:?} is grouped by {raqe:?}, which is not a subset of its \ @@ -77,6 +77,20 @@ pub enum MilpOutputError { Generator(#[from] ControllerError), } +/// Why a roll-up can't be written. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] +pub enum RollupBlocker { + #[error( + "roll-ups need DatasketchesKLL, DDSketch, HLL, or a MultipleSum or MultipleMinMax \ + answering sum, count, min or max" + )] + Aggregation, + #[error("an exact roll-up needs a query-time stage, which a binary query's arms can't run")] + BinaryArm, + #[error("an exact roll-up needs a query-time stage, which SQL queries can't run")] + Sql, +} + /// Errors on queries whose plan can be costed but not written as configs: /// PromQL avg queries, and range-query overrides, which only size retention. pub fn reject_unwritable_queries(config: MilpConfig) -> Result<(), MilpOutputError> { @@ -167,11 +181,19 @@ pub fn deployments_to_planner_output( item_keys.insert((index, item), key); } + let binary_leaves = binary_query_leaves(config); let mut queries: IndexMap = IndexMap::new(); for (raqe, &index) in served_by.iter().enumerate() { let item = workload.raqe_items[raqe]; let key = &item_keys[&(index, item)]; for query in &workload.items[item].query_strings { + let stage_blocker = match config { + MilpConfig::Sql(_) => Some(RollupBlocker::Sql), + MilpConfig::Promql(_) if binary_leaves.contains(query) => { + Some(RollupBlocker::BinaryArm) + } + MilpConfig::Promql(_) => None, + }; match queries.entry(query.clone()) { Entry::Vacant(entry) => { entry.insert(query_plan_entry( @@ -180,6 +202,7 @@ pub fn deployments_to_planner_output( &workload.raqes[raqe], &deployments[index], aggregations[key].aggregation_type, + stage_blocker, )?); } Entry::Occupied(entry) if entry.get().aggregation_keys[0].0 == *key => {} @@ -223,13 +246,15 @@ pub fn deployments_to_planner_output( /// roll-up. The engine merges single-subpopulation sketches across the /// deployment's groups itself, so those name the aggregation as is. Exact /// keyed accumulators are read at the deployment's grouping and regrouped -/// at query time, which is exact for sum, count, min and max. +/// at query time, which is exact for sum, count, min and max, unless +/// `stage_blocker` says `query` can't run a query-time stage. fn query_plan_entry( query: &str, key: &str, raqe: &Raqe, deployment: &Deployment, aggregation_type: AggregationType, + stage_blocker: Option, ) -> Result { let aggregation_keys = vec![(key.to_string(), None)]; let (fine, coarse) = (&deployment.grouping_labels, &raqe.grouping_labels); @@ -256,16 +281,19 @@ fn query_plan_entry( aggregation_keys, )); } + let unsupported = |reason| MilpOutputError::RollupUnsupported { + query: raqe.id.clone(), + aggregation_type, + raqe: labels(coarse), + deployment: labels(fine), + reason, + }; match deployment.capability { Capability::Sum | Capability::Count | Capability::Min | Capability::Max => {} - _ => { - return Err(MilpOutputError::RollupUnsupported { - query: raqe.id.clone(), - aggregation_type, - raqe: labels(coarse), - deployment: labels(fine), - }) - } + _ => return Err(unsupported(RollupBlocker::Aggregation)), + } + if let Some(reason) = stage_blocker { + return Err(unsupported(reason)); } let unregroupable = || MilpOutputError::UnregroupableQuery(query.to_string()); let aggregate = outer_aggregate(query).ok_or_else(unregroupable)?; @@ -281,6 +309,21 @@ fn query_plan_entry( }) } +/// The leaves of the workload's binary queries. The engine runs a binary +/// query's arms without query-time stages. +fn binary_query_leaves(config: MilpConfig) -> HashSet { + let MilpConfig::Promql(config) = config else { + return HashSet::new(); + }; + config + .query_groups + .iter() + .flat_map(|group| &group.queries) + .filter(|query| parse_binary_arms(query).is_some()) + .flat_map(|query| decompose_to_leaves(query)) + .collect() +} + /// The aggregation `query` applies last, inside any parentheses. fn outer_aggregate(query: &str) -> Option { let expr = promql_parser::parser::parse(query).ok()?; @@ -621,6 +664,48 @@ mod tests { ) } + /// Plans SQL `groups` on table `http`, whose value column `requests` has + /// `http_requests_total`'s labels and facts. + fn sql_plan(groups: &str) -> (SQLControllerConfig, MilpWorkload, MilpSolution) { + let sql: SQLControllerConfig = serde_yaml::from_str(&format!( + "tables:\n - name: http\n time_column: time\n value_columns: [requests]\n \ + metadata_columns: [instance, job]\nquery_groups:\n{groups}" + )) + .unwrap(); + let facts = parse_workload_facts( + &FACTS.replace("http_requests_total", "http.requests"), + &MilpConfig::Sql(&sql).hints().unwrap(), + SCRAPE_MS, + ) + .unwrap(); + let (workload, solution) = solve(MilpConfig::Sql(&sql), &facts); + (sql, workload, solution) + } + + /// The SQL engine runs no query-time stages, so a SQL query can't be an + /// exact roll-up. + #[test] + fn sql_exact_rollup_is_refused() { + let query = "SELECT job, SUM(requests) FROM http \ + WHERE time BETWEEN DATEADD(s, -15, NOW()) AND NOW() GROUP BY job"; + let (sql, workload, mut solution) = sql_plan(&group(query, 0.99)); + widen_to_instance_job(&mut solution); + let err = plan_to_planner_output(MilpConfig::Sql(&sql), &workload, &solution) + .err() + .unwrap(); + assert!( + matches!( + &err, + MilpOutputError::RollupUnsupported { + aggregation_type: AggregationType::MultipleSum, + reason: RollupBlocker::Sql, + .. + } + ), + "{err}" + ); + } + /// A workload written in SQL, on a table whose value column has /// `http_requests_total`'s labels and facts, plans exactly as in PromQL. #[test] @@ -670,18 +755,7 @@ mod tests { let (promql, promql_workload, promql_solution) = plan(&promql_groups); let sql_groups: String = twins.iter().map(|(_, q)| group(q, 0.99)).collect(); - let sql: SQLControllerConfig = serde_yaml::from_str(&format!( - "tables:\n - name: http\n time_column: time\n value_columns: [requests]\n \ - metadata_columns: [instance, job]\nquery_groups:\n{sql_groups}" - )) - .unwrap(); - let facts = parse_workload_facts( - &FACTS.replace("http_requests_total", "http.requests"), - &MilpConfig::Sql(&sql).hints().unwrap(), - SCRAPE_MS, - ) - .unwrap(); - let (sql_workload, sql_solution) = solve(MilpConfig::Sql(&sql), &facts); + let (sql, sql_workload, sql_solution) = sql_plan(&sql_groups); let (promql_cost, sql_cost) = (&promql_solution.plan_cost, &sql_solution.plan_cost); assert_eq!(sql_cost.cpu_secs_per_sec(), promql_cost.cpu_secs_per_sec()); @@ -1178,7 +1252,8 @@ mod tests { .unwrap(); assert!( matches!(&err, MilpOutputError::RollupUnsupported { - query: q, aggregation_type, raqe, deployment + query: q, aggregation_type, raqe, deployment, + reason: RollupBlocker::Aggregation, } if q.contains(query) && *aggregation_type == expected && raqe == &["job"] && deployment == &["instance", "job"]), "{err}" @@ -1186,6 +1261,51 @@ mod tests { } } + /// The engine runs a binary query's arms without query-time stages, so a + /// binary query's leaf can't be an exact roll-up. + #[test] + fn exact_rollup_of_a_binary_query_leaf_is_refused() { + for query in [ + "sum by (job) (http_requests_total) * 100", + "sum by (job) (http_requests_total) / sum by (job) (http_requests_total)", + ] { + let (config, workload, mut solution) = plan(&group(query, 0.99)); + widen_to_instance_job(&mut solution); + let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) + .err() + .unwrap(); + assert!( + matches!( + &err, + MilpOutputError::RollupUnsupported { + aggregation_type: AggregationType::MultipleSum, + reason: RollupBlocker::BinaryArm, + .. + } + ), + "{query}: {err}" + ); + } + } + + /// A sketch roll-up needs no query-time stage, so a binary query's leaf + /// can still be one. + #[test] + fn sketch_rollup_of_a_binary_query_leaf_is_planned() { + let leaf = "quantile by (job) (0.99, http_requests_total)"; + let (config, workload, mut solution) = plan(&group(&format!("{leaf} * 100"), 0.99)); + widen_to_instance_job(&mut solution); + let deployment = &mut solution.deployments[0].deployment; + deployment.config.sketch = "kll-percall".into(); + deployment.config.sketch_config["params"] = json!({"k": 200}); + deployment.capability = Capability::Quantile; + let output = + plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution).unwrap(); + let query_config = query_config_for(&output, leaf); + assert_eq!(query_config.planned_subquery, leaf); + assert!(query_config.query_time_aggregations.is_empty()); + } + /// Both are global top-k Raqes on one metric, so the solver may give them /// one deployment, but their heaps are keyed by different labels: each /// query gets its own heap. From 3859ddb43b0b231bc25241076940f7472fc77d67 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 09:57:50 -0400 Subject: [PATCH 08/12] fix(planner): share the roll-up type list and check groupings up front - AggregationType::merges_across_groups() is now the one list of types a roll-up can merge; the engine and the MILP output both use it, and the refusal message is built from it. - A query grouping outside its deployment's is refused before any aggregation is built, so a param error can't hide it. - A top-k heap takes its buckets from the query's own topk by labels. - Tests: HLL roll-up from a SQL COUNT(DISTINCT) query, a saved-plan DD roll-up that succeeds, and widen_to_instance_job sets the grouping. Co-Authored-By: Claude Opus 5.5 --- .../src/query_logics/enums.rs | 16 +++ asap-planner-rs/src/optimizer/milp_output.rs | 124 ++++++++++++------ asap-planner-rs/src/optimizer/saved_plan.rs | 43 +++++- .../src/engines/simple_engine/mod.rs | 13 +- 4 files changed, 145 insertions(+), 51 deletions(-) diff --git a/asap-common/dependencies/rs/promql_utilities/src/query_logics/enums.rs b/asap-common/dependencies/rs/promql_utilities/src/query_logics/enums.rs index a1fa5144..f255dcef 100644 --- a/asap-common/dependencies/rs/promql_utilities/src/query_logics/enums.rs +++ b/asap-common/dependencies/rs/promql_utilities/src/query_logics/enums.rs @@ -312,6 +312,22 @@ impl AggregationType { } } + /// Single-subpopulation types whose merge of two groups' states is the + /// state of their union, so a query can read one at a coarser grouping + /// (a roll-up). Increase is excluded: its merge joins pieces of one + /// counter in time. + pub const MERGEABLE_ACROSS_GROUPS: [AggregationType; 5] = [ + AggregationType::Sum, + AggregationType::MinMax, + AggregationType::DatasketchesKLL, + AggregationType::DDSketch, + AggregationType::HLL, + ]; + + pub fn merges_across_groups(self) -> bool { + Self::MERGEABLE_ACROSS_GROUPS.contains(&self) + } + /// Returns `true` if this type produces keyed (multi-population) accumulators. pub fn is_keyed(self) -> bool { matches!( diff --git a/asap-planner-rs/src/optimizer/milp_output.rs b/asap-planner-rs/src/optimizer/milp_output.rs index 2a32f0f4..74428090 100644 --- a/asap-planner-rs/src/optimizer/milp_output.rs +++ b/asap-planner-rs/src/optimizer/milp_output.rs @@ -49,7 +49,9 @@ pub enum MilpOutputError { QueryOnTwoDeployments(String), #[error( "query {query:?} is grouped by {raqe:?} but served by a {aggregation_type} deployment \ - grouped by {deployment:?}: the engine rolls up only DatasketchesKLL, DDSketch and HLL" + grouped by {deployment:?}: only aggregations that merge across groups roll up \ + ({mergeable:?})", + mergeable = AggregationType::MERGEABLE_ACROSS_GROUPS )] RollupUnsupported { query: String, @@ -127,6 +129,9 @@ pub fn deployments_to_planner_output( "one deployment per Raqe" ); reject_unwritable_queries(config)?; + for (raqe, &index) in workload.raqes.iter().zip(served_by) { + check_grouping(raqe, &deployments[index])?; + } // A SQL aggregation names its table and value column, and its metric is // the table. @@ -212,38 +217,42 @@ pub fn deployments_to_planner_output( })) } +fn label_names(set: &LabelSet) -> Vec { + set.iter().cloned().collect() +} + +/// A deployment can serve a Raqe grouped by its own grouping or a subset of +/// it (a roll-up), never one grouped by labels it doesn't keep. +fn check_grouping(raqe: &Raqe, deployment: &Deployment) -> Result<(), MilpOutputError> { + if raqe.grouping_labels.is_subset(&deployment.grouping_labels) { + return Ok(()); + } + Err(MilpOutputError::GroupingOutsideDeployment { + query: raqe.id.clone(), + raqe: label_names(&raqe.grouping_labels), + deployment: label_names(&deployment.grouping_labels), + }) +} + /// A Raqe grouped by a strict subset of its deployment's grouping is a -/// roll-up. The engine merges single-subpopulation sketches across the -/// deployment's groups at query time, so those need only a query_config -/// naming the aggregation. +/// roll-up. The engine merges such aggregations across the deployment's +/// groups at query time, so those need only a query_config naming the +/// aggregation. fn check_rollup( raqe: &Raqe, deployment: &Deployment, aggregation_type: AggregationType, ) -> Result<(), MilpOutputError> { - let (fine, coarse) = (&deployment.grouping_labels, &raqe.grouping_labels); - if coarse == fine { + if raqe.grouping_labels == deployment.grouping_labels || aggregation_type.merges_across_groups() + { return Ok(()); } - let labels = |set: &LabelSet| set.iter().cloned().collect(); - if !coarse.is_subset(fine) { - return Err(MilpOutputError::GroupingOutsideDeployment { - query: raqe.id.clone(), - raqe: labels(coarse), - deployment: labels(fine), - }); - } - match aggregation_type { - AggregationType::DatasketchesKLL | AggregationType::DDSketch | AggregationType::HLL => { - Ok(()) - } - _ => Err(MilpOutputError::RollupUnsupported { - query: raqe.id.clone(), - aggregation_type, - raqe: labels(coarse), - deployment: labels(fine), - }), - } + Err(MilpOutputError::RollupUnsupported { + query: raqe.id.clone(), + aggregation_type, + raqe: label_names(&raqe.grouping_labels), + deployment: label_names(&deployment.grouping_labels), + }) } /// The aggregation `deployment` runs for `item`, one of the optimizer items @@ -346,9 +355,9 @@ fn aggregation_config( /// The (grouping, aggregated) labels `item`'s query needs from `deployment`. /// A roll-up query is grouped by a subset of the deployment's grouping, so a /// non-top-k aggregation is always built at the deployment's grouping. A -/// top-k deployment is grouped by its `topk by` buckets, and its heap is -/// keyed by the item's own labels (all labels, or a topk-of-sum's inner -/// `sum by` labels), which can differ between items on one deployment. +/// top-k heap is bucketed and keyed by the item's own labels (its `topk by` +/// buckets; all labels, or a topk-of-sum's inner `sum by` labels), which can +/// differ between items on one deployment. Top-k never rolls up. fn subpopulation_labels( deployment: &Deployment, item: &OptimizerItem, @@ -358,9 +367,10 @@ fn subpopulation_labels( let deployment_labels = KeyByLabelNames::new(deployment.grouping_labels.iter().cloned().collect()); let (subpopulation_labels, topk_by_labels) = match deployment.capability { - Capability::TopKByValue | Capability::TopKByCount => { - (&requirements.grouping_labels, Some(&deployment_labels)) - } + Capability::TopKByValue | Capability::TopKByCount => ( + &requirements.grouping_labels, + requirements.topk_by_labels.as_ref(), + ), _ => (&deployment_labels, None), }; let mut grouping_labels = KeyByLabelNames::empty(); @@ -650,6 +660,50 @@ mod tests { } } + /// Cardinality only comes from SQL `COUNT(DISTINCT ...)`, so the HLL + /// roll-up is planned from a real SQL cardinality query. + #[test] + fn hll_count_distinct_rollup_is_deployed_at_the_deployment_grouping() { + let query = "SELECT job, COUNT(DISTINCT requests) FROM http WHERE \ + time BETWEEN DATEADD(s, -60, NOW()) AND NOW() GROUP BY job"; + let sql: SQLControllerConfig = serde_yaml::from_str(&format!( + "tables:\n - name: http\n time_column: time\n value_columns: [requests]\n \ + metadata_columns: [instance, job]\nquery_groups:\n{}", + group(query, 0.99) + )) + .unwrap(); + let facts = parse_workload_facts( + &FACTS.replace("http_requests_total", "http.requests"), + &MilpConfig::Sql(&sql).hints().unwrap(), + SCRAPE_MS, + ) + .unwrap(); + let workload = build_milp_workload(MilpConfig::Sql(&sql), &facts, SCRAPE_MS).unwrap(); + let raqe = &workload.raqes[0]; + assert_eq!(raqe.capability, Capability::Cardinality); + assert_eq!(raqe.grouping_labels, LabelSet::from(["job".into()])); + // The test cost table has no HLL rows to solve with, so reuse a + // solved deployment's shape as an HLL deployment on the table. + let (_, _, mut solution) = plan(&group( + "quantile by (job) (0.99, http_requests_total)", + 0.99, + )); + widen_to_instance_job(&mut solution); + let mut deployment = solution.deployments[0].deployment.clone(); + deployment.metric = raqe.metric.clone(); + deployment.capability = Capability::Cardinality; + deployment.config.sketch = "hll".into(); + deployment.config.sketch_config["params"] = json!({"lg_k": 12}); + + let output = + deployments_to_planner_output(MilpConfig::Sql(&sql), &workload, &[deployment], &[0]) + .unwrap(); + let a = aggregation_in(&output, query, QueryLanguage::sql); + assert_eq!(a.aggregation_type, AggregationType::HLL); + assert_eq!(a.grouping_labels, instance_job()); + assert!(a.aggregated_labels.is_empty()); + } + /// The heap `query`'s deployment needs for it: `m · k` for the windows /// its lookback merges and its own `k` (`Deployment::heap_needed`). fn heap_needed(workload: &MilpWorkload, solution: &MilpSolution, query: &str) -> u64 { @@ -879,7 +933,7 @@ mod tests { let [planned] = solution.deployments.as_mut_slice() else { panic!("expected one deployment, got {:?}", solution.deployments); }; - planned.deployment.grouping_labels.insert("instance".into()); + planned.deployment.grouping_labels = LabelSet::from(["instance".into(), "job".into()]); } /// A sketch `by (job)` served from a deployment by (instance, job) is @@ -901,12 +955,6 @@ mod tests { json!({"alpha": 0.01}), AggregationType::DDSketch, ), - ( - "hll", - Capability::Cardinality, - json!({"lg_k": 12}), - AggregationType::HLL, - ), ] { let (config, workload, mut solution) = plan(&group(query, 0.99)); assert_eq!( diff --git a/asap-planner-rs/src/optimizer/saved_plan.rs b/asap-planner-rs/src/optimizer/saved_plan.rs index 2b50235d..654034e9 100644 --- a/asap-planner-rs/src/optimizer/saved_plan.rs +++ b/asap-planner-rs/src/optimizer/saved_plan.rs @@ -391,6 +391,47 @@ mod tests { assert!(yaml.contains("alpha: 0.02"), "{yaml}"); } + /// A DDSketch grouped by (job, label_0) serves `quantile by (job)` as a + /// roll-up: one aggregation at the deployment's grouping. + #[test] + fn writes_a_sketch_roll_up_at_the_deployment_grouping() { + let table = r#"{"workloads": [{"e2e": { + "queries": [ + {"promql": "quantile by (job) (0.5, data)", "interval_secs": 1, "rqes": ["p50"]} + ], + "metrics": {"data": {"type": "gauge", "labels": [ + {"name": "job", "cardinality": 10}, + {"name": "label_0", "cardinality": 1000, "child_of": "job"} + ]}} + }}]}"#; + let mut p50 = pick("p50", 0, "dd", json!({"alpha": 0.02}), 1); + p50["deployment_grouping"] = json!(["job", "label_0"]); + let plan = json!({"plans": [ + {"method": "asap", "weights": "cpu", "kind": "cheapest", "chosen": [p50]}, + ]}); + + let output = run_on(table, plan.to_string().as_bytes(), None).unwrap(); + + let streaming = output + .to_streaming_config(asap_types::enums::QueryLanguage::promql) + .unwrap(); + let aggregations: Vec<_> = streaming.aggregation_configs.values().collect(); + let [aggregation] = aggregations.as_slice() else { + panic!("expected one aggregation, got {aggregations:?}"); + }; + assert_eq!( + aggregation.aggregation_type, + promql_utilities::query_logics::enums::AggregationType::DDSketch + ); + assert_eq!( + aggregation.grouping_labels, + promql_utilities::data_model::KeyByLabelNames::new(vec![ + "job".into(), + "label_0".into() + ]) + ); + } + #[test] fn sla_ms_selects_the_plan_at_that_sla() { let yaml = run(plans(shared_p50()), Some(100.0)) @@ -408,7 +449,7 @@ mod tests { let mut plan = plans(shared_p50()); plan["plans"][0]["chosen"][0]["deployment_grouping"] = json!(["job", "label_0"]); let err = refusal(plan, None); - assert!(err.contains("rolls up only"), "{err}"); + assert!(err.contains("merge across groups"), "{err}"); } #[test] diff --git a/asap-query-engine/src/engines/simple_engine/mod.rs b/asap-query-engine/src/engines/simple_engine/mod.rs index b66fec41..6b0d29aa 100644 --- a/asap-query-engine/src/engines/simple_engine/mod.rs +++ b/asap-query-engine/src/engines/simple_engine/mod.rs @@ -190,17 +190,6 @@ struct RangeQueryReads { keys: Option, } -/// Single-subpopulation types whose merge of two groups' states is the state -/// of their union. Increase is excluded: its merge joins pieces of one -/// counter in time. -const ROLLUP_AGGREGATION_TYPES: [AggregationType; 5] = [ - AggregationType::Sum, - AggregationType::MinMax, - AggregationType::DatasketchesKLL, - AggregationType::DDSketch, - AggregationType::HLL, -]; - type BucketMap = HashMap>>; type TopkPartition = (u64, Vec); type TopkCandidates<'a> = HashMap>; @@ -2586,7 +2575,7 @@ impl SimpleEngine { context.metric, output, context.agg_info.aggregation_id_for_value, grouping )) })?; - if !ROLLUP_AGGREGATION_TYPES.contains(&value_type) { + if !value_type.merges_across_groups() { return Err(QueryExecutionError::Native(format!( "query on metric {} is grouped by {:?}, a roll-up of aggregation {} (grouped by \ {:?}), but {} can't be merged across groups", From 97e480089692bfb6595e3bc7d5720723b1519d83 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 09:57:59 -0400 Subject: [PATCH 09/12] test(planner): check the saved plan's roll-up stage and planned subquery Co-Authored-By: Claude Opus 5.5 --- asap-planner-rs/src/optimizer/saved_plan.rs | 27 ++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/asap-planner-rs/src/optimizer/saved_plan.rs b/asap-planner-rs/src/optimizer/saved_plan.rs index d1f8c3eb..aa694419 100644 --- a/asap-planner-rs/src/optimizer/saved_plan.rs +++ b/asap-planner-rs/src/optimizer/saved_plan.rs @@ -408,9 +408,30 @@ mod tests { fn writes_an_exact_sum_roll_up_as_a_query_time_sum() { let mut plan = plans(shared_p50()); plan["plans"][0]["chosen"][0]["deployment_grouping"] = json!(["job", "label_0"]); - let yaml = run(plan, None).unwrap().to_inference_yaml_string().unwrap(); - assert!(yaml.contains("sum by (job, label_0) (data)"), "{yaml}"); - assert!(yaml.contains("operator: sum"), "{yaml}"); + let inference = run(plan, None) + .unwrap() + .to_inference_config(asap_types::enums::QueryLanguage::promql) + .unwrap(); + let query_config = inference + .query_configs + .iter() + .find(|q| q.query == "sum by (job) (data)") + .expect("a query_config for the roll-up"); + assert_eq!( + query_config.planned_subquery, + "sum by (job, label_0) (data)" + ); + let [stage] = query_config.query_time_aggregations.as_slice() else { + panic!("{:?}", query_config.query_time_aggregations); + }; + assert_eq!( + serde_json::to_value(stage).unwrap(), + json!({ + "operator": "sum", + "grouping": {"mode": "by", "labels": ["job"]}, + "parameter": null, + }) + ); } #[test] From 931f8933bf20d256884736946fc14a53b2a509d1 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 09:59:53 -0400 Subject: [PATCH 10/12] test(query-engine): e2e checks for min, max, count and all roll-ups Each reads a keyed (instance, job) aggregation and regroups it at query time, checking exact instant and range values: min and max from MultipleMinMax, count by (job) and sum of count_over_time from a keyed count, and sum() reducing a keyed sum to one series. Co-Authored-By: Claude Opus 5.5 --- .../tests/e2e_precompute_equivalence.rs | 230 ++++++++++++++---- 1 file changed, 183 insertions(+), 47 deletions(-) diff --git a/asap-query-engine/tests/e2e_precompute_equivalence.rs b/asap-query-engine/tests/e2e_precompute_equivalence.rs index ef83d7d9..964dae55 100644 --- a/asap-query-engine/tests/e2e_precompute_equivalence.rs +++ b/asap-query-engine/tests/e2e_precompute_equivalence.rs @@ -983,69 +983,103 @@ async fn e2e_nested_aggregation_operator_matrix_executes_instant_and_range() { } } -/// The MILP planner's exact roll-up: `sum by (job)` served from a MultipleSum -/// keyed by (instance, job), read at that grouping and summed by `job` at -/// query time. -#[tokio::test] -async fn e2e_exact_rollup_regroups_a_keyed_sum_at_query_time() { - use asap_types::query_config::{ - QueryTimeAggregationOperator, QueryTimeGrouping, QueryTimeGroupingMode, +const EXACT_ROLLUP_METRIC: &str = "exact_rollup_requests"; + +/// One sample per (job, instance) series in the window ending at 1s. +const EXACT_ROLLUP_SAMPLES: &[(&str, &str, i64, f64)] = &[ + ("api", "a", 1_000, 2.0), + ("api", "b", 1_000, 3.0), + ("api", "c", 1_000, 6.0), + ("worker", "a", 1_000, 9.0), + ("worker", "b", 1_000, 4.0), +]; + +/// A query-time stage over the keyed (instance, job) output. +fn rollup_stage( + operator: asap_types::query_config::QueryTimeAggregationOperator, + job_only: bool, +) -> QueryTimeAggregation { + use asap_types::query_config::{QueryTimeGrouping, QueryTimeGroupingMode}; + let grouping = if job_only { + QueryTimeGrouping { + mode: QueryTimeGroupingMode::By, + labels: vec!["job".to_string()], + } + } else { + QueryTimeGrouping { + mode: QueryTimeGroupingMode::All, + labels: Vec::new(), + } }; + QueryTimeAggregation { + operator, + grouping, + parameter: None, + } +} - let metric = "exact_rollup_requests"; - let query = "sum by (job) (exact_rollup_requests)"; +/// The MILP planner's exact roll-up: `query` served from an aggregation +/// keyed by (instance, job), read as `planned_subquery` at that grouping and +/// regrouped by `stage`. Checks the instant result at 1s and the range +/// result's last samples against `expected`, keyed by output label values. +#[allow(clippy::too_many_arguments)] +async fn assert_exact_rollup( + port: u16, + query: &str, + aggregation_type: AggregationType, + sub_type: &str, + planned_subquery: &str, + stage: QueryTimeAggregation, + samples: &[(&str, &str, i64, f64)], + expected: &[(&[&str], f64)], +) { + let metric = EXACT_ROLLUP_METRIC; + let series = |job, instance, timestamp_ms, value| { + make_timeseries( + metric, + vec![("job", job), ("instance", instance)], + timestamp_ms, + value, + ) + }; + let mut timeseries: Vec = samples + .iter() + .map(|&(job, instance, timestamp_ms, value)| series(job, instance, timestamp_ms, value)) + .collect(); + // Later samples on every series close the first windows. + let mut closed: Vec<(&str, &str)> = Vec::new(); + for &(job, instance, _, _) in samples { + if !closed.contains(&(job, instance)) { + closed.push((job, instance)); + timeseries.push(series(job, instance, 3_000, 0.0)); + } + } let scenario = NativeDagScenario { - port: 19430, + port, metric, query, aggregation_configs: vec![make_agg_config_full( 29, metric, - AggregationType::MultipleSum, - "sum", + aggregation_type, + sub_type, 1_000, 0, vec![], vec!["instance", "job"], )], schema_labels: vec!["instance".to_string(), "job".to_string()], - samples: vec![ - make_timeseries(metric, vec![("job", "api"), ("instance", "a")], 1_000, 2.0), - make_timeseries(metric, vec![("job", "api"), ("instance", "b")], 1_000, 3.0), - make_timeseries( - metric, - vec![("job", "worker"), ("instance", "a")], - 1_000, - 9.0, - ), - make_timeseries(metric, vec![("job", "api"), ("instance", "a")], 3_000, 0.0), - make_timeseries( - metric, - vec![("job", "worker"), ("instance", "a")], - 3_000, - 0.0, - ), - ], + samples: timeseries, evaluation_time_seconds: 1.0, base_interval_ms: 1_000, }; let (engine, query) = scenario - .build_engine_with_plan( - "sum by (instance, job) (exact_rollup_requests)", - vec![QueryTimeAggregation { - operator: QueryTimeAggregationOperator::Sum, - grouping: QueryTimeGrouping { - mode: QueryTimeGroupingMode::By, - labels: vec!["job".to_string()], - }, - parameter: None, - }], - ) + .build_engine_with_plan(planned_subquery, vec![stage]) .await; - let expected = vec![ - (vec!["api".to_string()], 5.0), - (vec!["worker".to_string()], 9.0), - ]; + let expected: Vec<(Vec, f64)> = expected + .iter() + .map(|(labels, value)| (labels.iter().map(|l| l.to_string()).collect(), *value)) + .collect(); let Some((_, instant)) = engine .handle_query_promql(query.clone(), 1.0) @@ -1062,10 +1096,10 @@ async fn e2e_exact_rollup_regroups_a_keyed_sum_at_query_time() { .map(|value| (value.labels.labels, value.value)) .collect(); instant_values.sort_by(|left, right| left.0.cmp(&right.0)); - assert_eq!(instant_values, expected, "instant"); + assert_eq!(instant_values, expected, "{query} instant"); let Some((_, range)) = engine - .handle_range_query_promql(query, 1.0, 2.0, 1.0) + .handle_range_query_promql(query.clone(), 1.0, 2.0, 1.0) .expect("roll-up range should execute locally") else { panic!("roll-up range returned no local result"); @@ -1085,7 +1119,109 @@ async fn e2e_exact_rollup_regroups_a_keyed_sum_at_query_time() { }) .collect(); range_values.sort_by(|left, right| left.0.cmp(&right.0)); - assert_eq!(range_values, expected, "range"); + assert_eq!(range_values, expected, "{query} range"); +} + +/// `sum by (job)` from a MultipleSum keyed by (instance, job), summed by +/// `job` at query time. +#[tokio::test] +async fn e2e_exact_rollup_regroups_a_keyed_sum_at_query_time() { + use asap_types::query_config::QueryTimeAggregationOperator; + assert_exact_rollup( + 19430, + "sum by (job) (exact_rollup_requests)", + AggregationType::MultipleSum, + "sum", + "sum by (instance, job) (exact_rollup_requests)", + rollup_stage(QueryTimeAggregationOperator::Sum, true), + EXACT_ROLLUP_SAMPLES, + &[(&["api"], 11.0), (&["worker"], 13.0)], + ) + .await; +} + +#[tokio::test] +async fn e2e_exact_rollup_regroups_a_keyed_min_at_query_time() { + use asap_types::query_config::QueryTimeAggregationOperator; + assert_exact_rollup( + 19431, + "min by (job) (exact_rollup_requests)", + AggregationType::MultipleMinMax, + "min", + "min by (instance, job) (exact_rollup_requests)", + rollup_stage(QueryTimeAggregationOperator::Min, true), + EXACT_ROLLUP_SAMPLES, + &[(&["api"], 2.0), (&["worker"], 4.0)], + ) + .await; +} + +#[tokio::test] +async fn e2e_exact_rollup_regroups_a_keyed_max_at_query_time() { + use asap_types::query_config::QueryTimeAggregationOperator; + assert_exact_rollup( + 19432, + "max by (job) (exact_rollup_requests)", + AggregationType::MultipleMinMax, + "max", + "max by (instance, job) (exact_rollup_requests)", + rollup_stage(QueryTimeAggregationOperator::Max, true), + EXACT_ROLLUP_SAMPLES, + &[(&["api"], 6.0), (&["worker"], 9.0)], + ) + .await; +} + +/// A coarse count is the sum of the fine counts, read from a keyed count. +#[tokio::test] +async fn e2e_exact_rollup_sums_a_keyed_count_at_query_time() { + use asap_types::query_config::QueryTimeAggregationOperator; + assert_exact_rollup( + 19433, + "count by (job) (exact_rollup_requests)", + AggregationType::MultipleSum, + "count", + "count by (instance, job) (exact_rollup_requests)", + rollup_stage(QueryTimeAggregationOperator::Sum, true), + EXACT_ROLLUP_SAMPLES, + &[(&["api"], 3.0), (&["worker"], 2.0)], + ) + .await; +} + +#[tokio::test] +async fn e2e_exact_rollup_sums_a_keyed_count_over_time_at_query_time() { + use asap_types::query_config::QueryTimeAggregationOperator; + let mut samples = EXACT_ROLLUP_SAMPLES.to_vec(); + samples.splice(0..0, [("api", "a", 500, 1.0), ("worker", "b", 500, 1.0)]); + assert_exact_rollup( + 19434, + "sum by (job) (count_over_time(exact_rollup_requests[1s]))", + AggregationType::MultipleSum, + "count", + "sum by (instance, job) (count_over_time(exact_rollup_requests[1s]))", + rollup_stage(QueryTimeAggregationOperator::Sum, true), + &samples, + &[(&["api"], 4.0), (&["worker"], 3.0)], + ) + .await; +} + +/// A query with no output labels reduces every keyed group into one. +#[tokio::test] +async fn e2e_exact_rollup_reduces_a_keyed_sum_to_all_at_query_time() { + use asap_types::query_config::QueryTimeAggregationOperator; + assert_exact_rollup( + 19435, + "sum(exact_rollup_requests)", + AggregationType::MultipleSum, + "sum", + "sum by (instance, job) (exact_rollup_requests)", + rollup_stage(QueryTimeAggregationOperator::Sum, false), + EXACT_ROLLUP_SAMPLES, + &[(&[], 24.0)], + ) + .await; } #[tokio::test] From 4f803dd91a5c5c5e4163adeb4b6f26295e204cfd Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 10:26:28 -0400 Subject: [PATCH 11/12] fix(planner): gate exact roll-ups on the aggregation type; test unregroupable and without - An exact roll-up needs a MultipleSum or MultipleMinMax aggregation, so the check reads the aggregation type instead of repeating the (variant, capability) mapping. - Test that a query with no outer aggregation is UnregroupableQuery. - Engine e2e test for a without stage over the keyed output. Co-Authored-By: Claude Opus 5.5 --- asap-planner-rs/src/optimizer/milp_output.rs | 28 +++++++++++++++++-- .../tests/e2e_precompute_equivalence.rs | 27 ++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/asap-planner-rs/src/optimizer/milp_output.rs b/asap-planner-rs/src/optimizer/milp_output.rs index 28abe76b..4b6653f4 100644 --- a/asap-planner-rs/src/optimizer/milp_output.rs +++ b/asap-planner-rs/src/optimizer/milp_output.rs @@ -298,9 +298,11 @@ fn query_plan_entry( deployment: label_names(fine), reason, }; - match deployment.capability { - Capability::Sum | Capability::Count | Capability::Min | Capability::Max => {} - _ => return Err(unsupported(RollupBlocker::Aggregation)), + if !matches!( + aggregation_type, + AggregationType::MultipleSum | AggregationType::MultipleMinMax + ) { + return Err(unsupported(RollupBlocker::Aggregation)); } if let Some(reason) = stage_blocker { return Err(unsupported(reason)); @@ -1228,6 +1230,26 @@ mod tests { ); } + /// A roll-up regroups the query's outer aggregation; a query without one + /// can't be regrouped. Unreachable from real workloads today (a leaf with + /// no aggregation keeps every label, so it is never a roll-up), hence the + /// swapped-in query string. + #[test] + fn exact_rollup_of_a_query_without_an_aggregation_is_unregroupable() { + let query = "sum_over_time(http_requests_total[1m])"; + let (config, mut workload, mut solution) = + plan(&group("sum by (job) (http_requests_total)", 0.99)); + widen_to_instance_job(&mut solution); + workload.items[0].query_strings = vec![query.to_string()]; + let err = plan_to_planner_output(MilpConfig::Promql(&config), &workload, &solution) + .err() + .unwrap(); + assert!( + matches!(&err, MilpOutputError::UnregroupableQuery(q) if q == query), + "{err}" + ); + } + /// A query with no output labels regroups over everything. #[test] fn exact_rollup_to_no_labels_regroups_all() { diff --git a/asap-query-engine/tests/e2e_precompute_equivalence.rs b/asap-query-engine/tests/e2e_precompute_equivalence.rs index 964dae55..7ca83075 100644 --- a/asap-query-engine/tests/e2e_precompute_equivalence.rs +++ b/asap-query-engine/tests/e2e_precompute_equivalence.rs @@ -1207,6 +1207,33 @@ async fn e2e_exact_rollup_sums_a_keyed_count_over_time_at_query_time() { .await; } +/// `sum without (instance)` regroups the keyed (instance, job) output by a +/// `without` stage, which keeps `job`. +#[tokio::test] +async fn e2e_exact_rollup_regroups_a_keyed_sum_without_a_label_at_query_time() { + use asap_types::query_config::{ + QueryTimeAggregationOperator, QueryTimeGrouping, QueryTimeGroupingMode, + }; + assert_exact_rollup( + 19436, + "sum without (instance) (exact_rollup_requests)", + AggregationType::MultipleSum, + "sum", + "sum by (instance, job) (exact_rollup_requests)", + QueryTimeAggregation { + operator: QueryTimeAggregationOperator::Sum, + grouping: QueryTimeGrouping { + mode: QueryTimeGroupingMode::Without, + labels: vec!["instance".to_string()], + }, + parameter: None, + }, + EXACT_ROLLUP_SAMPLES, + &[(&["api"], 11.0), (&["worker"], 13.0)], + ) + .await; +} + /// A query with no output labels reduces every keyed group into one. #[tokio::test] async fn e2e_exact_rollup_reduces_a_keyed_sum_to_all_at_query_time() { From ba348ee22894008644182b0196ba53cab8cf8a1b Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 13 Oct 2026 10:33:36 -0400 Subject: [PATCH 12/12] fix(promql-compliance): harden the fixed planner output override - Check docker compose >= 2.24.4 (needed for !override) before using it, and document it. - Clear COMPOSE_PROFILES so an inherited value can't re-enable the planner. - Bind the fixed directory with long syntax, so a ':' in its path is safe. - Only skip planner-input generation when a Compose project is used. - LoadSuite rejects a relative planner_output_dir and validates an absolute one, so every loaded suite holds an absolute, checked path. - The checked-in fixture test also checks aggregation ids, metrics and grouping labels; an engine test parses the fixed configs with the engine's own loaders. Co-Authored-By: Claude Opus 5.5 --- asap-query-engine/src/utils/file_io.rs | 37 ++++++++ promql-compliance/HOW_TO.md | 6 +- .../checked_in_fixture_validation_test.go | 58 +++++++++++- promql-compliance/runner/config.go | 26 +++++- promql-compliance/runner/config_test.go | 43 ++++++--- promql-compliance/runner/run.go | 90 ++++++++++++++++--- promql-compliance/runner/run_test.go | 78 ++++++++++++++-- 7 files changed, 303 insertions(+), 35 deletions(-) diff --git a/asap-query-engine/src/utils/file_io.rs b/asap-query-engine/src/utils/file_io.rs index dd129d49..f057f71a 100644 --- a/asap-query-engine/src/utils/file_io.rs +++ b/asap-query-engine/src/utils/file_io.rs @@ -41,6 +41,43 @@ mod tests { use std::io::Write; use tempfile::NamedTempFile; + /// The PromQL compliance roll-up suite ships hand-written planner output; + /// parse it the way the engine does so it can't drift from the format. + #[test] + fn compliance_rollup_planner_output_loads_as_engine_config() { + let directory = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../promql-compliance/suites/rollup-planner-output" + ); + let inference = read_inference_config( + &format!("{directory}/inference_config.yaml"), + QueryLanguage::promql, + ) + .unwrap(); + let streaming = + read_streaming_config(&format!("{directory}/streaming_config.yaml"), &inference) + .unwrap(); + + assert!(!inference.query_configs.is_empty()); + for query_config in &inference.query_configs { + assert!( + !query_config.aggregations.is_empty(), + "{}", + query_config.query + ); + for reference in &query_config.aggregations { + assert!( + streaming + .get_aggregation_config(reference.aggregation_id) + .is_some(), + "{} names missing aggregation {}", + query_config.query, + reference.aggregation_id + ); + } + } + } + #[test] fn test_read_streaming_config() { let streaming_yaml_content = r#" diff --git a/promql-compliance/HOW_TO.md b/promql-compliance/HOW_TO.md index e05602c3..d3a99a1f 100644 --- a/promql-compliance/HOW_TO.md +++ b/promql-compliance/HOW_TO.md @@ -144,8 +144,10 @@ The path is relative to the suite file. The directory must contain `streaming_config.yaml` and `inference_config.yaml` in the planner's output format; the runner fails at load time if either is missing. The planner service then does not run, and the query engine reads the directory in place -of the planner's output volume. With already-running targets the field has no -effect. `suites/rollup.yaml` is an example. +of the planner's output volume. This needs Docker Compose v2.24.4 or later +(for `!override`); the runner checks the version first. With +already-running targets the field has no effect, and the planner input is +generated and validated as usual. `suites/rollup.yaml` is an example. ## Configure tolerance diff --git a/promql-compliance/runner/checked_in_fixture_validation_test.go b/promql-compliance/runner/checked_in_fixture_validation_test.go index 8eb38c00..18b56400 100644 --- a/promql-compliance/runner/checked_in_fixture_validation_test.go +++ b/promql-compliance/runner/checked_in_fixture_validation_test.go @@ -85,10 +85,11 @@ func TestCheckedInNativeDagSuites(t *testing.T) { } // Every rollup suite query must have a query config in the fixed planner -// output; a mismatched string would make the engine reject it rather than -// exercise a roll-up. +// output, naming aggregations that exist, on a metric and labels the dataset +// has; a mismatch would otherwise surface only as a slow Docker-suite failure. func TestCheckedInRollupSuiteMatchesFixedPlannerOutput(t *testing.T) { - if _, err := seeder.LoadFixture("../datasets/rollup.yaml"); err != nil { + fixture, err := seeder.LoadFixture("../datasets/rollup.yaml") + if err != nil { t.Fatalf("LoadFixture: %v", err) } suite, err := LoadSuiteFile("../suites/rollup.yaml") @@ -104,12 +105,61 @@ func TestCheckedInRollupSuiteMatchesFixedPlannerOutput(t *testing.T) { } var inference struct { Queries []struct { - Query string `yaml:"query"` + Query string `yaml:"query"` + Aggregations []struct { + AggregationID int `yaml:"aggregation_id"` + } `yaml:"aggregations"` } `yaml:"queries"` } if err := yaml.Unmarshal(contents, &inference); err != nil { t.Fatalf("parse inference config: %v", err) } + streamingContents, err := os.ReadFile(filepath.Join(suite.PlannerOutputDir, plannerStreamingConfigFile)) + if err != nil { + t.Fatalf("read streaming config: %v", err) + } + var streaming struct { + Aggregations []struct { + AggregationID int `yaml:"aggregationId"` + Metric string `yaml:"metric"` + Labels struct { + Grouping []string `yaml:"grouping"` + } `yaml:"labels"` + } `yaml:"aggregations"` + } + if err := yaml.Unmarshal(streamingContents, &streaming); err != nil { + t.Fatalf("parse streaming config: %v", err) + } + datasetLabels := make(map[string]map[string]struct{}) + for _, series := range fixture.Series { + if datasetLabels[series.Metric] == nil { + datasetLabels[series.Metric] = make(map[string]struct{}) + } + for label := range series.Labels { + datasetLabels[series.Metric][label] = struct{}{} + } + } + aggregationIDs := make(map[int]struct{}, len(streaming.Aggregations)) + for _, aggregation := range streaming.Aggregations { + aggregationIDs[aggregation.AggregationID] = struct{}{} + labels, ok := datasetLabels[aggregation.Metric] + if !ok { + t.Errorf("aggregation %d is on metric %q, which the dataset lacks", aggregation.AggregationID, aggregation.Metric) + continue + } + for _, label := range aggregation.Labels.Grouping { + if _, ok := labels[label]; !ok { + t.Errorf("aggregation %d groups by %q, which %q series lack", aggregation.AggregationID, label, aggregation.Metric) + } + } + } + for _, query := range inference.Queries { + for _, reference := range query.Aggregations { + if _, ok := aggregationIDs[reference.AggregationID]; !ok { + t.Errorf("query %q names aggregation %d, which the streaming config lacks", query.Query, reference.AggregationID) + } + } + } configured := make(map[string]struct{}, len(inference.Queries)) for _, query := range inference.Queries { configured[query.Query] = struct{}{} diff --git a/promql-compliance/runner/config.go b/promql-compliance/runner/config.go index 0c197a40..5eee6309 100644 --- a/promql-compliance/runner/config.go +++ b/promql-compliance/runner/config.go @@ -74,8 +74,30 @@ type Tolerance struct { } // LoadSuite parses and validates a query suite. It rejects incomplete cases -// rather than silently selecting wall-clock defaults. +// rather than silently selecting wall-clock defaults. A planner_output_dir +// must be absolute here; a relative one needs the suite file's location, so +// use LoadSuiteFile. func LoadSuite(contents []byte) (Suite, error) { + suite, err := parseSuite(contents) + if err != nil { + return Suite{}, err + } + if suite.PlannerOutputDir != "" { + if !filepath.IsAbs(suite.PlannerOutputDir) { + return Suite{}, fmt.Errorf( + "query suite %q: planner_output_dir %q is relative; load the suite with LoadSuiteFile", + suite.Name, + suite.PlannerOutputDir, + ) + } + if suite.PlannerOutputDir, err = resolvePlannerOutputDir("", suite.PlannerOutputDir); err != nil { + return Suite{}, fmt.Errorf("query suite %q: %w", suite.Name, err) + } + } + return suite, nil +} + +func parseSuite(contents []byte) (Suite, error) { var suite Suite decoder := yaml.NewDecoder(bytes.NewReader(contents)) decoder.KnownFields(true) @@ -140,7 +162,7 @@ func LoadSuiteFile(path string) (Suite, error) { if err != nil { return Suite{}, fmt.Errorf("read query suite %q: %w", path, err) } - suite, err := LoadSuite(contents) + suite, err := parseSuite(contents) if err != nil { return Suite{}, err } diff --git a/promql-compliance/runner/config_test.go b/promql-compliance/runner/config_test.go index 8aa0a234..2e346699 100644 --- a/promql-compliance/runner/config_test.go +++ b/promql-compliance/runner/config_test.go @@ -195,11 +195,7 @@ func writePlannerOutputSuite(t *testing.T, configured string, files ...string) ( if err := os.MkdirAll(outputDirectory, 0o755); err != nil { t.Fatalf("create planner output directory: %v", err) } - for _, name := range files { - if err := os.WriteFile(filepath.Join(outputDirectory, name), []byte("{}\n"), 0o600); err != nil { - t.Fatalf("write %s: %v", name, err) - } - } + writePlannerOutputFiles(t, outputDirectory, files...) suitePath := filepath.Join(suiteDirectory, "suite.yaml") if err := os.WriteFile(suitePath, []byte(fmt.Sprintf(plannerOutputSuite, configured)), 0o600); err != nil { t.Fatalf("write suite: %v", err) @@ -207,6 +203,15 @@ func writePlannerOutputSuite(t *testing.T, configured string, files ...string) ( return suitePath, outputDirectory } +func writePlannerOutputFiles(t *testing.T, directory string, files ...string) { + t.Helper() + for _, name := range files { + if err := os.WriteFile(filepath.Join(directory, name), []byte("{}\n"), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } +} + func TestLoadSuiteFileResolvesPlannerOutputDirAgainstSuiteFile(t *testing.T) { suitePath, outputDirectory := writePlannerOutputSuite(t, "fixed", plannerStreamingConfigFile, plannerInferenceConfigFile) @@ -231,11 +236,7 @@ func TestLoadSuiteFileResolvesPlannerOutputDirAgainstSuiteFile(t *testing.T) { func TestLoadSuiteFileAcceptsAbsolutePlannerOutputDir(t *testing.T) { outputDirectory := t.TempDir() - for _, name := range []string{plannerStreamingConfigFile, plannerInferenceConfigFile} { - if err := os.WriteFile(filepath.Join(outputDirectory, name), []byte("{}\n"), 0o600); err != nil { - t.Fatalf("write %s: %v", name, err) - } - } + writePlannerOutputFiles(t, outputDirectory, plannerStreamingConfigFile, plannerInferenceConfigFile) suitePath, _ := writePlannerOutputSuite(t, outputDirectory) suite, err := LoadSuiteFile(suitePath) @@ -290,3 +291,25 @@ func TestLoadSuiteFileRejectsPlannerOutputDirectoryInPlaceOfFile(t *testing.T) { t.Fatal("LoadSuiteFile accepted a directory as a planner config file") } } + +// LoadSuite has no suite file to resolve a relative planner_output_dir +// against, so every Suite it returns already holds an absolute, validated one. +func TestLoadSuiteRequiresAnAbsoluteValidPlannerOutputDir(t *testing.T) { + if _, err := LoadSuite([]byte(fmt.Sprintf(plannerOutputSuite, "fixed"))); err == nil || + !strings.Contains(err.Error(), "LoadSuiteFile") { + t.Fatalf("LoadSuite accepted a relative planner_output_dir (err: %v)", err) + } + + outputDirectory := t.TempDir() + if _, err := LoadSuite([]byte(fmt.Sprintf(plannerOutputSuite, outputDirectory))); err == nil { + t.Fatal("LoadSuite accepted a planner_output_dir without its config files") + } + writePlannerOutputFiles(t, outputDirectory, plannerStreamingConfigFile, plannerInferenceConfigFile) + suite, err := LoadSuite([]byte(fmt.Sprintf(plannerOutputSuite, outputDirectory))) + if err != nil { + t.Fatalf("LoadSuite: %v", err) + } + if got := suite.PlannerOutputDir; got != outputDirectory { + t.Fatalf("planner output dir = %q, want %q", got, outputDirectory) + } +} diff --git a/promql-compliance/runner/run.go b/promql-compliance/runner/run.go index 595e4af3..c5c03ad4 100644 --- a/promql-compliance/runner/run.go +++ b/promql-compliance/runner/run.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "sort" + "strconv" "strings" "time" @@ -36,7 +37,8 @@ const ( // fixedPlannerOutputCompose overrides the base Compose file for suites with a // planner_output_dir: the planner sits behind a never-activated profile so it // does not run, and the query engine mounts the fixed directory where the -// planner's output volume would be. +// planner's output volume would be. The bind uses long syntax so a host path +// containing ':' stays one source. const fixedPlannerOutputCompose = `services: planner: profiles: ["generated-planner-output"] @@ -45,9 +47,16 @@ const fixedPlannerOutputCompose = `services: prometheus: condition: service_healthy volumes: - - "${` + fixedPlannerOutputDirEnv + `:?` + fixedPlannerOutputDirEnv + ` is required}:` + plannerOutputMountPath + `:ro" + - type: bind + source: "${` + fixedPlannerOutputDirEnv + `:?` + fixedPlannerOutputDirEnv + ` is required}" + target: ` + plannerOutputMountPath + ` + read_only: true ` +// The override's `!override` tag needs this Compose release or later; older +// ones merge depends_on and fail on the disabled planner. +var minComposeVersionForOverride = [3]int{2, 24, 4} + type RunOptions struct { DatasetPath string SuitePath string @@ -89,7 +98,15 @@ func Run(ctx context.Context, options RunOptions) (Report, error) { return Report{}, fmt.Errorf("create run directory: %w", err) } defer os.RemoveAll(runDirectory) - if err := writeGeneratedConfigs(runDirectory, fixture, suite); err != nil { + // Fixed planner output only replaces the planner inside a Compose project; + // with caller-owned targets the planner input is still generated. + fixedPlannerOutput := suite.PlannerOutputDir != "" && len(options.ComposeFiles) > 0 + if fixedPlannerOutput { + if err := requireComposeOverrideSupport(ctx); err != nil { + return Report{}, err + } + } + if err := writeGeneratedConfigs(runDirectory, fixture, suite, fixedPlannerOutput); err != nil { return Report{}, err } @@ -98,12 +115,17 @@ func Run(ctx context.Context, options RunOptions) (Report, error) { project: options.ComposeProject, env: []string{"ASAP_TEST_CONFIG_DIR=" + runDirectory}, } - if suite.PlannerOutputDir != "" { + if fixedPlannerOutput { lifecycle.files = append( append([]string(nil), options.ComposeFiles...), filepath.Join(runDirectory, fixedPlannerOutputComposeFile), ) - lifecycle.env = append(lifecycle.env, fixedPlannerOutputDirEnv+"="+suite.PlannerOutputDir) + lifecycle.env = append( + lifecycle.env, + fixedPlannerOutputDirEnv+"="+suite.PlannerOutputDir, + // An inherited COMPOSE_PROFILES could re-enable the planner. + "COMPOSE_PROFILES=", + ) } if len(options.ComposeFiles) > 0 { if err := lifecycle.Start(ctx); err != nil { @@ -372,14 +394,14 @@ type plannerCleanup struct { } // writeGeneratedConfigs writes the engine config plus either the planner input -// or, for a suite with fixed planner output, the Compose override that skips -// the planner. The planner input is not generated in the fixed case, so its -// timing derivation cannot reject queries the planner never sees. -func writeGeneratedConfigs(directory string, fixture seeder.Fixture, suite Suite) error { +// or, with fixed planner output, the Compose override that skips the planner. +// The planner input is not generated in the fixed case, so its timing +// derivation cannot reject queries the planner never sees. +func writeGeneratedConfigs(directory string, fixture seeder.Fixture, suite Suite, fixedPlannerOutput bool) error { if err := writeEngineConfig(directory); err != nil { return err } - if suite.PlannerOutputDir != "" { + if fixedPlannerOutput { if err := os.WriteFile(filepath.Join(directory, fixedPlannerOutputComposeFile), []byte(fixedPlannerOutputCompose), 0o600); err != nil { return fmt.Errorf("write fixed planner output compose override: %w", err) } @@ -388,6 +410,54 @@ func writeGeneratedConfigs(directory string, fixture seeder.Fixture, suite Suite return writePlannerConfig(directory, fixture, suite) } +// requireComposeOverrideSupport fails unless `docker compose` understands the +// fixed-output override's `!override` tag. +func requireComposeOverrideSupport(ctx context.Context) error { + output, err := exec.CommandContext(ctx, "docker", "compose", "version", "--short").Output() + if err != nil { + return fmt.Errorf("check docker compose version for planner_output_dir: %w", err) + } + version := strings.TrimSpace(string(output)) + ok, err := composeVersionAtLeast(version, minComposeVersionForOverride) + if err != nil { + return err + } + if !ok { + return fmt.Errorf( + "planner_output_dir needs docker compose >= %d.%d.%d, found %s", + minComposeVersionForOverride[0], + minComposeVersionForOverride[1], + minComposeVersionForOverride[2], + version, + ) + } + return nil +} + +// composeVersionAtLeast compares a `docker compose version --short` string +// such as "2.29.1" or "v2.24.4-desktop.1" against minimum. +func composeVersionAtLeast(version string, minimum [3]int) (bool, error) { + core := strings.SplitN(strings.TrimPrefix(version, "v"), "-", 2)[0] + parts := strings.Split(core, ".") + if len(parts) != 3 { + return false, fmt.Errorf("unrecognized docker compose version %q", version) + } + var got [3]int + for i, part := range parts { + number, err := strconv.Atoi(part) + if err != nil { + return false, fmt.Errorf("unrecognized docker compose version %q", version) + } + got[i] = number + } + for i := range got { + if got[i] != minimum[i] { + return got[i] > minimum[i], nil + } + } + return true, nil +} + func writePlannerConfig(directory string, fixture seeder.Fixture, suite Suite) error { metrics := make(map[string]map[string]struct{}) for _, series := range fixture.Series { diff --git a/promql-compliance/runner/run_test.go b/promql-compliance/runner/run_test.go index a7bcc188..735cb34c 100644 --- a/promql-compliance/runner/run_test.go +++ b/promql-compliance/runner/run_test.go @@ -66,7 +66,7 @@ func TestGeneratedConfigsUseCurrentPlannerAndEngineSchema(t *testing.T) { Queries: []QueryCase{{Name: "up", Expr: "up"}}, } - if err := writeGeneratedConfigs(directory, fixture, suite); err != nil { + if err := writeGeneratedConfigs(directory, fixture, suite, false); err != nil { t.Fatalf("writeGeneratedConfigs: %v", err) } @@ -124,7 +124,7 @@ func TestGeneratedPlannerGroupsUseCompatibleQueryTiming(t *testing.T) { }, } - if err := writeGeneratedConfigs(directory, fixture, suite); err != nil { + if err := writeGeneratedConfigs(directory, fixture, suite, false); err != nil { t.Fatalf("writeGeneratedConfigs: %v", err) } @@ -176,7 +176,7 @@ func TestGeneratedConfigsWithFixedPlannerOutputSkipPlanner(t *testing.T) { PlannerOutputDir: "/fixed/planner/output", } - if err := writeGeneratedConfigs(directory, fixture, suite); err != nil { + if err := writeGeneratedConfigs(directory, fixture, suite, true); err != nil { t.Fatalf("writeGeneratedConfigs: %v", err) } @@ -195,7 +195,12 @@ func TestGeneratedConfigsWithFixedPlannerOutputSkipPlanner(t *testing.T) { } `yaml:"planner"` QueryEngine struct { DependsOn yaml.Node `yaml:"depends_on"` - Volumes []string `yaml:"volumes"` + Volumes []struct { + Type string `yaml:"type"` + Source string `yaml:"source"` + Target string `yaml:"target"` + ReadOnly bool `yaml:"read_only"` + } `yaml:"volumes"` } `yaml:"queryengine"` } `yaml:"services"` } @@ -216,9 +221,11 @@ func TestGeneratedConfigsWithFixedPlannerOutputSkipPlanner(t *testing.T) { if _, ok := dependencies["planner"]; ok { t.Fatal("queryengine still depends on the planner") } - wantVolume := "${" + fixedPlannerOutputDirEnv + ":?" + fixedPlannerOutputDirEnv + " is required}:" + plannerOutputMountPath + ":ro" - if got := override.Services.QueryEngine.Volumes; len(got) != 1 || got[0] != wantVolume { - t.Fatalf("queryengine volumes = %v, want [%s]", got, wantVolume) + volumes := override.Services.QueryEngine.Volumes + wantSource := "${" + fixedPlannerOutputDirEnv + ":?" + fixedPlannerOutputDirEnv + " is required}" + if len(volumes) != 1 || volumes[0].Type != "bind" || volumes[0].Source != wantSource || + volumes[0].Target != plannerOutputMountPath || !volumes[0].ReadOnly { + t.Fatalf("queryengine volumes = %+v, want one read-only bind of %s at %s", volumes, wantSource, plannerOutputMountPath) } engineContents, err := os.ReadFile(filepath.Join(directory, "engine_config.yaml")) @@ -239,3 +246,60 @@ func TestGeneratedConfigsWithFixedPlannerOutputSkipPlanner(t *testing.T) { t.Fatalf("engine streaming_config = %q, want %q", got, want) } } + +// With caller-owned targets there is no Compose project to swap the planner +// out of, so the planner input is still generated and validated. +func TestGeneratedConfigsWithoutComposeStillWritePlannerInput(t *testing.T) { + directory := t.TempDir() + fixture := seeder.Fixture{ + Name: "single-series", + Series: []seeder.FixtureSeries{{ + Metric: "up", + Labels: map[string]string{"job": "test"}, + Samples: []seeder.FixtureSample{{OffsetSeconds: 0, Value: 1}}, + }}, + } + suite := Suite{ + Name: "fixed-output", + Queries: []QueryCase{{Name: "up", Expr: "up"}}, + PlannerOutputDir: "/fixed/planner/output", + } + + if err := writeGeneratedConfigs(directory, fixture, suite, false); err != nil { + t.Fatalf("writeGeneratedConfigs: %v", err) + } + + if _, err := os.Stat(filepath.Join(directory, "controller-config.yaml")); err != nil { + t.Fatalf("planner config not generated without Compose: %v", err) + } + if _, err := os.Stat(filepath.Join(directory, fixedPlannerOutputComposeFile)); !os.IsNotExist(err) { + t.Fatalf("compose override written without Compose (stat err: %v)", err) + } +} + +func TestComposeVersionAtLeast(t *testing.T) { + minimum := [3]int{2, 24, 4} + for _, test := range []struct { + version string + want bool + }{ + {"2.24.4", true}, + {"v2.29.1", true}, + {"2.24.4-desktop.1", true}, + {"3.0.0", true}, + {"2.24.3", false}, + {"2.9.10", false}, + {"1.29.2", false}, + } { + got, err := composeVersionAtLeast(test.version, minimum) + if err != nil { + t.Fatalf("composeVersionAtLeast(%q): %v", test.version, err) + } + if got != test.want { + t.Fatalf("composeVersionAtLeast(%q) = %v, want %v", test.version, got, test.want) + } + } + if _, err := composeVersionAtLeast("not-a-version", minimum); err == nil { + t.Fatal("an unrecognized version must be an error") + } +}