From 0bdff82b21c32b33b38b562a5ebcaafa8119d241 Mon Sep 17 00:00:00 2001 From: manjari Date: Fri, 16 Oct 2026 20:10:39 +0000 Subject: [PATCH] feat(conflict): Add the Tango conflict analyzer --- submitqueue/extension/conflict/README.md | 1 + .../extension/conflict/tango/BUILD.bazel | 11 + .../extension/conflict/tango/README.md | 19 ++ .../extension/conflict/tango/analyzer.go | 155 ++++++++++ .../extension/conflict/tango/analyzer_test.go | 275 ++++++++++++++++++ submitqueue/extension/conflict/tango/cache.go | 20 +- .../extension/conflict/tango/cache_test.go | 20 +- 7 files changed, 482 insertions(+), 19 deletions(-) create mode 100644 submitqueue/extension/conflict/tango/README.md create mode 100644 submitqueue/extension/conflict/tango/analyzer.go create mode 100644 submitqueue/extension/conflict/tango/analyzer_test.go diff --git a/submitqueue/extension/conflict/README.md b/submitqueue/extension/conflict/README.md index feab1f163..e9b8b3096 100644 --- a/submitqueue/extension/conflict/README.md +++ b/submitqueue/extension/conflict/README.md @@ -16,6 +16,7 @@ The analyzer does not mutate batch inputs and may be called concurrently. Implem - [`fake/`](fake/) — wraps another analyzer and optionally injects configured failures for tests and example wiring. - [`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. - [`pathoverlap/`](pathoverlap/) — resolves changed files and reports overlap conflicts by whole file or parent directory. +- [`tango/`](tango/) — compares the build targets each batch affects, as reported by a Tango build graph, and reports target-overlap conflicts. ## Adding a new backend diff --git a/submitqueue/extension/conflict/tango/BUILD.bazel b/submitqueue/extension/conflict/tango/BUILD.bazel index ae13cbc1a..7b5f51152 100644 --- a/submitqueue/extension/conflict/tango/BUILD.bazel +++ b/submitqueue/extension/conflict/tango/BUILD.bazel @@ -3,6 +3,7 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ + "analyzer.go", "cache.go", "intersect.go", "mock_request.go", @@ -17,6 +18,10 @@ go_library( visibility = ["//visibility:public"], deps = [ "//platform/base/change:go_default_library", + "//submitqueue/core/changeset:go_default_library", + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/conflict:go_default_library", + "//submitqueue/extension/storage:go_default_library", "@org_uber_go_mock//gomock:go_default_library", ], ) @@ -24,6 +29,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "analyzer_test.go", "cache_test.go", "intersect_test.go", "request_test.go", @@ -32,7 +38,12 @@ go_test( embed = [":go_default_library"], deps = [ "//platform/base/change:go_default_library", + "//submitqueue/core/changeset/mock:go_default_library", + "//submitqueue/entity:go_default_library", + "//submitqueue/extension/conflict:go_default_library", + "//submitqueue/extension/storage: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", ], ) diff --git a/submitqueue/extension/conflict/tango/README.md b/submitqueue/extension/conflict/tango/README.md new file mode 100644 index 000000000..f7ff591b4 --- /dev/null +++ b/submitqueue/extension/conflict/tango/README.md @@ -0,0 +1,19 @@ +# Tango conflict analyzer + +A `conflict.Analyzer` that decides whether two batches conflict by comparing the build targets each one affects, as reported by a Tango build graph. Two batches conflict when their target sets share an ID, which catches a change that touches code another batch's targets depend on, even when the changed files differ. It reports `ConflictTypeTargetOverlap`. + +## Signatures + +A batch's signature is the sorted set of its affected targets, each as a 64-bit ID (the first 8 bytes of the SHA-256 of the target label) with its distance from the nearest directly changed source. It is created once, when the batch is first analyzed, and never recomputed: asking Tango again later would use a newer base and could give a different answer. A signature is durable in the signature store, and a byte-bounded in-memory cache in front of it can be dropped at any time at the cost of store reads. + +## Behaviour + +- Only the batch being analyzed is ever sent to Tango, and only when it has no recorded signature. Redelivery finds the record the earlier attempt created and does not call Tango again. +- The new batch is signed even when nothing is in flight, so later batches find its record. +- An in-flight batch is read from the cache, then the store, and never from Tango. A missing record or a store error fails the analysis (`ErrNoSignature` for the missing record), because a signature computed later could differ from the one the batch was admitted with. +- The analyzer fails closed on any error. +- A queue must have no in-flight batches when it is switched to this analyzer, because batches admitted earlier have no signature. + +## Injected pieces + +The analyzer owns no connection. A deployment supplies the Tango client adapter, which collects the response and resolves each target's label, and a function that returns the commit SHA of the target branch tip a new batch is analyzed against. The changes of a batch come from the changeset resolver, the signature store is bound to one queue, and one signature cache can be shared by every queue's analyzer. The MySQL implementations of the signature and dictionary stores are in `mysql/`. diff --git a/submitqueue/extension/conflict/tango/analyzer.go b/submitqueue/extension/conflict/tango/analyzer.go new file mode 100644 index 000000000..83c1fdfdc --- /dev/null +++ b/submitqueue/extension/conflict/tango/analyzer.go @@ -0,0 +1,155 @@ +// 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" + "fmt" + "time" + + "github.com/uber/submitqueue/submitqueue/core/changeset" + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/conflict" + "github.com/uber/submitqueue/submitqueue/extension/storage" +) + +// ErrNoSignature means an in-flight batch has no recorded signature. Tango is +// never asked for it: a signature computed later could differ from the one the +// batch was admitted with. +var ErrNoSignature = errors.New("no target signature recorded for in-flight batch") + +// Params are the dependencies of one queue's Analyzer. +type Params struct { + // Signatures persists signatures. It must be bound to the queue the + // analyzer serves. + Signatures SignatureStore + + // Cache holds recently used signatures and may be shared across queues. + Cache *SignatureCache + + // Client returns the targets a set of changes affects. + Client ChangedTargetsClient + + // BaseRevision returns the full commit SHA of the target branch tip that a + // new batch of the named queue is analyzed against. + BaseRevision func(ctx context.Context, queue string) (string, error) + + // Changes resolves a batch into the changes it contains. + Changes changeset.Resolver + + // Now returns the current time. It defaults to time.Now. + Now func() time.Time +} + +type analyzer struct { + Params +} + +// New returns a conflict.Analyzer that reports an in-flight batch as a +// ConflictTypeTargetOverlap conflict when its signature shares a target ID with +// the analyzed batch's. It fails closed: any signature it cannot obtain fails +// the analysis. +func New(params Params) conflict.Analyzer { + if params.Now == nil { + params.Now = time.Now + } + return &analyzer{Params: params} +} + +// Analyze signs batch, which is the only step that can call Tango, and compares +// it with the recorded signature of each in-flight batch. The new batch is +// signed even when nothing is in flight, so later batches find its record. +func (a *analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]entity.Conflict, error) { + mine, err := a.signNewBatch(ctx, batch) + if err != nil { + return nil, fmt.Errorf("failed to sign batch %s: %w", batch.ID, err) + } + + // TODO(tango): read the in-flight signatures concurrently with a bound. + var conflicts []entity.Conflict + for _, other := range inFlight { + theirs, found, err := a.recordedSignature(ctx, other.Queue, other.ID) + if err != nil { + return nil, fmt.Errorf("failed to read the signature of in-flight batch %s: %w", other.ID, err) + } + if !found { + return nil, fmt.Errorf("in-flight batch %s: %w", other.ID, ErrNoSignature) + } + if sharesTarget(mine.Targets, theirs.Targets) { + conflicts = append(conflicts, entity.Conflict{BatchID: other.ID, Type: entity.ConflictTypeTargetOverlap}) + } + } + return conflicts, nil +} + +func (a *analyzer) recordedSignature(ctx context.Context, queue, batchID string) (TargetSignature, bool, error) { + if signature, found := a.Cache.get(queue, batchID); found { + return signature, true, nil + } + signature, found, err := a.Signatures.Get(ctx, batchID) + if err != nil || !found { + return TargetSignature{}, false, err + } + a.Cache.put(signature) + return signature, true, nil +} + +// signNewBatch returns the batch's own record if an earlier attempt created +// one; otherwise it creates the signature first, so racing attempts agree. +func (a *analyzer) signNewBatch(ctx context.Context, batch entity.Batch) (TargetSignature, error) { + signature, found, err := a.recordedSignature(ctx, batch.Queue, batch.ID) + if err != nil || found { + return signature, err + } + + changes, err := a.Changes.ChangesForBatch(ctx, batch) + if err != nil { + return TargetSignature{}, fmt.Errorf("failed to resolve the changes: %w", err) + } + baseRevision, err := a.BaseRevision(ctx, batch.Queue) + if err != nil { + return TargetSignature{}, fmt.Errorf("failed to resolve the base revision of queue %s: %w", batch.Queue, err) + } + affected, err := a.Client.ChangedTargets(ctx, changedTargetsRequestFor(baseRevision, changes)) + if err != nil { + return TargetSignature{}, fmt.Errorf("failed to get the changed targets: %w", err) + } + + // TODO(tango): record the labels in the target dictionary before creating the signature. + targets, _ := toSignatureTargets(affected) + signature = TargetSignature{ + Queue: batch.Queue, + BatchID: batch.ID, + BaseRevision: baseRevision, + Targets: targets, + CreatedAt: a.Now().UnixMilli(), + } + + if err := a.Signatures.Create(ctx, signature); err != nil { + if !errors.Is(err, storage.ErrAlreadyExists) { + return TargetSignature{}, fmt.Errorf("failed to create the signature: %w", err) + } + signature, found, err = a.Signatures.Get(ctx, batch.ID) + if err != nil { + return TargetSignature{}, fmt.Errorf("failed to read the existing signature: %w", err) + } + if !found { + return TargetSignature{}, fmt.Errorf("signature reported as existing but not found") + } + } + a.Cache.put(signature) + return signature, nil +} diff --git a/submitqueue/extension/conflict/tango/analyzer_test.go b/submitqueue/extension/conflict/tango/analyzer_test.go new file mode 100644 index 000000000..9a87c0677 --- /dev/null +++ b/submitqueue/extension/conflict/tango/analyzer_test.go @@ -0,0 +1,275 @@ +// 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" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/uber/submitqueue/platform/base/change" + changesetmock "github.com/uber/submitqueue/submitqueue/core/changeset/mock" + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/conflict" + "github.com/uber/submitqueue/submitqueue/extension/storage" +) + +const analyzerTestQueue = "monorepo" + +var ( + analyzerTestNow = time.UnixMilli(1700000000000) + analyzerTestBaseRevision = "9f2c1d0e3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d" + analyzerTestNew = entity.Batch{ID: "10", Queue: analyzerTestQueue} + analyzerTestFailure = errors.New("dependency down") +) + +type analyzerHarness struct { + signatures *MockSignatureStore + client *MockChangedTargetsClient + baseErr error + changes *changesetmock.MockResolver + analyzer conflict.Analyzer +} + +func newAnalyzerHarness(t *testing.T) *analyzerHarness { + t.Helper() + ctrl := gomock.NewController(t) + h := &analyzerHarness{ + signatures: NewMockSignatureStore(ctrl), + client: NewMockChangedTargetsClient(ctrl), + changes: changesetmock.NewMockResolver(ctrl), + } + h.analyzer = New(Params{ + Signatures: h.signatures, + Cache: NewSignatureCache(1 << 20), + Client: h.client, + BaseRevision: func(context.Context, string) (string, error) { return analyzerTestBaseRevision, h.baseErr }, + Changes: h.changes, + Now: func() time.Time { return analyzerTestNow }, + }) + return h +} + +var analyzerTestAffected = []LabelDistance{{Label: "//app:lib", Distance: 0}, {Label: "//app:bin", Distance: 1}} + +func analyzerTestNewSignature() TargetSignature { + targets, _ := toSignatureTargets(analyzerTestAffected) + return TargetSignature{ + Queue: analyzerTestQueue, + BatchID: analyzerTestNew.ID, + BaseRevision: analyzerTestBaseRevision, + Targets: targets, + CreatedAt: analyzerTestNow.UnixMilli(), + } +} + +func analyzerTestSignature(batchID string, ids ...uint64) TargetSignature { + targets := make([]SignatureTarget, len(ids)) + for i, id := range ids { + targets[i] = SignatureTarget{ID: id} + } + return TargetSignature{Queue: analyzerTestQueue, BatchID: batchID, Targets: targets} +} + +func analyzerTestBatch(id string) entity.Batch { + return entity.Batch{ID: id, Queue: analyzerTestQueue} +} + +func (h *analyzerHarness) expectNewBatchNotRecorded() { + h.signatures.EXPECT().Get(gomock.Any(), analyzerTestNew.ID).Return(TargetSignature{}, false, nil) +} + +func (h *analyzerHarness) expectSigningNewBatch(createErr error) { + h.expectNewBatchNotRecorded() + h.changes.EXPECT().ChangesForBatch(gomock.Any(), analyzerTestNew).Return([]change.Change{{URIs: []string{"u1", "u2"}}}, nil) + h.client.EXPECT().ChangedTargets(gomock.Any(), ChangedTargetsRequest{ + BaseRevision: analyzerTestBaseRevision, + ChangeURIs: []string{"u1", "u2"}, + }).Return(analyzerTestAffected, nil) + h.signatures.EXPECT().Create(gomock.Any(), analyzerTestNewSignature()).Return(createErr) +} + +func TestAnalyzer_Analyze(t *testing.T) { + mine := analyzerTestNewSignature().Targets + shared := mine[0].ID + + tests := []struct { + name string + inFlight []entity.Batch + setup func(h *analyzerHarness) + wantConflicts []entity.Conflict + wantErr error + }{ + { + name: "signs the new batch even when nothing is in flight", + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(nil) + }, + }, + { + name: "reports overlapping in-flight batches in in-flight order", + inFlight: []entity.Batch{analyzerTestBatch("11"), analyzerTestBatch("12"), analyzerTestBatch("13")}, + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(analyzerTestSignature("11", 1, shared), true, nil) + h.signatures.EXPECT().Get(gomock.Any(), "12").Return(analyzerTestSignature("12", 1, 2), true, nil) + h.signatures.EXPECT().Get(gomock.Any(), "13").Return(analyzerTestSignature("13", shared), true, nil) + }, + wantConflicts: []entity.Conflict{ + {BatchID: "11", Type: entity.ConflictTypeTargetOverlap}, + {BatchID: "13", Type: entity.ConflictTypeTargetOverlap}, + }, + }, + { + name: "reports nothing when no in-flight batch overlaps", + inFlight: []entity.Batch{analyzerTestBatch("11")}, + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(analyzerTestSignature("11", 1, 2), true, nil) + }, + }, + { + name: "uses its own record without calling Tango", + inFlight: []entity.Batch{analyzerTestBatch("11")}, + setup: func(h *analyzerHarness) { + h.signatures.EXPECT().Get(gomock.Any(), analyzerTestNew.ID).Return(analyzerTestNewSignature(), true, nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(analyzerTestSignature("11", shared), true, nil) + }, + wantConflicts: []entity.Conflict{{BatchID: "11", Type: entity.ConflictTypeTargetOverlap}}, + }, + { + name: "uses the stored signature when another attempt created it first", + inFlight: []entity.Batch{analyzerTestBatch("11")}, + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(storage.ErrAlreadyExists) + winner := analyzerTestSignature(analyzerTestNew.ID, 7) + h.signatures.EXPECT().Get(gomock.Any(), analyzerTestNew.ID).Return(winner, true, nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(analyzerTestSignature("11", 7), true, nil) + }, + wantConflicts: []entity.Conflict{{BatchID: "11", Type: entity.ConflictTypeTargetOverlap}}, + }, + { + name: "an in-flight batch with no record fails closed without asking Tango for it", + inFlight: []entity.Batch{analyzerTestBatch("11")}, + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(TargetSignature{}, false, nil) + }, + wantErr: ErrNoSignature, + }, + { + name: "a store error reading an in-flight batch fails the analysis", + inFlight: []entity.Batch{analyzerTestBatch("11")}, + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(TargetSignature{}, false, analyzerTestFailure) + }, + wantErr: analyzerTestFailure, + }, + { + name: "a store error reading the new batch fails the analysis", + setup: func(h *analyzerHarness) { + h.signatures.EXPECT().Get(gomock.Any(), analyzerTestNew.ID).Return(TargetSignature{}, false, analyzerTestFailure) + }, + wantErr: analyzerTestFailure, + }, + { + name: "a Tango error fails the analysis and creates nothing", + setup: func(h *analyzerHarness) { + h.expectNewBatchNotRecorded() + h.changes.EXPECT().ChangesForBatch(gomock.Any(), analyzerTestNew).Return(nil, nil) + h.client.EXPECT().ChangedTargets(gomock.Any(), gomock.Any()).Return(nil, analyzerTestFailure) + }, + wantErr: analyzerTestFailure, + }, + { + name: "a base resolver error fails the analysis", + setup: func(h *analyzerHarness) { + h.expectNewBatchNotRecorded() + h.changes.EXPECT().ChangesForBatch(gomock.Any(), analyzerTestNew).Return(nil, nil) + h.baseErr = analyzerTestFailure + }, + wantErr: analyzerTestFailure, + }, + { + name: "a changes resolver error fails the analysis", + setup: func(h *analyzerHarness) { + h.expectNewBatchNotRecorded() + h.changes.EXPECT().ChangesForBatch(gomock.Any(), analyzerTestNew).Return(nil, analyzerTestFailure) + }, + wantErr: analyzerTestFailure, + }, + { + name: "a create error other than already-exists fails the analysis", + setup: func(h *analyzerHarness) { + h.expectSigningNewBatch(analyzerTestFailure) + }, + wantErr: analyzerTestFailure, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newAnalyzerHarness(t) + tt.setup(h) + + got, err := h.analyzer.Analyze(context.Background(), analyzerTestNew, tt.inFlight) + + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantConflicts, got) + }) + } +} + +func TestAnalyzer_Analyze_NoSignatureIsDistinctFromStoreError(t *testing.T) { + h := newAnalyzerHarness(t) + h.expectSigningNewBatch(nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(TargetSignature{}, false, analyzerTestFailure) + + _, err := h.analyzer.Analyze(context.Background(), analyzerTestNew, []entity.Batch{analyzerTestBatch("11")}) + + require.Error(t, err) + assert.NotErrorIs(t, err, ErrNoSignature) +} + +func TestAnalyzer_Analyze_ReadsEachSignatureOnce(t *testing.T) { + h := newAnalyzerHarness(t) + h.expectSigningNewBatch(nil) + h.signatures.EXPECT().Get(gomock.Any(), "11").Return(analyzerTestSignature("11", 1), true, nil).Times(1) + inFlight := []entity.Batch{analyzerTestBatch("11")} + + _, err := h.analyzer.Analyze(context.Background(), analyzerTestNew, inFlight) + require.NoError(t, err) + _, err = h.analyzer.Analyze(context.Background(), analyzerTestNew, inFlight) + require.NoError(t, err) +} + +func TestAnalyzer_Analyze_ExistingSignatureVanishes(t *testing.T) { + h := newAnalyzerHarness(t) + h.expectSigningNewBatch(storage.ErrAlreadyExists) + h.signatures.EXPECT().Get(gomock.Any(), analyzerTestNew.ID).Return(TargetSignature{}, false, nil) + + _, err := h.analyzer.Analyze(context.Background(), analyzerTestNew, nil) + + require.Error(t, err) +} diff --git a/submitqueue/extension/conflict/tango/cache.go b/submitqueue/extension/conflict/tango/cache.go index aca0711a3..62ad019e0 100644 --- a/submitqueue/extension/conflict/tango/cache.go +++ b/submitqueue/extension/conflict/tango/cache.go @@ -38,10 +38,11 @@ type signatureCacheEntry struct { bytes int64 } -// signatureCache is a byte-bounded LRU of target signatures keyed by queue and -// batch ID, safe for concurrent use. Signatures are immutable, so a cached -// value is shared with callers: neither side may modify Targets. -type signatureCache struct { +// SignatureCache is a byte-bounded LRU of target signatures keyed by queue and +// batch ID, safe for concurrent use and meant to be shared by every queue's +// analyzer. Signatures are immutable, so a cached value is shared rather than +// copied: nobody may modify Targets. A zero budget caches nothing. +type SignatureCache struct { mu sync.Mutex maxBytes int64 usedBytes int64 @@ -49,8 +50,9 @@ type signatureCache struct { entries map[signatureCacheKey]*list.Element } -func newSignatureCache(maxBytes int64) *signatureCache { - return &signatureCache{ +// NewSignatureCache returns an empty cache that holds at most maxBytes. +func NewSignatureCache(maxBytes int64) *SignatureCache { + return &SignatureCache{ maxBytes: maxBytes, byRecency: list.New(), entries: make(map[signatureCacheKey]*list.Element), @@ -64,7 +66,7 @@ func signatureCacheBytes(signature TargetSignature) int64 { } // get returns the cached signature and marks it most recently used. -func (c *signatureCache) get(queue, batchID string) (TargetSignature, bool) { +func (c *SignatureCache) get(queue, batchID string) (TargetSignature, bool) { c.mu.Lock() defer c.mu.Unlock() @@ -78,7 +80,7 @@ func (c *signatureCache) get(queue, batchID string) (TargetSignature, bool) { // put caches signature, evicting least recently used entries to stay within the // budget. A signature larger than the whole budget is not cached. -func (c *signatureCache) put(signature TargetSignature) { +func (c *SignatureCache) put(signature TargetSignature) { size := signatureCacheBytes(signature) if size > c.maxBytes { return @@ -99,7 +101,7 @@ func (c *signatureCache) put(signature TargetSignature) { } } -func (c *signatureCache) removeLocked(element *list.Element) { +func (c *SignatureCache) removeLocked(element *list.Element) { entry := c.byRecency.Remove(element).(*signatureCacheEntry) delete(c.entries, entry.key) c.usedBytes -= entry.bytes diff --git a/submitqueue/extension/conflict/tango/cache_test.go b/submitqueue/extension/conflict/tango/cache_test.go index 2fb171af0..57c925135 100644 --- a/submitqueue/extension/conflict/tango/cache_test.go +++ b/submitqueue/extension/conflict/tango/cache_test.go @@ -48,7 +48,7 @@ func TestSignatureCache_GetPut(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cache := newSignatureCache(1 << 20) + cache := NewSignatureCache(1 << 20) for _, signature := range tt.put { cache.put(signature) } @@ -71,12 +71,12 @@ func TestSignatureCache_Eviction(t *testing.T) { tests := []struct { name string - run func(cache *signatureCache) + run func(cache *SignatureCache) wantFound map[string]bool }{ { name: "least recently put is evicted first", - run: func(cache *signatureCache) { + run: func(cache *SignatureCache) { cache.put(a) cache.put(b) cache.put(c) @@ -85,7 +85,7 @@ func TestSignatureCache_Eviction(t *testing.T) { }, { name: "a read refreshes recency", - run: func(cache *signatureCache) { + run: func(cache *SignatureCache) { cache.put(a) cache.put(b) cache.get("q", "a") @@ -95,7 +95,7 @@ func TestSignatureCache_Eviction(t *testing.T) { }, { name: "putting an existing key does not evict a neighbour", - run: func(cache *signatureCache) { + run: func(cache *SignatureCache) { cache.put(a) cache.put(b) for i := 0; i < 100; i++ { @@ -106,7 +106,7 @@ func TestSignatureCache_Eviction(t *testing.T) { }, { name: "a signature larger than the budget is not cached", - run: func(cache *signatureCache) { + run: func(cache *SignatureCache) { cache.put(a) cache.put(cacheTestSignature("q", "c", 1000)) }, @@ -115,7 +115,7 @@ func TestSignatureCache_Eviction(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cache := newSignatureCache(budgetForTwo) + cache := NewSignatureCache(budgetForTwo) tt.run(cache) @@ -131,7 +131,7 @@ func TestSignatureCache_ChargesSpareCapacity(t *testing.T) { tight := cacheTestSignature("q", "tight", 10) roomy := cacheTestSignature("q", "roomy", 10) roomy.Targets = append(make([]SignatureTarget, 0, 1000), roomy.Targets...) - cache := newSignatureCache(signatureCacheBytes(tight) * 2) + cache := NewSignatureCache(signatureCacheBytes(tight) * 2) cache.put(roomy) @@ -140,7 +140,7 @@ func TestSignatureCache_ChargesSpareCapacity(t *testing.T) { } func TestSignatureCache_ZeroBudgetCachesNothing(t *testing.T) { - cache := newSignatureCache(0) + cache := NewSignatureCache(0) cache.put(cacheTestSignature("q", "1", 0)) @@ -152,7 +152,7 @@ func TestSignatureCache_ConcurrentUse(t *testing.T) { const workers = 8 const perWorker = 200 sample := cacheTestSignature("q", "0", 5) - cache := newSignatureCache(signatureCacheBytes(sample) * 4) + cache := NewSignatureCache(signatureCacheBytes(sample) * 4) var wg sync.WaitGroup for w := 0; w < workers; w++ {