feat(mergeconflict): make merge-conflict check asynchronous via runway by behinddwalls · Pull Request #245 · uber/submitqueue

behinddwalls · 2026-06-15T22:43:35Z

Summary

Why?

The merge-conflict check ran synchronously inside the validate consumer by calling the mergechecker extension inline. A real merge attempt is slow and I/O-heavy, so doing it on the partition lease blocks the pipeline and couples SubmitQueue to the checker's latency. This moves the check to an asynchronous round-trip with runway, modelled on build/buildsignal but across a service boundary.

What?

The pipeline gains validate → mergeconflict ⇢ (runway) ⇢ mergeconflictsignal → batch:

validate drops the inline mergechecker call and now publishes the request id to the internal mergeconflict topic (dedup + change-metadata + claim are unchanged).
mergeconflict (new) publishes the full MergeConflictCheckRequest to the runway-owned merge-conflict-checker queue, keyed by the request id as the client-owned correlation id. No local record is needed — the request id round-trips, so the result correlates straight back to the request (unlike build, whose server-generated id needs a mapping store).
mergeconflictsignal (new) consumes runway's MergeConflictCheckResult off merge-conflict-checker-signal, advances the request to batch when mergeable, or fails it (user error) when conflicted.
DLQ reconcilers drive the request to Error on dead-letter; the signal DLQ reads the request id straight off the result.

Crossing the runway boundary is why these payloads carry full data rather than entity IDs; the new queue-payload-boundary rule is documented in CLAUDE.md, with the pipeline diagram and stage table updated in workflow.md and the superseded mergechecker validate-path row noted in extension-contract.md. The mergechecker package is left in-tree (unused on the validate path); removing it is a follow-up. Runway's service implementation is out of scope — only its contract (added in the parent PR) is consumed here.

Test Plan

✅ bazel build //...
✅ bazel test //... --test_tag_filters=-integration,-e2e (54 tests pass)
✅ make gazelle clean

Stack

behinddwalls · 2026-06-16T03:35:22Z


 **Outputs are unchanged except `pusher`.** This RFC moves the *input* toward identity; five of the six return contracts — conflicts, score, mergeability, change info, build id/status — are exactly what they are today. `pusher` is the lone exception: because its input becomes a *list* of independently-landed batches, its result regroups per batch (`BatchID`-tagged, per-change commit detail kept underneath) so each batch's outcome stays correlatable — the "output mirrors the input unit" principle above. No other output shape changes.

+> **Note (superseded for the validate path).** The `mergechecker.MergeChecker` row above describes the *synchronous*, in-process mergeability check `validate` used to run. That check is now **asynchronous and out-of-process**: `validate` hands off to the `mergeconflict` controller, which publishes a full check request to the runway-owned `merge-conflict-checker` queue; runway performs the merge attempt and returns a result that `mergeconflictsignal` consumes (see [workflow.md](workflow.md)). The `mergechecker` extension package is kept in-tree but unused on the validate path; removing it is a follow-up. The row is retained here as the historical contract.


lets update the RFC to reflect the new contract rather than adding a note here

behinddwalls · 2026-06-16T03:36:51Z

+	// TopicKeyMergeConflict is the internal pipeline stage where validated
+	// requests are published to start an asynchronous merge-conflict check. The
+	// mergeconflict controller consumes it, records the check, and publishes the
+	// full request to runway's merge-conflict-checker queue.
+	TopicKeyMergeConflict TopicKey = "mergeconflict"


records the check? we removed storage layer so no recording anymore, request itself captures the state for it.

behinddwalls · 2026-06-16T03:39:37Z

+		// User error: reject to DLQ, where the request is reconciled to Error.
+		return errs.NewUserError(fmt.Errorf("request %s is not mergeable: %s", request.ID, result.Reason))
+	}


why reject to DLQ when we know the request has failed and we should just mark it failed?

