docs(rfc): Add Tango conflict analyzer design by manjari25 · Pull Request #845 · uber/submitqueue

manjari25 · 2026-10-12T19:50:33Z

Why?

Provides a way to perform conflict analysis rooted in a build graph as opposed to file overlaps or all conflicts.

What?

Adds an RFC for a Tango-backed conflict.Analyzer that stores each batch's changed targets as a signature of 64-bit IDs in a durable store with an in-memory cache, and treats batches as conflicting when their signatures share an ID.

Test Plan

Docs only change - Implementation to follow.

Issue

sbalabanov · 2026-10-13T22:35:04Z

+| 2 | The analyzer gets a signature for each batch |
+| 3a | Look in the in-memory cache |
+| 3b | On a cache miss, look in the signature store; a hit fills the cache |
+| 3c | On a store miss, take the Tango path |


I'd prefer to keep signature store as fail close and if it fails, error out instead of Tango fallback. This may mask the original issue and only extends the agony as Tango would unlikely be able to process the storm of the requests in case of systematic issue.

To implement this, the only call to Tango should be for the new batch only and became #2 Step, it would also populate the local cache and signature store for this batch. We can drive the request through local cache / signature store if we believe it could be seen before, which I think can only happen if some next step fails with infra failure and we retry the same batch.

Updated. Tango is called only for the new batch, and the result goes to the signature store and cache. In-flight batches are read from the cache or store only, and a miss or error fails the analysis. A retry of the new batch uses its existing record and doesn't call Tango. A queue must have no in-flight batches when switched to tango.

sbalabanov · 2026-10-13T22:42:32Z

+| --- | --- | --- | --- |
+| **Entity** | `TargetSignature` | `TargetLabel` | same `TargetSignature` |
+| **Holds** | Per batch: sorted `(ID, distance)` entries, base revision | Per ID: its label (more than one only on a hash collision) | Signatures this process has seen |
+| **Key** | `(queue, batch ID)` | `(queue, ID, label)` | `(queue, batch ID)` |


(queue, ID, label) isn't label the value, and {queue, ID} the key ?

Updated. The key is now (queue, ID) and the label is the value. I had put the label in the key to keep both labels on a hash collision. Collisions are rare, so the first label stays and a mismatch is counted and logged. Is this what you had in mind?

sbalabanov · 2026-10-13T22:43:55Z

+| **Holds** | Per batch: sorted `(ID, distance)` entries, base revision | Per ID: its label (more than one only on a hash collision) | Signatures this process has seen |
+| **Key** | `(queue, batch ID)` | `(queue, ID, label)` | `(queue, batch ID)` |
+| **Durable** | Yes | Yes | No |
+| **Written** | Once per batch, create-if-absent, before the batch is promoted to `Created` | When a Tango response is decoded; asynchronous, best-effort, idempotent | On a miss, after the store or Tango answers |


plz explain why dictionary update is best effort? If it fails open, every op that uses the dictionary should be fail open.

The conflict check never reads the target dictionary, so a missing label can't change a decision. A fail-closed write would put a store with no reader yet, intended for debugging, on the hot path. Its writes could be large, since responses can have millions of targets. A dropped write is usually repaired later, because rows are shared across batches and any later batch touching the same target rewrites the label.

Every dictionary reader shows the ID when no label is found, and failed writes are counted and logged.

My expectation is that conflict relaxation, the only reader I can think of right now, can work from just the IDs and distances in the signature, so it shouldn't need labels. Thoughts?

sbalabanov · 2026-10-13T23:08:37Z

+| **Durable** | Yes | Yes | No |
+| **Written** | Once per batch, create-if-absent, before the batch is promoted to `Created` | When a Tango response is decoded; asynchronous, best-effort, idempotent | On a miss, after the store or Tango answers |
+| **Read by** | The analyzer; relaxation | Debugging and conflict explanations; never the conflict check | The analyzer |
+| **Lifetime** | Created once, never updated | Created once, never updated | Evicted when a batch leaves the in-flight set, or LRU when over the memory budget |


target dictionary should likely be the one per queue, and updated with each batch's new targets?

Yes, it's intended per queue. Each batch adds rows for the targets it brings that the queue hasn't seen before. Rows already there are left alone, since a label is a pure function of its ID and never needs updating. "Created once, never updated" referred to the rows, not the dictionary as a whole. I'll reword that cell.

sbalabanov · 2026-10-13T23:12:19Z

