From 19ee301d829ebaeac055167ff30f8f43fb3dae52 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Fri, 24 Jul 2026 10:53:15 -0700 Subject: [PATCH] feat(speculation): standard composed speculator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add submitqueue/extension/speculation/speculator/standard, the standard Speculator composed from a Generator and an Allocator. It decides nothing itself — it opens the generator's candidate stream and hands it to the allocator — so its behavior is entirely that of the two parts it is built from. Pairing bestfirst with sticky yields the default speculation policy. --- .../speculation/speculator/README.md | 6 + .../speculator/standard/BUILD.bazel | 31 +++++ .../speculator/standard/standard.go | 52 +++++++++ .../speculator/standard/standard_test.go | 108 ++++++++++++++++++ 4 files changed, 197 insertions(+) create mode 100644 submitqueue/extension/speculation/speculator/standard/BUILD.bazel create mode 100644 submitqueue/extension/speculation/speculator/standard/standard.go create mode 100644 submitqueue/extension/speculation/speculator/standard/standard_test.go diff --git a/submitqueue/extension/speculation/speculator/README.md b/submitqueue/extension/speculation/speculator/README.md index 835af489..b5fe11cb 100644 --- a/submitqueue/extension/speculation/speculator/README.md +++ b/submitqueue/extension/speculation/speculator/README.md @@ -8,6 +8,12 @@ The `speculator` package defines the one speculation extension the speculate con Like the other extensions, a `Speculator` is selected **per queue** by the wiring layer through the `Config` (queue name) and `Factory` interface. Budget, depth bound, clock, and any extra data are injected at construction by the integrator, not carried on the contract. +## The `standard` Speculator + +The `standard` subpackage is a `Speculator` assembled from two swappable composition points — a `Generator` and an `Allocator` (the sibling `generator` and `allocator` packages). The `standard` Speculator decides nothing itself: it opens the `Generator`'s candidate stream and hands it to the `Allocator`. All of its behavior is therefore the behavior of whichever `Generator` and `Allocator` it is built from — the `Generator` chooses which paths are worth considering and in what order, the `Allocator` chooses which of them to fund. Splitting the two lets candidate scoring and budget-rationing policy vary independently. + +These composition points are **not** controller-facing extensions: an alternate `Speculator` could compute build and cancel decisions directly, without a `Generator`/`Allocator` split at all. The controller depends only on the `Speculator` contract either way. + ## Adding a backend Create a package under `speculator//` whose `New(...)` returns a `speculator.Speculator`, injecting whatever it needs at construction. Resolve any content it requires internally; do not add a `Config` or `Factory` implementation here — per-queue routing and the factory adapter live in the wiring layer. diff --git a/submitqueue/extension/speculation/speculator/standard/BUILD.bazel b/submitqueue/extension/speculation/speculator/standard/BUILD.bazel new file mode 100644 index 00000000..7aa19319 --- /dev/null +++ b/submitqueue/extension/speculation/speculator/standard/BUILD.bazel @@ -0,0 +1,31 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["standard.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator/standard", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/speculation/allocator:go_default_library", + "//submitqueue/extension/speculation/generator:go_default_library", + "//submitqueue/extension/speculation/speculator:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["standard_test.go"], + embed = [":go_default_library"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/speculation/allocator/mock:go_default_library", + "//submitqueue/extension/speculation/allocator/sticky:go_default_library", + "//submitqueue/extension/speculation/generator/bestfirst:go_default_library", + "//submitqueue/extension/speculation/generator/mock:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + "@org_uber_go_zap//zaptest:go_default_library", + ], +) diff --git a/submitqueue/extension/speculation/speculator/standard/standard.go b/submitqueue/extension/speculation/speculator/standard/standard.go new file mode 100644 index 00000000..72e0c05e --- /dev/null +++ b/submitqueue/extension/speculation/speculator/standard/standard.go @@ -0,0 +1,52 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package standard provides the standard speculator.Speculator: it composes a +// Generator and an Allocator so a queue's candidate scoring and its budget +// rationing policy can vary independently without changing the Speculator. The +// Speculator itself decides nothing — it opens the Generator's candidate stream +// and hands it to the Allocator; all of its behavior comes from those two parts. +package standard + +import ( + "context" + + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/speculation/allocator" + "github.com/uber/submitqueue/submitqueue/extension/speculation/generator" + "github.com/uber/submitqueue/submitqueue/extension/speculation/speculator" +) + +// spec is a speculator.Speculator that opens the generator's candidate stream +// and hands it to the allocator to spend the build budget. +type spec struct { + gen generator.Generator + alloc allocator.Allocator +} + +// New returns the standard speculator.Speculator, composing a Generator and an +// Allocator. +func New(gen generator.Generator, alloc allocator.Allocator) speculator.Speculator { + return spec{gen: gen, alloc: alloc} +} + +// Speculate opens the generator over the queue's batches and path sets, then +// lets the allocator spend the budget over the resulting candidate iterator. +func (s spec) Speculate(ctx context.Context, batches []entity.Batch, pathSets []entity.SpeculationPathSet) ([]entity.Speculation, error) { + iter, err := s.gen.Open(ctx, batches, pathSets) + if err != nil { + return nil, err + } + return s.alloc.Allocate(ctx, pathSets, iter) +} diff --git a/submitqueue/extension/speculation/speculator/standard/standard_test.go b/submitqueue/extension/speculation/speculator/standard/standard_test.go new file mode 100644 index 00000000..bb764cc8 --- /dev/null +++ b/submitqueue/extension/speculation/speculator/standard/standard_test.go @@ -0,0 +1,108 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package standard + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "go.uber.org/zap/zaptest" + + "github.com/uber/submitqueue/submitqueue/entity" + allocatormock "github.com/uber/submitqueue/submitqueue/extension/speculation/allocator/mock" + "github.com/uber/submitqueue/submitqueue/extension/speculation/allocator/sticky" + "github.com/uber/submitqueue/submitqueue/extension/speculation/generator/bestfirst" + generatormock "github.com/uber/submitqueue/submitqueue/extension/speculation/generator/mock" +) + +// betFor returns the bet a path carries for a given dependency batch. +func betFor(p entity.SpeculationPath, dep string) entity.DependencyBetType { + for _, b := range p.Bets { + if b.Batch == dep { + return b.Bet + } + } + return entity.BetUnknown +} + +// constScorer is a minimal scorer.Scorer that scores every batch identically. +type constScorer struct{ v float64 } + +func (c constScorer) Score(context.Context, entity.Batch) (float64, error) { return c.v, nil } + +func TestComposed_EndToEnd_NaivePair(t *testing.T) { + batches := []entity.Batch{ + {ID: "q/1", State: entity.BatchStateSpeculating}, + {ID: "q/2", State: entity.BatchStateSpeculating, Dependencies: []string{"q/1"}}, + } + + // bestfirst generator + sticky allocator with a 2-build budget. + spec := New(bestfirst.New(2, constScorer{0.9}), sticky.New(zaptest.NewLogger(t).Sugar(), 2)) + got, err := spec.Speculate(context.Background(), batches, nil) + require.NoError(t, err) + + // Two free budget slots -> two builds. Everything proposed is a build. + require.Len(t, got, 2) + var q2 *entity.Speculation + for i := range got { + assert.Equal(t, entity.PathActionBuild, got[i].Action) + if got[i].Path.Head == "q/2" { + q2 = &got[i] + } + } + // q/2's funded path is the optimistic all-included guess on q/1. + require.NotNil(t, q2) + assert.Equal(t, entity.BetIncluded, betFor(q2.Path, "q/1")) +} + +func TestComposed_WiresGeneratorIntoAllocator(t *testing.T) { + ctrl := gomock.NewController(t) + + batches := []entity.Batch{{ID: "q/1", State: entity.BatchStateSpeculating}} + pathSets := []entity.SpeculationPathSet{{BatchID: "q/1"}} + want := []entity.Speculation{{ + Path: entity.SpeculationPath{Head: "q/1"}, + Action: entity.PathActionBuild, + }} + + iter := generatormock.NewMockPathIterator(ctrl) + gen := generatormock.NewMockGenerator(ctrl) + alloc := allocatormock.NewMockAllocator(ctrl) + + gen.EXPECT().Open(gomock.Any(), batches, pathSets).Return(iter, nil) + alloc.EXPECT().Allocate(gomock.Any(), pathSets, iter).Return(want, nil) + + got, err := New(gen, alloc).Speculate(context.Background(), batches, pathSets) + require.NoError(t, err) + assert.Equal(t, want, got) +} + +func TestComposed_PropagatesGeneratorError(t *testing.T) { + ctrl := gomock.NewController(t) + + errOpen := errors.New("open boom") + gen := generatormock.NewMockGenerator(ctrl) + alloc := allocatormock.NewMockAllocator(ctrl) + + gen.EXPECT().Open(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errOpen) + // Allocate must not be called when Open fails (no alloc.EXPECT()). + + _, err := New(gen, alloc).Speculate(context.Background(), nil, nil) + require.ErrorIs(t, err, errOpen) +}