## Summary ### Why? The merge-conflict check ran synchronously inside the `validate` consumer by calling the `mergechecker` extension inline. A real merge attempt is slow and I/O-heavy, so doing it on the partition lease blocks the pipeline and couples SubmitQueue to the checker's latency. This moves the check to an asynchronous round-trip with runway, modelled on `build`/`buildsignal` but across a service boundary. ### What? The pipeline gains `validate → mergeconflict ⇢ (runway) ⇢ mergeconflictsignal → batch`: - `validate` drops the inline `mergechecker` call and now publishes the request id to the internal `mergeconflict` topic (dedup + change-metadata + claim are unchanged). - `mergeconflict` (new) publishes the full `MergeConflictCheckRequest` to the runway-owned `merge-conflict-checker` queue, keyed by the request id as the client-owned correlation id. No local record is needed — the request id round-trips, so the result correlates straight back to the request (unlike `build`, whose server-generated id needs a mapping store). - `mergeconflictsignal` (new) consumes runway's `MergeConflictCheckResult` off `merge-conflict-checker-signal`, advances the request to `batch` when mergeable, or fails it (user error) when conflicted. - DLQ reconcilers drive the request to `Error` on dead-letter; the signal DLQ reads the request id straight off the result. Crossing the runway boundary is why these payloads carry full data rather than entity IDs; the new queue-payload-boundary rule is documented in CLAUDE.md, with the pipeline diagram and stage table updated in workflow.md and the superseded `mergechecker` validate-path row noted in extension-contract.md. The `mergechecker` package is left in-tree (unused on the validate path); removing it is a follow-up. Runway's service implementation is out of scope — only its contract (added in the parent PR) is consumed here. ## Test Plan ✅ `bazel build //...` ✅ `bazel test //... --test_tag_filters=-integration,-e2e` (54 tests pass) ✅ `make gazelle` clean

behinddwalls added the ChangesPlanned label Jun 15, 2026

This was referenced Jun 15, 2026

feat(runway): add merge-conflict check wire contract and topic keys #244

Open

refactor(entity): promote RequestLandStrategy to shared entity/mergestrategy #243

Open

behinddwalls force-pushed the preetam/runway-contract branch from b8f3e22 to ddbb5ab Compare June 16, 2026 03:31

behinddwalls force-pushed the preetam/mergeconflict-async branch from 2d68e19 to e6d2164 Compare June 16, 2026 03:32

behinddwalls force-pushed the preetam/runway-contract branch from ddbb5ab to edf7ab9 Compare June 16, 2026 03:36

behinddwalls force-pushed the preetam/mergeconflict-async branch from e6d2164 to a51a86b Compare June 16, 2026 03:36

behinddwalls commented Jun 16, 2026

View reviewed changes

behinddwalls force-pushed the preetam/mergeconflict-async branch from a51a86b to d565b91 Compare June 16, 2026 03:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(mergeconflict): make merge-conflict check asynchronous via runway#245

feat(mergeconflict): make merge-conflict check asynchronous via runway#245
behinddwalls wants to merge 1 commit into
preetam/runway-contractfrom
preetam/mergeconflict-async

behinddwalls commented Jun 15, 2026 •

edited

Loading

Uh oh!

behinddwalls Jun 16, 2026

Uh oh!

behinddwalls Jun 16, 2026

Uh oh!

behinddwalls Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		Outputs are unchanged except `pusher`. This RFC moves the input toward identity; five of the six return contracts — conflicts, score, mergeability, change info, build id/status — are exactly what they are today. `pusher` is the lone exception: because its input becomes a list of independently-landed batches, its result regroups per batch (`BatchID`-tagged, per-change commit detail kept underneath) so each batch's outcome stays correlatable — the "output mirrors the input unit" principle above. No other output shape changes.

		> Note (superseded for the validate path). The `mergechecker.MergeChecker` row above describes the synchronous, in-process mergeability check `validate` used to run. That check is now asynchronous and out-of-process: `validate` hands off to the `mergeconflict` controller, which publishes a full check request to the runway-owned `merge-conflict-checker` queue; runway performs the merge attempt and returns a result that `mergeconflictsignal` consumes (see [workflow.md](workflow.md)). The `mergechecker` extension package is kept in-tree but unused on the validate path; removing it is a follow-up. The row is retained here as the historical contract.

Conversation

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

Uh oh!

Summary

Why?

What?

Test Plan

Stack

Uh oh!

behinddwalls Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

behinddwalls Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

behinddwalls Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

behinddwalls commented Jun 15, 2026 •

edited

Loading