+// TargetLabel is one label and the ID it hashes to.
+type TargetLabel struct {
+    // Queue is the queue the label was seen in. Unique together with ID and Label.
+    Queue string


If we store a lot of target labels in some sort of a dictionary, does it makes sense to have a Queue as a field in each one of them?

Every orchestrator table leads its key with queue, enforced by the queueshard linter (make lint-queue-shard), so that each queue's rows can be sharded and isolated. The dictionary follows the same rule.

sbalabanov · 2026-10-13T23:20:31Z

+- The change type is `INVALID`.
+- Tango returns any other error or times out.
+
+Any of these fails the analysis, and a failure never becomes "no conflicts", because dropping a target would hide a conflict. A queue-configured fallback to `all` is a possible later option.


let's not mention this fallback; I suggest to make conflict analysis fail-close
fallback, if any, is to be implemented outside of SQ ecosystem

behinddwalls · 2026-10-14T19:35:08Z

+| | Signature store | Target dictionary | In-memory cache |
+| --- | --- | --- | --- |
+| **Entity** | `TargetSignature` | `TargetLabel` | same `TargetSignature` |
+| **Holds** | Per batch: sorted `(ID, distance)` entries, base revision | Per ID: its label | Signatures this process has seen |


base revision

what is base revision?

behinddwalls · 2026-10-14T19:36:25Z

+
+## Stores and caches
+
+| | Signature store | Target dictionary | In-memory cache |


Target dictionary

is it needed? i thought we would just convert string targets to int64 IDs by converting the unit64(hash(target))

behinddwalls · 2026-10-14T19:38:39Z

+
+## Entities
+
+`TargetSignature` and `TargetLabel` are immutable records under `submitqueue/entity/`. `SignatureTarget` is a value inside a signature. Each store instance serves a single queue, so a read takes only the batch ID or target ID.


submitqueue/entity/

should they be in entity at all? these specific to an impl of conflictAnalyzer but not really the part of core extension itself.. right?

behinddwalls · 2026-10-14T19:39:38Z

+
+`TargetSignature` and `TargetLabel` are immutable records under `submitqueue/entity/`. `SignatureTarget` is a value inside a signature. Each store instance serves a single queue, so a read takes only the batch ID or target ID.
+
+```go


BaseRevision string

should this be represented using git changeURI and for all purposes we could just say BaseChangeURI

behinddwalls · 2026-10-14T19:42:38Z

+
+The `direct_dependencies` column lists the local IDs of what each target depends on; the signature does not use it. Collect keeps the pairs (3, 0), (1, 1), (4, 2), (2, 0) and (5, 0). Hash maps each local ID to its label and hashes it. Merge sorts by ID, giving the signature stored for the batch:
+
+| ID (hex) | Label | Distance |


ID (hex)

did we decide on inheritence or not? do we have any configurability to choose between no inheritence (current V1 behaviours) and with inheritence?

behinddwalls · 2026-10-14T19:43:08Z

+
+Any of these fails the analysis. The analyzer fails closed, and any fallback belongs outside SubmitQueue.
+
+## Algorithm


Algorithm

i think it's fine to skip it from the rfc.. maybe just raw steps are fine, but no code as such

manjari25 requested review from a team, behinddwalls and sbalabanov as code owners October 12, 2026 19:50

sbalabanov requested changes Oct 13, 2026

View reviewed changes

manjari25 added 6 commits October 14, 2026 01:15

docs(rfc): Add Tango conflict analyzer design

0b7dfd9

docs(rfc): Fail when no signature detected

402566d

docs(rfc): Change key to exclude label, no hash collisions

41d7ac2

docs(rfc): Lifetime update for target dictionary

2d00858

docs(rfc): Reword conflict analysis fallback

f032a4b

docs(rfc): Update diagram and text

746d206

manjari25 force-pushed the manjari/tango-conflict-analyzer-rfc branch from 8ac91f0 to 746d206 Compare October 14, 2026 01:16

manjari25 requested a review from sbalabanov October 14, 2026 01:17

behinddwalls reviewed Oct 14, 2026

View reviewed changes


		## Stores and caches

		\| \| Signature store \| Target dictionary \| In-memory cache \|


		## Entities

		`TargetSignature` and `TargetLabel` are immutable records under `submitqueue/entity/`. `SignatureTarget` is a value inside a signature. Each store instance serves a single queue, so a read takes only the batch ID or target ID.


		`TargetSignature` and `TargetLabel` are immutable records under `submitqueue/entity/`. `SignatureTarget` is a value inside a signature. Each store instance serves a single queue, so a read takes only the batch ID or target ID.

		```go


		The `direct_dependencies` column lists the local IDs of what each target depends on; the signature does not use it. Collect keeps the pairs (3, 0), (1, 1), (4, 2), (2, 0) and (5, 0). Hash maps each local ID to its label and hashes it. Merge sorts by ID, giving the signature stored for the batch:

		\| ID (hex) \| Label \| Distance \|


		Any of these fails the analysis. The analyzer fails closed, and any fallback belongs outside SubmitQueue.

		## Algorithm

Conversation

manjari25 commented Oct 12, 2026

Why?

What?

Test Plan

Issue

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

behinddwalls Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

behinddwalls Oct 14, 2026 •

edited

Loading