Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ local-stovepipe-stop: ## Stop Stovepipe service

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

proto: ## Generate protobuf files from .proto definitions
Expand Down
1 change: 1 addition & 0 deletions example/server/orchestrator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ go_library(
"//extension/changeprovider/github",
"//extension/changestore",
"//extension/changestore/mysql",
"//extension/conflict/all",
"//extension/counter",
"//extension/counter/mysql",
"//extension/mergechecker",
Expand Down
4 changes: 4 additions & 0 deletions example/server/orchestrator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
githubprovider "github.com/uber/submitqueue/extension/changeprovider/github"
"github.com/uber/submitqueue/extension/changestore"
mysqlchangestore "github.com/uber/submitqueue/extension/changestore/mysql"
"github.com/uber/submitqueue/extension/conflict/all"
"github.com/uber/submitqueue/extension/counter"
mysqlcounter "github.com/uber/submitqueue/extension/counter/mysql"
"github.com/uber/submitqueue/extension/mergechecker"
Expand Down Expand Up @@ -436,6 +437,9 @@ func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope t
registry,
cnt,
store,
// TODO: replace with a real conflict analyzer (e.g. one backed by
// Tango target analysis). The "all" stub serializes the queue.
all.New(),
consumer.TopicKeyBatch,
"orchestrator-batch",
)
Expand Down
9 changes: 9 additions & 0 deletions extension/conflict/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("@rules_go//go:def.bzl", "go_library")

go_library(
name = "conflict",
srcs = ["conflict.go"],
importpath = "github.com/uber/submitqueue/extension/conflict",
visibility = ["//visibility:public"],
deps = ["//entity"],
)
46 changes: 46 additions & 0 deletions extension/conflict/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Conflict

Vendor-agnostic interface for detecting conflicts between a candidate batch
and the batches already in flight.

## Interface

`Analyzer` exposes a single `Analyze` method that takes the candidate batch
and the list of in-flight batches it might conflict with. It returns the
subset of in-flight batches that conflict with the candidate, each paired
with a `ConflictType` describing the kind of conflict. An empty result means
the candidate is free to advance independently.

Callers are responsible for filtering out the candidate itself and any
terminal batches from the in-flight list before invoking the analyzer. The
analyzer itself stays free of lifecycle knowledge. A non-nil error reports
an infrastructure failure of the analysis and should be treated as
retryable by the caller.

The analyzer is intentionally pure with respect to batch state: it does not
mutate inputs, does not read storage, and may be called concurrently. Real
implementations are expected to resolve the batch contents (e.g. changed
build targets, modified files) via whichever upstream system they depend
on, and to return as much classification detail as that system supports.

## Implementations

- [`all/`](all/) — pessimistic stub: reports every in-flight batch as a
`ConflictTypeConservative` conflict. Useful as a worst-case baseline and
for wiring tests where speculation must serialize.
- [`none/`](none/) — optimistic stub: reports no conflicts. Useful as a
best-case baseline and for wiring tests where speculation should run all
batches in parallel.

## Adding a new backend

1. Create `extension/conflict/{backend}/` with an `Analyzer` implementation.
2. Resolve each `entity.Batch` into whatever signal the backend needs
(e.g. changed build targets, files touched, dependency graphs).
3. Emit one `Conflict` per (in-flight batch, detected conflict type). Pick
the most specific `ConflictType` your backend can determine; use
`ConflictTypeConservative` only when the backend cannot prove the absence
of a conflict and falls back to a pessimistic default. Introduce a new
`ConflictType` constant when you can classify the conflict more precisely.
4. Return a plain error for transient infrastructure failures so callers
can classify and retry.
24 changes: 24 additions & 0 deletions extension/conflict/all/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
load("@rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "all",
srcs = ["all.go"],
importpath = "github.com/uber/submitqueue/extension/conflict/all",
visibility = ["//visibility:public"],
deps = [
"//entity",
"//extension/conflict",
],
)

go_test(
name = "all_test",
srcs = ["all_test.go"],
embed = [":all"],
deps = [
"//entity",
"//extension/conflict",
"@com_github_stretchr_testify//assert",
"@com_github_stretchr_testify//require",
],
)
51 changes: 51 additions & 0 deletions extension/conflict/all/all.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// 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 all provides a conflict.Analyzer that pessimistically reports a
// conflict against every in-flight batch. It is intended as a stub for
// wiring tests and as a worst-case baseline for speculation behavior.
package all

import (
"context"

"github.com/uber/submitqueue/entity"
"github.com/uber/submitqueue/extension/conflict"
)

