Skip to content

Commit 84c018e

Browse files
sbalabanov-zzclaude
andcommitted
feat(conflict): add Analyzer extension and wire it into the batch controller
Introduces a vendor-agnostic conflict.Analyzer interface that takes the candidate batch and the in-flight batches it might serialize behind, and returns the subset that conflict along with a typed reason. Ships two stub backends (all/ conservative, none/ optimistic) and replaces the batch controller's hard-coded "depend on every active batch" loop with a pluggable Analyze call plus dedupe-by-BatchID. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e004fc4 commit 84c018e

17 files changed

Lines changed: 591 additions & 26 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ local-stovepipe-stop: ## Stop Stovepipe service
274274

275275
mocks: ## Generate mock files using mockgen
276276
@echo "Generating mocks..."
277-
@$(BAZEL) run @rules_go//go -- generate ./extension/storage/... ./extension/changestore/... ./extension/counter/... ./extension/queue/... ./extension/mergechecker/... ./extension/scorer/... ./core/consumer/...
277+
@$(BAZEL) run @rules_go//go -- generate ./extension/storage/... ./extension/changestore/... ./extension/counter/... ./extension/queue/... ./extension/mergechecker/... ./extension/pusher/... ./extension/scorer/... ./extension/conflict/... ./core/consumer/...
278278
@echo "Mocks generated successfully!"
279279

280280
proto: ## Generate protobuf files from .proto definitions

example/server/orchestrator/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ go_library(
1818
"//extension/changeprovider/github",
1919
"//extension/changestore",
2020
"//extension/changestore/mysql",
21+
"//extension/conflict/all",
2122
"//extension/counter",
2223
"//extension/counter/mysql",
2324
"//extension/mergechecker",

example/server/orchestrator/main.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
githubprovider "github.com/uber/submitqueue/extension/changeprovider/github"
3838
"github.com/uber/submitqueue/extension/changestore"
3939
mysqlchangestore "github.com/uber/submitqueue/extension/changestore/mysql"
40+
"github.com/uber/submitqueue/extension/conflict/all"
4041
"github.com/uber/submitqueue/extension/counter"
4142
mysqlcounter "github.com/uber/submitqueue/extension/counter/mysql"
4243
"github.com/uber/submitqueue/extension/mergechecker"
@@ -436,6 +437,9 @@ func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope t
436437
registry,
437438
cnt,
438439
store,
440+
// TODO: replace with a real conflict analyzer (e.g. one backed by
441+
// Tango target analysis). The "all" stub serializes the queue.
442+
all.New(),
439443
consumer.TopicKeyBatch,
440444
"orchestrator-batch",
441445
)

extension/conflict/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "conflict",
5+
srcs = ["conflict.go"],
6+
importpath = "github.com/uber/submitqueue/extension/conflict",
7+
visibility = ["//visibility:public"],
8+
deps = ["//entity"],
9+
)

extension/conflict/README.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Conflict
2+
3+
Vendor-agnostic interface for detecting conflicts between a candidate batch
4+
and the batches already in flight.
5+
6+
## Interface
7+
8+
`Analyzer` exposes a single `Analyze` method that takes the candidate batch
9+
and the list of in-flight batches it might conflict with. It returns the
10+
subset of in-flight batches that conflict with the candidate, each paired
11+
with a `ConflictType` describing the kind of conflict. An empty result means
12+
the candidate is free to advance independently.
13+
14+
Callers are responsible for filtering out the candidate itself and any
15+
terminal batches from the in-flight list before invoking the analyzer. The
16+
analyzer itself stays free of lifecycle knowledge. A non-nil error reports
17+
an infrastructure failure of the analysis and should be treated as
18+
retryable by the caller.
19+
20+
The analyzer is intentionally pure with respect to batch state: it does not
21+
mutate inputs, does not read storage, and may be called concurrently. Real
22+
implementations are expected to resolve the batch contents (e.g. changed
23+
build targets, modified files) via whichever upstream system they depend
24+
on, and to return as much classification detail as that system supports.
25+
26+
## Implementations
27+
28+
- [`all/`](all/) — pessimistic stub: reports every in-flight batch as a
29+
`ConflictTypeConservative` conflict. Useful as a worst-case baseline and
30+
for wiring tests where speculation must serialize.
31+
- [`none/`](none/) — optimistic stub: reports no conflicts. Useful as a
32+
best-case baseline and for wiring tests where speculation should run all
33+
batches in parallel.
34+
35+
## Adding a new backend
36+
37+
1. Create `extension/conflict/{backend}/` with an `Analyzer` implementation.
38+
2. Resolve each `entity.Batch` into whatever signal the backend needs
39+
(e.g. changed build targets, files touched, dependency graphs).
40+
3. Emit one `Conflict` per (in-flight batch, detected conflict type). Pick
41+
the most specific `ConflictType` your backend can determine; use
42+
`ConflictTypeConservative` only when the backend cannot prove the absence
43+
of a conflict and falls back to a pessimistic default. Introduce a new
44+
`ConflictType` constant when you can classify the conflict more precisely.
45+
4. Return a plain error for transient infrastructure failures so callers
46+
can classify and retry.

extension/conflict/all/BUILD.bazel

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "all",
5+
srcs = ["all.go"],
6+
importpath = "github.com/uber/submitqueue/extension/conflict/all",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//entity",
10+
"//extension/conflict",
11+
],
12+
)
13+
14+
go_test(
15+
name = "all_test",
16+
srcs = ["all_test.go"],
17+
embed = [":all"],
18+
deps = [
19+
"//entity",
20+
"//extension/conflict",
21+
"@com_github_stretchr_testify//assert",
22+
"@com_github_stretchr_testify//require",
23+
],
24+
)

extension/conflict/all/all.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Package all provides a conflict.Analyzer that pessimistically reports a
16+
// conflict against every in-flight batch. It is intended as a stub for
17+
// wiring tests and as a worst-case baseline for speculation behavior.
18+
package all
19+
20+
import (
21+
"context"
22+
23+
"github.com/uber/submitqueue/entity"
24+
"github.com/uber/submitqueue/extension/conflict"
25+
)
26+
27+
// analyzer is a conflict.Analyzer that reports every in-flight batch as a
28+
// conflict, classified as ConflictTypeConservative.
29+
type analyzer struct{}
30+
31+
// New returns a conflict.Analyzer that reports a conflict against every
32+
// in-flight batch.
33+
func New() conflict.Analyzer {
34+
return analyzer{}
35+
}
36+
37+
// Analyze returns one ConflictTypeConservative Conflict per in-flight batch,
38+
// preserving the input order. Returns an empty slice when inFlight is empty.
39+
func (analyzer) Analyze(_ context.Context, _ entity.Batch, inFlight []entity.Batch) ([]conflict.Conflict, error) {
40+
if len(inFlight) == 0 {
41+
return nil, nil
42+
}
43+
conflicts := make([]conflict.Conflict, len(inFlight))
44+
for i, b := range inFlight {
45+
conflicts[i] = conflict.Conflict{
46+
BatchID: b.ID,
47+
Type: conflict.ConflictTypeConservative,
48+
}
49+
}
50+
return conflicts, nil
51+
}

extension/conflict/all/all_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package all
16+
17+
import (
18+
"context"
19+
"testing"
20+
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
"github.com/uber/submitqueue/entity"
24+
"github.com/uber/submitqueue/extension/conflict"
25+
)
26+
27+
func TestAnalyze(t *testing.T) {
28+
batch := entity.Batch{ID: "queueA/batch/10"}
29+
30+
tests := []struct {
31+
name string
32+
inFlight []entity.Batch
33+
want []conflict.Conflict
34+
}{
35+
{
36+
name: "no in-flight batches yields no conflicts",
37+
inFlight: nil,
38+
want: nil,
39+
},
40+
{
41+
name: "empty in-flight slice yields no conflicts",
42+
inFlight: []entity.Batch{},
43+
want: nil,
44+
},
45+
{
46+
name: "every in-flight batch is reported in input order",
47+
inFlight: []entity.Batch{
48+
{ID: "queueA/batch/1"},
49+
{ID: "queueA/batch/2"},
50+
{ID: "queueA/batch/3"},
51+
},
52+
want: []conflict.Conflict{
53+
{BatchID: "queueA/batch/1", Type: conflict.ConflictTypeConservative},
54+
{BatchID: "queueA/batch/2", Type: conflict.ConflictTypeConservative},
55+
{BatchID: "queueA/batch/3", Type: conflict.ConflictTypeConservative},
56+
},
57+
},
58+
}
59+
60+
a := New()
61+
for _, tt := range tests {
62+
t.Run(tt.name, func(t *testing.T) {
63+
got, err := a.Analyze(context.Background(), batch, tt.inFlight)
64+
require.NoError(t, err)
65+
assert.Equal(t, tt.want, got)
66+
})
67+
}
68+
}

extension/conflict/conflict.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package conflict
16+
17+
//go:generate mockgen -source=conflict.go -destination=mock/conflict_mock.go -package=mock
18+
19+
import (
20+
"context"
21+
22+
"github.com/uber/submitqueue/entity"
23+
)
24+
25+
// ConflictType classifies why two batches are considered to conflict.
26+
// New values may be added as more sophisticated analyzers are introduced.
27+
type ConflictType string
28+
29+
const (
30+
// ConflictTypeUnknown is the unreachable zero value, set by default when
31+
// the structure is initialized. It should never be seen in the system.
32+
ConflictTypeUnknown ConflictType = ""
33+
// ConflictTypeConservative means the analyzer treated the batches as
34+
// conflicting because it could not prove otherwise, without identifying a
35+
// specific reason. Used by conservative analyzers that serialize
36+
// everything by default.
37+
ConflictTypeConservative ConflictType = "conservative"
38+
// ConflictTypeTargetOverlap means the two batches modify one or more of
39+
// the same build targets and may therefore interfere with each other.
40+
ConflictTypeTargetOverlap ConflictType = "target_overlap"
41+
)
42+
43+
// Conflict reports a single conflict between the analyzed batch and one of
44+
// the in-flight batches.
45+
type Conflict struct {
46+
// BatchID is the ID of the in-flight batch that conflicts with the
47+
// analyzed batch.
48+
BatchID string
49+
// Type classifies the conflict. A single (analyzed, in-flight) pair may
50+
// be reported with multiple Conflict entries when different conflict
51+
// types apply.
52+
Type ConflictType
53+
}
54+
55+
// Analyzer detects conflicts between a candidate batch and the batches
56+
// already in flight, so the speculation layer can decide which batches can
57+
// safely advance in parallel.
58+
type Analyzer interface {
59+
// Analyze returns the subset of inFlight batches that conflict with
60+
// batch, each paired with the type of conflict detected. An empty
61+
// result means batch does not conflict with any in-flight batch.
62+
//
63+
// Callers should not include batch itself in inFlight; terminal batches
64+
// should be filtered out before calling. A non-nil error indicates the
65+
// analysis itself failed (infrastructure issue) and should be treated as
66+
// retryable by the caller.
67+
Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]Conflict, error)
68+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "mock",
5+
srcs = ["conflict_mock.go"],
6+
importpath = "github.com/uber/submitqueue/extension/conflict/mock",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//entity",
10+
"//extension/conflict",
11+
"@org_uber_go_mock//gomock",
12+
],
13+
)

0 commit comments

Comments
 (0)