From 2f0435beb1e735aa58005aa098fa02c6c8783e3a Mon Sep 17 00:00:00 2001 From: manjari Date: Fri, 14 Aug 2026 05:46:46 +0000 Subject: [PATCH 1/5] feat(conflict): Init Tango-backed target-overlap conflict analyzer --- .../extension/conflict/tango/BUILD.bazel | 24 +++ submitqueue/extension/conflict/tango/tango.go | 112 ++++++++++++ .../extension/conflict/tango/tango_test.go | 166 ++++++++++++++++++ 3 files changed, 302 insertions(+) create mode 100644 submitqueue/extension/conflict/tango/BUILD.bazel create mode 100644 submitqueue/extension/conflict/tango/tango.go create mode 100644 submitqueue/extension/conflict/tango/tango_test.go diff --git a/submitqueue/extension/conflict/tango/BUILD.bazel b/submitqueue/extension/conflict/tango/BUILD.bazel new file mode 100644 index 000000000..c768a8edb --- /dev/null +++ b/submitqueue/extension/conflict/tango/BUILD.bazel @@ -0,0 +1,24 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["tango.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/tango", + visibility = ["//visibility:public"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/conflict:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["tango_test.go"], + embed = [":go_default_library"], + deps = [ + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/conflict:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + ], +) diff --git a/submitqueue/extension/conflict/tango/tango.go b/submitqueue/extension/conflict/tango/tango.go new file mode 100644 index 000000000..05c67767a --- /dev/null +++ b/submitqueue/extension/conflict/tango/tango.go @@ -0,0 +1,112 @@ +// Copyright (c) 2026 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 tango provides a conflict.Analyzer that reports a conflict between +// two batches when their changed build targets overlap. The targets a batch +// affects are resolved through an injected TargetResolver, whose production +// implementation calls the Tango service. +package tango + +import ( + "context" + "fmt" + + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/conflict" +) + +// TargetResolver resolves the set of build targets a batch affects. The +// production implementation translates the batch's changes into a Tango +// GetChangedTargets call; tests supply a fake. +type TargetResolver interface { + ChangedTargets(ctx context.Context, batch entity.Batch) ([]string, error) +} + +// New returns a conflict.Analyzer that flags an in-flight batch as conflicting +// when its changed build targets overlap with the candidate batch's, bound to +// the queue named in cfg. +func New(cfg conflict.Config, targets TargetResolver) conflict.Analyzer { + return &analyzer{cfg: cfg, targets: targets} +} + +type analyzer struct { + cfg conflict.Config + targets TargetResolver + // TODO: cache resolved target sets per batch ID so in-flight batches + // compared against successive arrivals pay only one resolution each. Consider + // a TTL for high-traffic queues where trunk moves fast, and a max-size cap. +} + +// Analyze returns one ConflictTypeTargetOverlap Conflict per in-flight batch +// whose changed build targets overlap with batch, preserving the in-flight +// order. A batch that affects no targets conflicts with nothing. +func (a *analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]entity.Conflict, error) { + if len(inFlight) == 0 { + return nil, nil + } + + // TODO: when TargetResolver fails, fall back to a queue-configured + // analyzer (all or none) instead of propagating the error. The queue config + // decides whether a Tango outage over-serializes (all) or maximizes + // parallelism (none). + candidate, err := a.resolve(ctx, batch) + if err != nil { + return nil, fmt.Errorf("failed to resolve targets for batch %s: %w", batch.ID, err) + } + if len(candidate) == 0 { + return nil, nil + } + + var conflicts []entity.Conflict + for _, other := range inFlight { + keys, err := a.resolve(ctx, other) + if err != nil { + return nil, fmt.Errorf("failed to resolve targets for batch %s: %w", other.ID, err) + } + if intersects(candidate, keys) { + conflicts = append(conflicts, entity.Conflict{ + BatchID: other.ID, + Type: entity.ConflictTypeTargetOverlap, + }) + } + } + return conflicts, nil +} + +// resolve returns the set of build targets the batch affects. +func (a *analyzer) resolve(ctx context.Context, batch entity.Batch) (map[string]struct{}, error) { + targets, err := a.targets.ChangedTargets(ctx, batch) + if err != nil { + return nil, err + } + + keys := make(map[string]struct{}, len(targets)) + for _, t := range targets { + keys[t] = struct{}{} + } + return keys, nil +} + +// intersects reports whether the two sets share any element. +func intersects(a, b map[string]struct{}) bool { + if len(b) < len(a) { + a, b = b, a + } + for k := range a { + if _, ok := b[k]; ok { + return true + } + } + return false +} diff --git a/submitqueue/extension/conflict/tango/tango_test.go b/submitqueue/extension/conflict/tango/tango_test.go new file mode 100644 index 000000000..5cbd21e87 --- /dev/null +++ b/submitqueue/extension/conflict/tango/tango_test.go @@ -0,0 +1,166 @@ +// Copyright (c) 2026 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 tango + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/conflict" +) + +// fakeResolver is an in-test TargetResolver that returns pre-configured target +// sets per batch ID. +type fakeResolver struct { + targets map[string][]string + err error +} + +func newFakeResolver() *fakeResolver { + return &fakeResolver{targets: make(map[string][]string)} +} + +func (f *fakeResolver) set(batchID string, targets ...string) *fakeResolver { + f.targets[batchID] = targets + return f +} + +func (f *fakeResolver) failWith(err error) *fakeResolver { + f.err = err + return f +} + +func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]string, error) { + if f.err != nil { + return nil, f.err + } + return f.targets[batch.ID], nil +} + +func cfg() conflict.Config { + return conflict.Config{QueueName: "test-queue"} +} + +func TestAnalyze(t *testing.T) { + tests := []struct { + name string + candidate string + candTargets []string + inFlight []struct { + id string + targets []string + } + wantBatches []string + }{ + { + name: "overlap on a shared target conflicts", + candidate: "cand", + candTargets: []string{"//foo:lib", "//bar:lib"}, + inFlight: []struct { + id string + targets []string + }{ + {id: "x", targets: []string{"//bar:lib", "//baz:lib"}}, + }, + wantBatches: []string{"x"}, + }, + { + name: "disjoint targets do not conflict", + candidate: "cand", + candTargets: []string{"//foo:lib"}, + inFlight: []struct { + id string + targets []string + }{ + {id: "x", targets: []string{"//bar:lib"}}, + }, + wantBatches: nil, + }, + { + name: "only overlapping in-flight batches are reported, in order", + candidate: "cand", + candTargets: []string{"//foo:lib"}, + inFlight: []struct { + id string + targets []string + }{ + {id: "x", targets: []string{"//foo:lib"}}, + {id: "y", targets: []string{"//bar:lib"}}, + {id: "z", targets: []string{"//foo:lib"}}, + }, + wantBatches: []string{"x", "z"}, + }, + { + name: "candidate with no targets conflicts with nothing", + candidate: "cand", + candTargets: nil, + inFlight: []struct { + id string + targets []string + }{ + {id: "x", targets: []string{"//foo:lib"}}, + }, + wantBatches: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resolver := newFakeResolver().set(tt.candidate, tt.candTargets...) + inFlight := make([]entity.Batch, 0, len(tt.inFlight)) + for _, f := range tt.inFlight { + resolver.set(f.id, f.targets...) + inFlight = append(inFlight, entity.Batch{ID: f.id}) + } + + got, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: tt.candidate}, inFlight) + require.NoError(t, err) + + var ids []string + for _, c := range got { + assert.Equal(t, entity.ConflictTypeTargetOverlap, c.Type) + ids = append(ids, c.BatchID) + } + assert.Equal(t, tt.wantBatches, ids) + }) + } +} + +func TestAnalyze_EmptyInFlight(t *testing.T) { + got, err := New(cfg(), newFakeResolver()).Analyze(context.Background(), entity.Batch{ID: "cand"}, nil) + require.NoError(t, err) + assert.Empty(t, got) +} + +func TestAnalyze_ResolverError(t *testing.T) { + sentinel := errors.New("tango unavailable") + + t.Run("candidate resolution fails", func(t *testing.T) { + resolver := newFakeResolver().failWith(sentinel) + _, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) + require.ErrorIs(t, err, sentinel) + }) + + t.Run("in-flight resolution fails", func(t *testing.T) { + resolver := newFakeResolver().set("cand", "//foo:lib").failWith(sentinel) + _, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) + require.ErrorIs(t, err, sentinel) + }) +} From b06bb062668811403c90341bd63d4a0eb5e439e3 Mon Sep 17 00:00:00 2001 From: manjari Date: Mon, 17 Aug 2026 14:51:51 +0000 Subject: [PATCH 2/5] feat(conflict): Move targets resolver and implementation under dependency --- .../conflict/tango/BUILD.bazel | 3 +- .../{ => dependency}/conflict/tango/tango.go | 16 +++------ .../conflict/tango/tango_test.go | 0 .../extension/dependency/resolver/BUILD.bazel | 9 +++++ .../dependency/resolver/targetresolver.go | 33 +++++++++++++++++++ 5 files changed, 49 insertions(+), 12 deletions(-) rename submitqueue/extension/{ => dependency}/conflict/tango/BUILD.bazel (87%) rename submitqueue/extension/{ => dependency}/conflict/tango/tango.go (86%) rename submitqueue/extension/{ => dependency}/conflict/tango/tango_test.go (100%) create mode 100644 submitqueue/extension/dependency/resolver/BUILD.bazel create mode 100644 submitqueue/extension/dependency/resolver/targetresolver.go diff --git a/submitqueue/extension/conflict/tango/BUILD.bazel b/submitqueue/extension/dependency/conflict/tango/BUILD.bazel similarity index 87% rename from submitqueue/extension/conflict/tango/BUILD.bazel rename to submitqueue/extension/dependency/conflict/tango/BUILD.bazel index c768a8edb..9c3e0ded9 100644 --- a/submitqueue/extension/conflict/tango/BUILD.bazel +++ b/submitqueue/extension/dependency/conflict/tango/BUILD.bazel @@ -3,11 +3,12 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = ["tango.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/tango", + importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/tango", visibility = ["//visibility:public"], deps = [ "//submitqueue/entity:go_default_library", "//submitqueue/extension/conflict:go_default_library", + "//submitqueue/extension/dependency/resolver:go_default_library", ], ) diff --git a/submitqueue/extension/conflict/tango/tango.go b/submitqueue/extension/dependency/conflict/tango/tango.go similarity index 86% rename from submitqueue/extension/conflict/tango/tango.go rename to submitqueue/extension/dependency/conflict/tango/tango.go index 05c67767a..2c92c1ffd 100644 --- a/submitqueue/extension/conflict/tango/tango.go +++ b/submitqueue/extension/dependency/conflict/tango/tango.go @@ -14,8 +14,8 @@ // Package tango provides a conflict.Analyzer that reports a conflict between // two batches when their changed build targets overlap. The targets a batch -// affects are resolved through an injected TargetResolver, whose production -// implementation calls the Tango service. +// affects are resolved through an injected resolver.TargetResolver, whose +// production implementation calls the Tango service. package tango import ( @@ -24,25 +24,19 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/conflict" + "github.com/uber/submitqueue/submitqueue/extension/dependency/resolver" ) -// TargetResolver resolves the set of build targets a batch affects. The -// production implementation translates the batch's changes into a Tango -// GetChangedTargets call; tests supply a fake. -type TargetResolver interface { - ChangedTargets(ctx context.Context, batch entity.Batch) ([]string, error) -} - // New returns a conflict.Analyzer that flags an in-flight batch as conflicting // when its changed build targets overlap with the candidate batch's, bound to // the queue named in cfg. -func New(cfg conflict.Config, targets TargetResolver) conflict.Analyzer { +func New(cfg conflict.Config, targets resolver.TargetResolver) conflict.Analyzer { return &analyzer{cfg: cfg, targets: targets} } type analyzer struct { cfg conflict.Config - targets TargetResolver + targets resolver.TargetResolver // TODO: cache resolved target sets per batch ID so in-flight batches // compared against successive arrivals pay only one resolution each. Consider // a TTL for high-traffic queues where trunk moves fast, and a max-size cap. diff --git a/submitqueue/extension/conflict/tango/tango_test.go b/submitqueue/extension/dependency/conflict/tango/tango_test.go similarity index 100% rename from submitqueue/extension/conflict/tango/tango_test.go rename to submitqueue/extension/dependency/conflict/tango/tango_test.go diff --git a/submitqueue/extension/dependency/resolver/BUILD.bazel b/submitqueue/extension/dependency/resolver/BUILD.bazel new file mode 100644 index 000000000..7944b6623 --- /dev/null +++ b/submitqueue/extension/dependency/resolver/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["targetresolver.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/resolver", + visibility = ["//visibility:public"], + deps = ["//submitqueue/entity:go_default_library"], +) diff --git a/submitqueue/extension/dependency/resolver/targetresolver.go b/submitqueue/extension/dependency/resolver/targetresolver.go new file mode 100644 index 000000000..387da245c --- /dev/null +++ b/submitqueue/extension/dependency/resolver/targetresolver.go @@ -0,0 +1,33 @@ +// Copyright (c) 2026 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 resolver defines the TargetResolver interface for resolving the set +// of build targets a batch affects. The interface is deliberately free of +// Tango wire types so that each deployment can provide its own adapter against +// whatever proto import path its monorepo uses — the analyzer sees only batch +// identity in and target names out. +package resolver + +import ( + "context" + + "github.com/uber/submitqueue/submitqueue/entity" +) + +// TargetResolver resolves the set of build targets a batch affects. The +// production implementation translates the batch's changes into a Tango +// GetChangedTargets call; tests supply a fake. +type TargetResolver interface { + ChangedTargets(ctx context.Context, batch entity.Batch) ([]string, error) +} From 484dbcd1477dffa478ed62cf6a1208a01639da63 Mon Sep 17 00:00:00 2001 From: manjari Date: Fri, 21 Aug 2026 18:49:42 +0000 Subject: [PATCH 3/5] feat(conflict): Rename and iterate the interface contract --- .../{tango => targetoverlap}/BUILD.bazel | 7 +++-- .../targetoverlap.go} | 14 +++++----- .../targetoverlap_test.go} | 28 +++++++++++-------- .../dependency/resolver/targetresolver.go | 14 ++++++++-- 4 files changed, 40 insertions(+), 23 deletions(-) rename submitqueue/extension/dependency/conflict/{tango => targetoverlap}/BUILD.bazel (80%) rename submitqueue/extension/dependency/conflict/{tango/tango.go => targetoverlap/targetoverlap.go} (88%) rename submitqueue/extension/dependency/conflict/{tango/tango_test.go => targetoverlap/targetoverlap_test.go} (81%) diff --git a/submitqueue/extension/dependency/conflict/tango/BUILD.bazel b/submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel similarity index 80% rename from submitqueue/extension/dependency/conflict/tango/BUILD.bazel rename to submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel index 9c3e0ded9..7d05516b7 100644 --- a/submitqueue/extension/dependency/conflict/tango/BUILD.bazel +++ b/submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel @@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["tango.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/tango", + srcs = ["targetoverlap.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/targetoverlap", visibility = ["//visibility:public"], deps = [ "//submitqueue/entity:go_default_library", @@ -14,11 +14,12 @@ go_library( go_test( name = "go_default_test", - srcs = ["tango_test.go"], + srcs = ["targetoverlap_test.go"], embed = [":go_default_library"], deps = [ "//submitqueue/entity:go_default_library", "//submitqueue/extension/conflict:go_default_library", + "//submitqueue/extension/dependency/resolver:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", ], diff --git a/submitqueue/extension/dependency/conflict/tango/tango.go b/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go similarity index 88% rename from submitqueue/extension/dependency/conflict/tango/tango.go rename to submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go index 2c92c1ffd..6f77d296b 100644 --- a/submitqueue/extension/dependency/conflict/tango/tango.go +++ b/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package tango provides a conflict.Analyzer that reports a conflict between -// two batches when their changed build targets overlap. The targets a batch -// affects are resolved through an injected resolver.TargetResolver, whose -// production implementation calls the Tango service. -package tango +// Package targetoverlap provides a conflict.Analyzer that reports a conflict +// between two batches when their changed build targets overlap. The targets a +// batch affects are resolved through an injected resolver.TargetResolver, +// keeping the analyzer independent of any particular target-resolution backend. +package targetoverlap import ( "context" @@ -52,7 +52,7 @@ func (a *analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []e // TODO: when TargetResolver fails, fall back to a queue-configured // analyzer (all or none) instead of propagating the error. The queue config - // decides whether a Tango outage over-serializes (all) or maximizes + // decides whether a resolver outage over-serializes (all) or maximizes // parallelism (none). candidate, err := a.resolve(ctx, batch) if err != nil { @@ -87,7 +87,7 @@ func (a *analyzer) resolve(ctx context.Context, batch entity.Batch) (map[string] keys := make(map[string]struct{}, len(targets)) for _, t := range targets { - keys[t] = struct{}{} + keys[t.Name] = struct{}{} } return keys, nil } diff --git a/submitqueue/extension/dependency/conflict/tango/tango_test.go b/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go similarity index 81% rename from submitqueue/extension/dependency/conflict/tango/tango_test.go rename to submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go index 5cbd21e87..66154e713 100644 --- a/submitqueue/extension/dependency/conflict/tango/tango_test.go +++ b/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package tango +package targetoverlap import ( "context" @@ -24,6 +24,7 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/conflict" + "github.com/uber/submitqueue/submitqueue/extension/dependency/resolver" ) // fakeResolver is an in-test TargetResolver that returns pre-configured target @@ -47,11 +48,16 @@ func (f *fakeResolver) failWith(err error) *fakeResolver { return f } -func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]string, error) { +func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]resolver.Target, error) { if f.err != nil { return nil, f.err } - return f.targets[batch.ID], nil + names := f.targets[batch.ID] + targets := make([]resolver.Target, len(names)) + for i, n := range names { + targets[i] = resolver.Target{Name: n} + } + return targets, nil } func cfg() conflict.Config { @@ -123,14 +129,14 @@ func TestAnalyze(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - resolver := newFakeResolver().set(tt.candidate, tt.candTargets...) + r := newFakeResolver().set(tt.candidate, tt.candTargets...) inFlight := make([]entity.Batch, 0, len(tt.inFlight)) for _, f := range tt.inFlight { - resolver.set(f.id, f.targets...) + r.set(f.id, f.targets...) inFlight = append(inFlight, entity.Batch{ID: f.id}) } - got, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: tt.candidate}, inFlight) + got, err := New(cfg(), r).Analyze(context.Background(), entity.Batch{ID: tt.candidate}, inFlight) require.NoError(t, err) var ids []string @@ -150,17 +156,17 @@ func TestAnalyze_EmptyInFlight(t *testing.T) { } func TestAnalyze_ResolverError(t *testing.T) { - sentinel := errors.New("tango unavailable") + sentinel := errors.New("resolver unavailable") t.Run("candidate resolution fails", func(t *testing.T) { - resolver := newFakeResolver().failWith(sentinel) - _, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) + r := newFakeResolver().failWith(sentinel) + _, err := New(cfg(), r).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) require.ErrorIs(t, err, sentinel) }) t.Run("in-flight resolution fails", func(t *testing.T) { - resolver := newFakeResolver().set("cand", "//foo:lib").failWith(sentinel) - _, err := New(cfg(), resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) + r := newFakeResolver().set("cand", "//foo:lib").failWith(sentinel) + _, err := New(cfg(), r).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) require.ErrorIs(t, err, sentinel) }) } diff --git a/submitqueue/extension/dependency/resolver/targetresolver.go b/submitqueue/extension/dependency/resolver/targetresolver.go index 387da245c..8b77c05d0 100644 --- a/submitqueue/extension/dependency/resolver/targetresolver.go +++ b/submitqueue/extension/dependency/resolver/targetresolver.go @@ -16,7 +16,7 @@ // of build targets a batch affects. The interface is deliberately free of // Tango wire types so that each deployment can provide its own adapter against // whatever proto import path its monorepo uses — the analyzer sees only batch -// identity in and target names out. +// identity in and targets out. package resolver import ( @@ -25,9 +25,19 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" ) +// Target is a build target a batch affects. +type Target struct { + // Name identifies the target (e.g. "//service/foo:lib"). + Name string + // Attributes carries backend-specific metadata the analyzer does not + // interpret today. Future consumers (e.g. conflict relaxation) can read + // keys like "distance" or "rule_type" without an interface change. + Attributes map[string]string +} + // TargetResolver resolves the set of build targets a batch affects. The // production implementation translates the batch's changes into a Tango // GetChangedTargets call; tests supply a fake. type TargetResolver interface { - ChangedTargets(ctx context.Context, batch entity.Batch) ([]string, error) + ChangedTargets(ctx context.Context, batch entity.Batch) ([]Target, error) } From 13f58c2870fbd835a2844bf5b3dc374bc6c27ce1 Mon Sep 17 00:00:00 2001 From: manjari Date: Wed, 26 Aug 2026 20:33:10 +0000 Subject: [PATCH 4/5] feat(conflict): Lightweight interface for tango resolver and tango analyzer impl --- .../{targetoverlap => tango}/BUILD.bazel | 8 ++-- .../targetoverlap.go => tango/tango.go} | 42 +++++++++++++----- .../tango_test.go} | 11 +++-- .../extension/dependency/resolver/BUILD.bazel | 9 ---- .../dependency/resolver/targetresolver.go | 43 ------------------- 5 files changed, 40 insertions(+), 73 deletions(-) rename submitqueue/extension/dependency/conflict/{targetoverlap => tango}/BUILD.bazel (72%) rename submitqueue/extension/dependency/conflict/{targetoverlap/targetoverlap.go => tango/tango.go} (65%) rename submitqueue/extension/dependency/conflict/{targetoverlap/targetoverlap_test.go => tango/tango_test.go} (94%) delete mode 100644 submitqueue/extension/dependency/resolver/BUILD.bazel delete mode 100644 submitqueue/extension/dependency/resolver/targetresolver.go diff --git a/submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel b/submitqueue/extension/dependency/conflict/tango/BUILD.bazel similarity index 72% rename from submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel rename to submitqueue/extension/dependency/conflict/tango/BUILD.bazel index 7d05516b7..181b93a8c 100644 --- a/submitqueue/extension/dependency/conflict/targetoverlap/BUILD.bazel +++ b/submitqueue/extension/dependency/conflict/tango/BUILD.bazel @@ -2,24 +2,22 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["targetoverlap.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/targetoverlap", + srcs = ["tango.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/tango", visibility = ["//visibility:public"], deps = [ "//submitqueue/entity:go_default_library", "//submitqueue/extension/conflict:go_default_library", - "//submitqueue/extension/dependency/resolver:go_default_library", ], ) go_test( name = "go_default_test", - srcs = ["targetoverlap_test.go"], + srcs = ["tango_test.go"], embed = [":go_default_library"], deps = [ "//submitqueue/entity:go_default_library", "//submitqueue/extension/conflict:go_default_library", - "//submitqueue/extension/dependency/resolver:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", "@com_github_stretchr_testify//require:go_default_library", ], diff --git a/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go b/submitqueue/extension/dependency/conflict/tango/tango.go similarity index 65% rename from submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go rename to submitqueue/extension/dependency/conflict/tango/tango.go index 6f77d296b..fc71dc4d0 100644 --- a/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap.go +++ b/submitqueue/extension/dependency/conflict/tango/tango.go @@ -12,11 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package targetoverlap provides a conflict.Analyzer that reports a conflict -// between two batches when their changed build targets overlap. The targets a -// batch affects are resolved through an injected resolver.TargetResolver, -// keeping the analyzer independent of any particular target-resolution backend. -package targetoverlap +// Package tango provides a conflict.Analyzer that reports a conflict between +// two batches when their changed build targets overlap. The targets a batch +// affects are resolved through an injected TargetResolver, whose production +// implementation calls the Tango service. The interface is deliberately free +// of Tango wire types so that each deployment can provide its own adapter +// against whatever proto import path its monorepo uses. +package tango import ( "context" @@ -24,19 +26,39 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/conflict" - "github.com/uber/submitqueue/submitqueue/extension/dependency/resolver" ) +// Target is a build target a batch affects, carrying the fields Tango reports +// for each changed target. The analyzer reads only Name for overlap today; +// ChangeType and Distance are available for future consumers such as conflict +// relaxation. +type Target struct { + // Name identifies the target (e.g. "//service/foo:lib"). + Name string + // ChangeType classifies how the target changed: "new", "deleted", or "changed". + ChangeType string + // Distance from the nearest directly changed source file in the reverse + // dependency graph. 0 = directly changed, 1+ = transitive. + Distance int +} + +// TargetResolver resolves the set of build targets a batch affects. The +// production implementation translates the batch's changes into a Tango +// GetChangedTargets call; tests supply a fake. +type TargetResolver interface { + ChangedTargets(ctx context.Context, batch entity.Batch) ([]Target, error) +} + // New returns a conflict.Analyzer that flags an in-flight batch as conflicting // when its changed build targets overlap with the candidate batch's, bound to // the queue named in cfg. -func New(cfg conflict.Config, targets resolver.TargetResolver) conflict.Analyzer { +func New(cfg conflict.Config, targets TargetResolver) conflict.Analyzer { return &analyzer{cfg: cfg, targets: targets} } type analyzer struct { cfg conflict.Config - targets resolver.TargetResolver + targets TargetResolver // TODO: cache resolved target sets per batch ID so in-flight batches // compared against successive arrivals pay only one resolution each. Consider // a TTL for high-traffic queues where trunk moves fast, and a max-size cap. @@ -52,7 +74,7 @@ func (a *analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []e // TODO: when TargetResolver fails, fall back to a queue-configured // analyzer (all or none) instead of propagating the error. The queue config - // decides whether a resolver outage over-serializes (all) or maximizes + // decides whether a Tango outage over-serializes (all) or maximizes // parallelism (none). candidate, err := a.resolve(ctx, batch) if err != nil { @@ -78,7 +100,7 @@ func (a *analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []e return conflicts, nil } -// resolve returns the set of build targets the batch affects. +// resolve returns the set of target names the batch affects. func (a *analyzer) resolve(ctx context.Context, batch entity.Batch) (map[string]struct{}, error) { targets, err := a.targets.ChangedTargets(ctx, batch) if err != nil { diff --git a/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go b/submitqueue/extension/dependency/conflict/tango/tango_test.go similarity index 94% rename from submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go rename to submitqueue/extension/dependency/conflict/tango/tango_test.go index 66154e713..1dd5de38a 100644 --- a/submitqueue/extension/dependency/conflict/targetoverlap/targetoverlap_test.go +++ b/submitqueue/extension/dependency/conflict/tango/tango_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package targetoverlap +package tango import ( "context" @@ -24,7 +24,6 @@ import ( "github.com/uber/submitqueue/submitqueue/entity" "github.com/uber/submitqueue/submitqueue/extension/conflict" - "github.com/uber/submitqueue/submitqueue/extension/dependency/resolver" ) // fakeResolver is an in-test TargetResolver that returns pre-configured target @@ -48,14 +47,14 @@ func (f *fakeResolver) failWith(err error) *fakeResolver { return f } -func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]resolver.Target, error) { +func (f *fakeResolver) ChangedTargets(_ context.Context, batch entity.Batch) ([]Target, error) { if f.err != nil { return nil, f.err } names := f.targets[batch.ID] - targets := make([]resolver.Target, len(names)) + targets := make([]Target, len(names)) for i, n := range names { - targets[i] = resolver.Target{Name: n} + targets[i] = Target{Name: n} } return targets, nil } @@ -156,7 +155,7 @@ func TestAnalyze_EmptyInFlight(t *testing.T) { } func TestAnalyze_ResolverError(t *testing.T) { - sentinel := errors.New("resolver unavailable") + sentinel := errors.New("tango unavailable") t.Run("candidate resolution fails", func(t *testing.T) { r := newFakeResolver().failWith(sentinel) diff --git a/submitqueue/extension/dependency/resolver/BUILD.bazel b/submitqueue/extension/dependency/resolver/BUILD.bazel deleted file mode 100644 index 7944b6623..000000000 --- a/submitqueue/extension/dependency/resolver/BUILD.bazel +++ /dev/null @@ -1,9 +0,0 @@ -load("@rules_go//go:def.bzl", "go_library") - -go_library( - name = "go_default_library", - srcs = ["targetresolver.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/resolver", - visibility = ["//visibility:public"], - deps = ["//submitqueue/entity:go_default_library"], -) diff --git a/submitqueue/extension/dependency/resolver/targetresolver.go b/submitqueue/extension/dependency/resolver/targetresolver.go deleted file mode 100644 index 8b77c05d0..000000000 --- a/submitqueue/extension/dependency/resolver/targetresolver.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (c) 2026 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 resolver defines the TargetResolver interface for resolving the set -// of build targets a batch affects. The interface is deliberately free of -// Tango wire types so that each deployment can provide its own adapter against -// whatever proto import path its monorepo uses — the analyzer sees only batch -// identity in and targets out. -package resolver - -import ( - "context" - - "github.com/uber/submitqueue/submitqueue/entity" -) - -// Target is a build target a batch affects. -type Target struct { - // Name identifies the target (e.g. "//service/foo:lib"). - Name string - // Attributes carries backend-specific metadata the analyzer does not - // interpret today. Future consumers (e.g. conflict relaxation) can read - // keys like "distance" or "rule_type" without an interface change. - Attributes map[string]string -} - -// TargetResolver resolves the set of build targets a batch affects. The -// production implementation translates the batch's changes into a Tango -// GetChangedTargets call; tests supply a fake. -type TargetResolver interface { - ChangedTargets(ctx context.Context, batch entity.Batch) ([]Target, error) -} From 47274a76d909fecbe82dcc203602425bb12d0c42 Mon Sep 17 00:00:00 2001 From: manjari Date: Wed, 26 Aug 2026 21:47:08 +0000 Subject: [PATCH 5/5] feat(conflict): Move tango anazlyer under conflict extension --- .../extension/{dependency => }/conflict/tango/BUILD.bazel | 2 +- submitqueue/extension/{dependency => }/conflict/tango/tango.go | 0 .../extension/{dependency => }/conflict/tango/tango_test.go | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename submitqueue/extension/{dependency => }/conflict/tango/BUILD.bazel (95%) rename submitqueue/extension/{dependency => }/conflict/tango/tango.go (100%) rename submitqueue/extension/{dependency => }/conflict/tango/tango_test.go (100%) diff --git a/submitqueue/extension/dependency/conflict/tango/BUILD.bazel b/submitqueue/extension/conflict/tango/BUILD.bazel similarity index 95% rename from submitqueue/extension/dependency/conflict/tango/BUILD.bazel rename to submitqueue/extension/conflict/tango/BUILD.bazel index 181b93a8c..c768a8edb 100644 --- a/submitqueue/extension/dependency/conflict/tango/BUILD.bazel +++ b/submitqueue/extension/conflict/tango/BUILD.bazel @@ -3,7 +3,7 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = ["tango.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/dependency/conflict/tango", + importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/tango", visibility = ["//visibility:public"], deps = [ "//submitqueue/entity:go_default_library", diff --git a/submitqueue/extension/dependency/conflict/tango/tango.go b/submitqueue/extension/conflict/tango/tango.go similarity index 100% rename from submitqueue/extension/dependency/conflict/tango/tango.go rename to submitqueue/extension/conflict/tango/tango.go diff --git a/submitqueue/extension/dependency/conflict/tango/tango_test.go b/submitqueue/extension/conflict/tango/tango_test.go similarity index 100% rename from submitqueue/extension/dependency/conflict/tango/tango_test.go rename to submitqueue/extension/conflict/tango/tango_test.go