// analyzer is a conflict.Analyzer that reports every in-flight batch as a
// conflict, classified as ConflictTypeConservative.
type analyzer struct{}

// New returns a conflict.Analyzer that reports a conflict against every
// in-flight batch.
func New() conflict.Analyzer {
return analyzer{}
}

// Analyze returns one ConflictTypeConservative Conflict per in-flight batch,
// preserving the input order. Returns an empty slice when inFlight is empty.
func (analyzer) Analyze(_ context.Context, _ entity.Batch, inFlight []entity.Batch) ([]conflict.Conflict, error) {
if len(inFlight) == 0 {
return nil, nil
}
conflicts := make([]conflict.Conflict, len(inFlight))
for i, b := range inFlight {
conflicts[i] = conflict.Conflict{
BatchID: b.ID,
Type: conflict.ConflictTypeConservative,
}
}
return conflicts, nil
}
68 changes: 68 additions & 0 deletions extension/conflict/all/all_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// 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 all

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber/submitqueue/entity"
"github.com/uber/submitqueue/extension/conflict"
)

func TestAnalyze(t *testing.T) {
batch := entity.Batch{ID: "queueA/batch/10"}

tests := []struct {
name string
inFlight []entity.Batch
want []conflict.Conflict
}{
{
name: "no in-flight batches yields no conflicts",
inFlight: nil,
want: nil,
},
{
name: "empty in-flight slice yields no conflicts",
inFlight: []entity.Batch{},
want: nil,
},
{
name: "every in-flight batch is reported in input order",
inFlight: []entity.Batch{
{ID: "queueA/batch/1"},
{ID: "queueA/batch/2"},
{ID: "queueA/batch/3"},
},
want: []conflict.Conflict{
{BatchID: "queueA/batch/1", Type: conflict.ConflictTypeConservative},
{BatchID: "queueA/batch/2", Type: conflict.ConflictTypeConservative},
{BatchID: "queueA/batch/3", Type: conflict.ConflictTypeConservative},
},
},
}

a := New()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := a.Analyze(context.Background(), batch, tt.inFlight)
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}
68 changes: 68 additions & 0 deletions extension/conflict/conflict.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// 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 conflict

//go:generate mockgen -source=conflict.go -destination=mock/conflict_mock.go -package=mock

import (
"context"

"github.com/uber/submitqueue/entity"
)

// ConflictType classifies why two batches are considered to conflict.
// New values may be added as more sophisticated analyzers are introduced.
type ConflictType string

const (
// ConflictTypeUnknown is the unreachable zero value, set by default when
// the structure is initialized. It should never be seen in the system.
ConflictTypeUnknown ConflictType = ""
// ConflictTypeConservative means the analyzer treated the batches as
// conflicting because it could not prove otherwise, without identifying a
// specific reason. Used by conservative analyzers that serialize
// everything by default.
ConflictTypeConservative ConflictType = "conservative"
// ConflictTypeTargetOverlap means the two batches modify one or more of
// the same build targets and may therefore interfere with each other.
ConflictTypeTargetOverlap ConflictType = "target_overlap"
)

// Conflict reports a single conflict between the analyzed batch and one of
// the in-flight batches.
type Conflict struct {
// BatchID is the ID of the in-flight batch that conflicts with the
// analyzed batch.
BatchID string
// Type classifies the conflict. A single (analyzed, in-flight) pair may
// be reported with multiple Conflict entries when different conflict
// types apply.
Type ConflictType
}

// Analyzer detects conflicts between a candidate batch and the batches
// already in flight, so the speculation layer can decide which batches can
// safely advance in parallel.
type Analyzer interface {
// Analyze returns the subset of inFlight batches that conflict with
// batch, each paired with the type of conflict detected. An empty
// result means batch does not conflict with any in-flight batch.
//
// Callers should not include batch itself in inFlight; terminal batches
// should be filtered out before calling. A non-nil error indicates the
// analysis itself failed (infrastructure issue) and should be treated as
// retryable by the caller.
Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]Conflict, error)
}
13 changes: 13 additions & 0 deletions extension/conflict/mock/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
load("@rules_go//go:def.bzl", "go_library")

go_library(
name = "mock",
srcs = ["conflict_mock.go"],
importpath = "github.com/uber/submitqueue/extension/conflict/mock",
visibility = ["//visibility:public"],
deps = [
"//entity",
"//extension/conflict",
"@org_uber_go_mock//gomock",
],
)
58 changes: 58 additions & 0 deletions extension/conflict/mock/conflict_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading