diff --git a/Makefile b/Makefile index fdca00055..d137f3067 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ export REPO_ROOT := $(shell pwd) # path, so adding a provider is mostly adding a directory — see # service/submitqueue/demo/provider/README.md. # -# fake a change is a URI; nothing merges anywhere. Needs nothing. +# fake a change is a URI; nothing lands anywhere. Needs nothing. # git branches in a bare repository on disk; real fetch, cherry-pick, push. # github real pull requests. Needs a repository and GITHUB_TOKEN. PROVIDER ?= fake @@ -60,7 +60,7 @@ PROVIDER_COMPOSE_FILE_git = service/submitqueue/docker-compose.git.yml PROVIDER_COMPOSE_FILE_github = service/submitqueue/docker-compose.provider.yml PROVIDER_COMPOSE_FILE = $(PROVIDER_COMPOSE_FILE_$(PROVIDER)) -# Where PROVIDER=git keeps the bare repository it merges into. Outside the +# Where PROVIDER=git keeps the bare repository it lands into. Outside the # repository, so a demo leaves nothing in a checkout, and bind-mounted rather # than kept in a volume so `git log` on the host can show what landed. # @@ -266,7 +266,7 @@ deps: tidy-go ## Download and tidy Go dependencies e2e-git-test: ## Run the hermetic git E2E (real merger against a bare repo; no credentials) @echo "Running hermetic git end-to-end tests..." @$(BAZEL) test //test/e2e/submitqueue:go_default_test --test_output=errors \ - --test_filter='TestGitMergeE2E' + --test_filter='TestGitLandE2E' e2e-test: ## Run end-to-end tests (hermetic; Bazel builds all inputs; runs in parallel) @echo "Running end-to-end tests (parallel)..." @@ -519,7 +519,7 @@ local-submitqueue-start: build-all-linux ## Start full stack (PROVIDER=fake|git| @echo "" @echo "Gateway gRPC port: $$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'unknown')" @if [ "$(PROVIDER)" = "git" ]; then \ - echo "Merge target: $(SQ_GIT_SANDBOX_DIR)/sandbox.git"; \ + echo "Land target: $(SQ_GIT_SANDBOX_DIR)/sandbox.git"; \ fi @echo "" @echo "Generate traffic with:" @@ -579,7 +579,7 @@ local-stovepipe-stop: ## Stop the Stovepipe service mocks: ## Generate mock files using mockgen @echo "Generating mocks..." - @$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/consumergate/... ./platform/extension/hook/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/conflict/... ./submitqueue/extension/speculation/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/core/requestlog/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/... + @$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/consumergate/... ./platform/extension/hook/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./runway/extension/merger/... ./submitqueue/extension/conflict/... ./submitqueue/extension/speculation/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/core/requestlog/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/... @echo "Mocks generated successfully!" proto: ## Generate protobuf files from .proto definitions diff --git a/platform/publish/publish_test.go b/platform/publish/publish_test.go index 23ec08ae7..6dab1f773 100644 --- a/platform/publish/publish_test.go +++ b/platform/publish/publish_test.go @@ -143,8 +143,8 @@ func TestIntentID(t *testing.T) { { name: "single cause", entityID: "batch-1", - cause: []string{"merged"}, - want: "batch-1/merged", + cause: []string{"landed"}, + want: "batch-1/landed", }, { name: "multiple causes join in order", @@ -164,9 +164,9 @@ func TestIntentID(t *testing.T) { // The convention only works if the same cause is repeatable and a different // cause is distinguishable — the two properties every call site relies on. func TestIntentID_StableAcrossCallsAndDistinctPerCause(t *testing.T) { - assert.Equal(t, IntentID("batch-1", "merged"), IntentID("batch-1", "merged")) - assert.NotEqual(t, IntentID("batch-1", "merged"), IntentID("batch-1")) - assert.NotEqual(t, IntentID("batch-1", "merged"), IntentID("batch-1", "cancelling")) + assert.Equal(t, IntentID("batch-1", "landed"), IntentID("batch-1", "landed")) + assert.NotEqual(t, IntentID("batch-1", "landed"), IntentID("batch-1")) + assert.NotEqual(t, IntentID("batch-1", "landed"), IntentID("batch-1", "cancelling")) } func TestUniqueID(t *testing.T) { diff --git a/service/submitqueue/demo/requests/main.go b/service/submitqueue/demo/requests/main.go index c1e3becfa..e978177b9 100644 --- a/service/submitqueue/demo/requests/main.go +++ b/service/submitqueue/demo/requests/main.go @@ -50,7 +50,7 @@ // // - fake (default): a change is a URI and nothing else. No repository, no // credential, no I/O — the fastest way to put traffic through the queue. -// - git: a branch pushed to the sandbox repository the stack merges into. +// - git: a branch pushed to the sandbox repository the stack lands into. // Real commits, still no credential. // - github: a real pull request over the REST API, which needs no clone and // no git binary, only GITHUB_TOKEN — the same credential the stack uses. @@ -130,7 +130,7 @@ func parseFlags() config { flag.BoolVar(&c.tls, "tls", false, "dial the gateway with transport security") flag.StringVar(&c.tokenEnv, "token-env", client.DefaultTokenEnv, "environment variable holding the gateway bearer token") flag.StringVar(&c.queue, "queue", "demo-queue", "queue to land on") - flag.StringVar(&c.strategy, "strategy", "SQUASH_REBASE", "merge strategy") + flag.StringVar(&c.strategy, "strategy", "SQUASH_REBASE", "land strategy") flag.Parse() // Only the GitHub source reads a credential; the other two must not fail, diff --git a/service/submitqueue/gateway/server/mapper/land.go b/service/submitqueue/gateway/server/mapper/land.go index 8c77d56fb..b33fc2c03 100644 --- a/service/submitqueue/gateway/server/mapper/land.go +++ b/service/submitqueue/gateway/server/mapper/land.go @@ -36,7 +36,7 @@ var errUnknownStrategy = errors.New("unknown land strategy in proto message") // ProtoToLandRequest maps the wire LandRequest to the entity.LandRequest the controller operates on. // The ID is left empty; the controller assigns it. func ProtoToLandRequest(req *pb.LandRequest) (entity.LandRequest, error) { - strategy, err := resolveMergeStrategy(req.GetStrategy()) + strategy, err := resolveLandStrategy(req.GetStrategy()) if err != nil { return entity.LandRequest{}, fmt.Errorf("failed to map land strategy: %w", err) } @@ -47,8 +47,8 @@ func ProtoToLandRequest(req *pb.LandRequest) (entity.LandRequest, error) { }, nil } -// resolveMergeStrategy maps a proto Strategy enum to the shared mergestrategy.MergeStrategy. -func resolveMergeStrategy(s mergestrategypb.Strategy) (mergestrategy.MergeStrategy, error) { +// resolveLandStrategy maps a proto Strategy enum to the shared mergestrategy.MergeStrategy. +func resolveLandStrategy(s mergestrategypb.Strategy) (mergestrategy.MergeStrategy, error) { switch s { case mergestrategypb.Strategy_DEFAULT: // TODO: resolve default strategy based on queue configuration diff --git a/service/submitqueue/gateway/server/mapper/land_test.go b/service/submitqueue/gateway/server/mapper/land_test.go index 348282856..f1e6ef253 100644 --- a/service/submitqueue/gateway/server/mapper/land_test.go +++ b/service/submitqueue/gateway/server/mapper/land_test.go @@ -83,7 +83,7 @@ func TestProtoToLandRequest(t *testing.T) { } } -func TestResolveMergeStrategy(t *testing.T) { +func TestResolveLandStrategy(t *testing.T) { tests := []struct { name string in mergestrategypb.Strategy @@ -100,7 +100,7 @@ func TestResolveMergeStrategy(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := resolveMergeStrategy(tt.in) + got, err := resolveLandStrategy(tt.in) if tt.errMsg != "" { assert.ErrorContains(t, err, tt.errMsg) return diff --git a/submitqueue/core/batch/transition_test.go b/submitqueue/core/batch/transition_test.go index b99ff47ce..3bc814d42 100644 --- a/submitqueue/core/batch/transition_test.go +++ b/submitqueue/core/batch/transition_test.go @@ -147,7 +147,7 @@ func TestEnsureRecord(t *testing.T) { batch := entity.Batch{ ID: "monorepo/batch/7", Queue: "monorepo", - State: entity.BatchStateMerging, + State: entity.BatchStateLanding, Version: 5, } storeErr := errors.New("storage failed") diff --git a/submitqueue/core/request/request_test.go b/submitqueue/core/request/request_test.go index fe081ce4a..fd14736ca 100644 --- a/submitqueue/core/request/request_test.go +++ b/submitqueue/core/request/request_test.go @@ -54,9 +54,9 @@ func TestGetCurrentStateFromRequestLog(t *testing.T) { name: "terminal error status with last error", logs: []entity.RequestLog{ {RequestID: "q/1", TimestampMs: 1000, Type: entity.RequestLogTypeStatus, Status: entity.RequestStatusStarted, RequestVersion: 1, LastError: "", Metadata: map[string]string{}}, - {RequestID: "q/1", TimestampMs: 2000, Type: entity.RequestLogTypeStatus, Status: entity.RequestStatusError, RequestVersion: 4, LastError: "merge conflict", Metadata: map[string]string{"step": "merge"}}, + {RequestID: "q/1", TimestampMs: 2000, Type: entity.RequestLogTypeStatus, Status: entity.RequestStatusError, RequestVersion: 4, LastError: "merge conflict", Metadata: map[string]string{"step": "land"}}, }, - expected: CurrentState{Status: entity.RequestStatusError, LastError: "merge conflict", Metadata: map[string]string{"step": "merge"}}, + expected: CurrentState{Status: entity.RequestStatusError, LastError: "merge conflict", Metadata: map[string]string{"step": "land"}}, }, { name: "multiple terminal records picks highest version", diff --git a/submitqueue/core/topickey/topickey.go b/submitqueue/core/topickey/topickey.go index fa31076fa..6bdae00f3 100644 --- a/submitqueue/core/topickey/topickey.go +++ b/submitqueue/core/topickey/topickey.go @@ -44,16 +44,16 @@ const ( // so the state machine re-evaluates, and holds the delivery for the next // poll when the build has not yet reached a terminal state. TopicKeyBuildSignal TopicKey = "buildsignal" - // TopicKeyMerge is the pipeline stage where speculated batches are published for merging. - TopicKeyMerge TopicKey = "submitqueue-merge" - // TopicKeyConclude is the pipeline stage where merged requests are published for conclusion. + // TopicKeyLand is the pipeline stage where speculated batches are published for landing. + TopicKeyLand TopicKey = "submitqueue-land" + // TopicKeyConclude is the pipeline stage where landed requests are published for conclusion. TopicKeyConclude TopicKey = "conclude" // TopicKeyLog is the pipeline stage where per-request logs are written. TopicKeyLog TopicKey = "log" ) // MetadataKeyFailureReason is the conclude message's metadata attribute carrying -// a failed batch's human-readable reason. Set by the failure sites (merge and +// a failed batch's human-readable reason. Set by the failure sites (land and // speculate) on the conclude publish and read by conclude to stamp the request's // terminal log; absent on the landed and cancelled paths. const MetadataKeyFailureReason = "failure_reason" diff --git a/submitqueue/entity/batch.go b/submitqueue/entity/batch.go index f9b280cb8..25f9d4af8 100644 --- a/submitqueue/entity/batch.go +++ b/submitqueue/entity/batch.go @@ -31,15 +31,15 @@ const ( BatchStateCreated BatchState = "created" // BatchStateSpeculating is the state of a batch that is undergoing speculative execution. BatchStateSpeculating BatchState = "speculating" - // BatchStateMerging is the state of a batch that is being merged after speculative execution. - BatchStateMerging BatchState = "merging" + // BatchStateLanding is the state of a batch that is being landed after speculative execution. + BatchStateLanding BatchState = "landing" // BatchStateSucceeded is the terminal state of a batch that has been successfully landed. BatchStateSucceeded BatchState = "succeeded" // BatchStateFailed is the terminal state of a batch that has failed. BatchStateFailed BatchState = "failed" // BatchStateCancelling is the non-terminal intent state set when a cancel has been requested but the // batch has not yet been transitioned to BatchStateCancelled. A batch in this state may still reach - // BatchStateSucceeded or BatchStateFailed if a concurrent merge wins the race (e.g. the push had + // BatchStateSucceeded or BatchStateFailed if a concurrent land wins the race (e.g. the push had // already completed before the cancel CAS observed the batch); those terminal states prevail. // Forward-progress controllers must treat this state as halted (no new work). The speculate // controller owns the transition to the terminal BatchStateCancelled and the downstream fan-out @@ -67,7 +67,7 @@ func (s BatchState) IsTerminal() bool { var nonCancellableBatchStates = map[BatchState]bool{ BatchStateUnknown: true, BatchStateCreating: true, - BatchStateMerging: true, + BatchStateLanding: true, BatchStateSucceeded: true, BatchStateFailed: true, BatchStateCancelled: true, @@ -80,7 +80,7 @@ func (s BatchState) IsCancellable() bool { } // IsBatchStateHalted returns true if the batch is either terminal or in the process of being cancelled. -// Forward-progress controllers (build, buildsignal, speculate, merge) use this to short-circuit +// Forward-progress controllers (build, buildsignal, speculate, land) use this to short-circuit // work for batches that the user has asked to cancel — even though Cancelling is non-terminal, no // further pipeline work should start (cancel will write the terminal state and fan out). func IsBatchStateHalted(s BatchState) bool { @@ -94,7 +94,7 @@ func AllBatchStates() []BatchState { BatchStateCreating, BatchStateCreated, BatchStateSpeculating, - BatchStateMerging, + BatchStateLanding, BatchStateSucceeded, BatchStateFailed, BatchStateCancelling, @@ -108,7 +108,7 @@ func ActiveBatchStates() []BatchState { return []BatchState{ BatchStateCreated, BatchStateSpeculating, - BatchStateMerging, + BatchStateLanding, BatchStateCancelling, } } @@ -127,11 +127,11 @@ func DependencyBatchStates() []BatchState { return []BatchState{ BatchStateCreated, BatchStateSpeculating, - BatchStateMerging, + BatchStateLanding, } } -// Batch represents a group of requests to land (merge into target branch of the source control repository). +// Batch represents a group of requests to land on the source control repository's target branch. type Batch struct { // ID is the globally unique identifier for the batch. Format: "/batch/". ID string diff --git a/submitqueue/entity/batch_test.go b/submitqueue/entity/batch_test.go index 558d95141..0d1496d14 100644 --- a/submitqueue/entity/batch_test.go +++ b/submitqueue/entity/batch_test.go @@ -31,7 +31,7 @@ func TestBatchState_IsTerminal(t *testing.T) { {name: "creating", state: BatchStateCreating, terminal: false}, {name: "created", state: BatchStateCreated, terminal: false}, {name: "speculating", state: BatchStateSpeculating, terminal: false}, - {name: "merging", state: BatchStateMerging, terminal: false}, + {name: "landing", state: BatchStateLanding, terminal: false}, {name: "succeeded", state: BatchStateSucceeded, terminal: true}, {name: "failed", state: BatchStateFailed, terminal: true}, {name: "cancelled", state: BatchStateCancelled, terminal: true}, @@ -52,7 +52,7 @@ func TestIsCancellable(t *testing.T) { assert.True(t, BatchState("future").IsCancellable()) assert.False(t, BatchStateUnknown.IsCancellable()) assert.False(t, BatchStateCreating.IsCancellable()) - assert.False(t, BatchStateMerging.IsCancellable()) + assert.False(t, BatchStateLanding.IsCancellable()) assert.False(t, BatchStateSucceeded.IsCancellable()) assert.False(t, BatchStateFailed.IsCancellable()) assert.False(t, BatchStateCancelled.IsCancellable()) diff --git a/submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go b/submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go index 7dfb9f352..5f0a5d560 100644 --- a/submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go +++ b/submitqueue/extension/speculation/generator/bestfirst/bestfirst_test.go @@ -337,7 +337,7 @@ func TestBestFirst_OnlySpeculatingHeadsProduceCandidates(t *testing.T) { {state: entity.BatchStateUnknown}, {state: entity.BatchStateCreated}, {state: entity.BatchStateSpeculating, want: true}, - {state: entity.BatchStateMerging}, + {state: entity.BatchStateLanding}, {state: entity.BatchStateSucceeded}, {state: entity.BatchStateFailed}, {state: entity.BatchStateCancelling}, @@ -417,13 +417,13 @@ func TestBestFirst_NeverScoresAnAbsentDependency(t *testing.T) { assert.InDelta(t, math.Log(defaultProbability), cands[0].RankingScore, 1e-9) } -// A merging dependency is still in progress — the merge can fail — so it stays +// A landing dependency is still in progress — the land can fail — so it stays // an open question here like any other. Whether a path betting against it is // worth funding is a matter of price, which is the scorer's to say, not a // state the search hard-codes. -func TestBestFirst_MergingDependencyStaysOpen(t *testing.T) { +func TestBestFirst_LandingDependencyStaysOpen(t *testing.T) { batches := []entity.Batch{ - {ID: "q/landing", State: entity.BatchStateMerging}, + {ID: "q/landing", State: entity.BatchStateLanding}, {ID: "q/H", State: entity.BatchStateSpeculating, Dependencies: []string{"q/landing"}}, } sc := newCountingScorer(map[string]float64{"q/landing": 0.9}) @@ -432,8 +432,8 @@ func TestBestFirst_MergingDependencyStaysOpen(t *testing.T) { require.NoError(t, err) cands := drainAll(t, iter) - assert.Equal(t, 1, sc.calls["q/landing"], "a merging dependency is priced like any other") - require.Len(t, cands, 2, "both sides of a merge that has not landed yet") + assert.Equal(t, 1, sc.calls["q/landing"], "a landing dependency is priced like any other") + require.Len(t, cands, 2, "both sides of a land that has not completed yet") assert.Equal(t, entity.DependencyAssumptionSucceeds, assumptionFor(cands[0].Path, "q/landing")) assert.Equal(t, entity.DependencyAssumptionFails, assumptionFor(cands[1].Path, "q/landing")) } diff --git a/submitqueue/extension/storage/mysql/batch_store_test.go b/submitqueue/extension/storage/mysql/batch_store_test.go index 7cd69aeff..145fd7620 100644 --- a/submitqueue/extension/storage/mysql/batch_store_test.go +++ b/submitqueue/extension/storage/mysql/batch_store_test.go @@ -205,7 +205,7 @@ func TestBatchStore_Update(t *testing.T) { Queue: "monorepo", Contains: []string{"monorepo/3", "monorepo/4"}, Dependencies: []string{"monorepo/batch/1", "monorepo/batch/2"}, - State: entity.BatchStateMerging, + State: entity.BatchStateLanding, Version: oldVersion, } containsJSON, err := json.Marshal(batch.Contains) diff --git a/submitqueue/orchestrator/BUILD.bazel b/submitqueue/orchestrator/BUILD.bazel index 35b1d581d..f806c7789 100644 --- a/submitqueue/orchestrator/BUILD.bazel +++ b/submitqueue/orchestrator/BUILD.bazel @@ -28,9 +28,9 @@ go_library( "//submitqueue/orchestrator/controller/conclude:go_default_library", "//submitqueue/orchestrator/controller/dependencyanalysis:go_default_library", "//submitqueue/orchestrator/controller/dlq:go_default_library", - "//submitqueue/orchestrator/controller/merge:go_default_library", - "//submitqueue/orchestrator/controller/mergeconflictsignal:go_default_library", - "//submitqueue/orchestrator/controller/mergesignal:go_default_library", + "//submitqueue/orchestrator/controller/land:go_default_library", + "//submitqueue/orchestrator/controller/landconflictsignal:go_default_library", + "//submitqueue/orchestrator/controller/landsignal:go_default_library", "//submitqueue/orchestrator/controller/speculate:go_default_library", "//submitqueue/orchestrator/controller/start:go_default_library", "//submitqueue/orchestrator/controller/validate:go_default_library", diff --git a/submitqueue/orchestrator/controller/cancel/cancel.go b/submitqueue/orchestrator/controller/cancel/cancel.go index 243e3af32..07493accf 100644 --- a/submitqueue/orchestrator/controller/cancel/cancel.go +++ b/submitqueue/orchestrator/controller/cancel/cancel.go @@ -26,14 +26,14 @@ // - The request is associated with one or more batch attempts — the controller // records cancellation intent on every cancellable attempt and hands each one // to speculate. Creating attempts are ignored because their dependency set is -// not yet resolved and nothing downstream can see them, while Merging and +// not yet resolved and nothing downstream can see them, while Landing and // terminal attempts retain their existing outcome for conclude to reconcile. // // The split exists so that the terminal write and the work that must precede // it (cancelling builds, respeculating dependents) live in the same controller // — speculate is the single writer of every non-Cancelling batch state and is // already wired with the build/dependent stores. Forward-progress controllers -// (build, buildsignal, merge) observe BatchStateCancelling via +// (build, buildsignal, land) observe BatchStateCancelling via // IsBatchStateHalted and short-circuit while speculate drives the batch to // its terminal state. // @@ -149,7 +149,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // Step 1: record the cancellation intent on the request itself by transitioning // to RequestStateCancelling. This is non-terminal; forward-progress controllers // (validate, batch) treat it as halted, but conclude may still write a different - // terminal state if a concurrent merge or failure wins the race. + // terminal state if a concurrent land or failure wins the race. request, err = c.markCancelling(ctx, store, request) if err != nil { return err @@ -180,10 +180,10 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er firstErr = err } } - case batch.State == entity.BatchStateMerging: - // Merge owns the outcome once it has started. Conclude will reconcile the request with that outcome. + case batch.State == entity.BatchStateLanding: + // Land owns the outcome once it has started. Conclude will reconcile the request with that outcome. foundApplicableBatch = true - metrics.NamedCounter(c.metricsScope, opName, "batch_merging", 1) + metrics.NamedCounter(c.metricsScope, opName, "batch_landing", 1) case batch.State.IsTerminal(): // The terminal batch outcome wins; conclude may not have reconciled the request yet. foundApplicableBatch = true @@ -311,7 +311,7 @@ func (c *Controller) cancelBatch(ctx context.Context, store storage.Storage, bat if err != nil { metrics.NamedCounter(c.metricsScope, opName, "batch_update_errors", 1) // storage.ErrVersionMismatch here means the batch advanced concurrently - // (e.g. speculate / merge progressed). Returned as-is because the + // (e.g. speculate / land progressed). Returned as-is because the // sentinel is intrinsically retryable; the re-fetch will see the new state // and either short-circuit (already terminal) or attempt the transition // again. diff --git a/submitqueue/orchestrator/controller/cancel/cancel_test.go b/submitqueue/orchestrator/controller/cancel/cancel_test.go index 01274bafa..e5bc743e4 100644 --- a/submitqueue/orchestrator/controller/cancel/cancel_test.go +++ b/submitqueue/orchestrator/controller/cancel/cancel_test.go @@ -517,7 +517,7 @@ func TestProcess_NonCancellableBatchSuppressesRequestCancellation(t *testing.T) name string state entity.BatchState }{ - {name: "merging", state: entity.BatchStateMerging}, + {name: "landing", state: entity.BatchStateLanding}, {name: "succeeded", state: entity.BatchStateSucceeded}, {name: "failed", state: entity.BatchStateFailed}, {name: "cancelled", state: entity.BatchStateCancelled}, diff --git a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go index 1a04e7032..c04619d17 100644 --- a/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go +++ b/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis_test.go @@ -410,7 +410,7 @@ func TestController_Process_HaltedBatchAcksWithoutPublishing(t *testing.T) { // A batch already admitted got its announcement; re-sending one would only buy // a redundant re-plan. func TestController_Process_AlreadyAdmittedAcksWithoutPublishing(t *testing.T) { - for _, state := range []entity.BatchState{entity.BatchStateSpeculating, entity.BatchStateMerging} { + for _, state := range []entity.BatchState{entity.BatchStateSpeculating, entity.BatchStateLanding} { t.Run(string(state), func(t *testing.T) { ctrl := gomock.NewController(t) diff --git a/submitqueue/orchestrator/controller/dlq/BUILD.bazel b/submitqueue/orchestrator/controller/dlq/BUILD.bazel index ffcbd2eb1..a78013a4e 100644 --- a/submitqueue/orchestrator/controller/dlq/BUILD.bazel +++ b/submitqueue/orchestrator/controller/dlq/BUILD.bazel @@ -6,9 +6,9 @@ go_library( "batch.go", "buildsignal.go", "dlq.go", + "landconflictsignal.go", + "landsignal.go", "log.go", - "mergeconflictsignal.go", - "mergesignal.go", "request.go", "speculate.go", ], @@ -35,9 +35,9 @@ go_test( "batch_test.go", "buildsignal_test.go", "dlq_test.go", + "landconflictsignal_test.go", + "landsignal_test.go", "log_test.go", - "mergeconflictsignal_test.go", - "mergesignal_test.go", "publisher_test.go", "request_test.go", "speculate_test.go", diff --git a/submitqueue/orchestrator/controller/dlq/batch_test.go b/submitqueue/orchestrator/controller/dlq/batch_test.go index a125271df..a75fea26b 100644 --- a/submitqueue/orchestrator/controller/dlq/batch_test.go +++ b/submitqueue/orchestrator/controller/dlq/batch_test.go @@ -33,11 +33,11 @@ func TestDLQBatchController_InterfaceAndAccessors(t *testing.T) { store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyMerge), "orchestrator-merge-dlq") + c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") - assert.Equal(t, "submitqueue-merge_dlq", c.Name()) - assert.Equal(t, consumer.TopicKey("submitqueue-merge_dlq"), c.TopicKey()) - assert.Equal(t, "orchestrator-merge-dlq", c.ConsumerGroup()) + assert.Equal(t, "submitqueue-land_dlq", c.Name()) + assert.Equal(t, consumer.TopicKey("submitqueue-land_dlq"), c.TopicKey()) + assert.Equal(t, "orchestrator-land-dlq", c.ConsumerGroup()) } func TestDLQBatchController_Process_FailsAndFansOut(t *testing.T) { @@ -46,7 +46,7 @@ func TestDLQBatchController_Process_FailsAndFansOut(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batch := entity.Batch{ ID: "q/batch/9", Queue: "q", Contains: []string{"q/1"}, - State: entity.BatchStateMerging, Version: 2, + State: entity.BatchStateLanding, Version: 2, } batchStore.EXPECT().Get(gomock.Any(), "q/batch/9").Return(batch, nil) batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateFailed), int32(2), int32(3)).Return(nil) @@ -67,7 +67,7 @@ func TestDLQBatchController_Process_FailsAndFansOut(t *testing.T) { store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() - c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(topickey.TopicKeyMerge), "orchestrator-merge-dlq") + c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") payload, err := entity.BatchID{ID: "q/batch/9"}.ToBytes() require.NoError(t, err) @@ -81,7 +81,7 @@ func TestDLQBatchController_Process_MalformedPayloadFails(t *testing.T) { store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyMerge), "orchestrator-merge-dlq") + c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") delivery := newMockDelivery(ctrl, []byte("garbage")) err := c.Process(context.Background(), delivery) @@ -93,7 +93,7 @@ func TestDLQBatchController_Process_EmptyIDFails(t *testing.T) { store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyMerge), "orchestrator-merge-dlq") + c := NewDLQBatchController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(topickey.TopicKeyLand), "orchestrator-land-dlq") payload, err := entity.BatchID{ID: ""}.ToBytes() require.NoError(t, err) diff --git a/submitqueue/orchestrator/controller/dlq/dlq_test.go b/submitqueue/orchestrator/controller/dlq/dlq_test.go index 8d7f63b98..d44bf3443 100644 --- a/submitqueue/orchestrator/controller/dlq/dlq_test.go +++ b/submitqueue/orchestrator/controller/dlq/dlq_test.go @@ -221,7 +221,7 @@ func TestFailBatch_TransitionsAndFansOut(t *testing.T) { batchStore := storagemock.NewMockBatchStore(ctrl) batch := entity.Batch{ ID: "q/batch/1", Queue: "q", Contains: []string{"q/1", "q/2"}, - State: entity.BatchStateMerging, Version: 4, + State: entity.BatchStateLanding, Version: 4, } batchStore.EXPECT().Get(gomock.Any(), "q/batch/1").Return(batch, nil) batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateFailed), int32(4), int32(5)).Return(nil) diff --git a/submitqueue/orchestrator/controller/dlq/mergeconflictsignal.go b/submitqueue/orchestrator/controller/dlq/landconflictsignal.go similarity index 75% rename from submitqueue/orchestrator/controller/dlq/mergeconflictsignal.go rename to submitqueue/orchestrator/controller/dlq/landconflictsignal.go index c51561288..471b30a0d 100644 --- a/submitqueue/orchestrator/controller/dlq/mergeconflictsignal.go +++ b/submitqueue/orchestrator/controller/dlq/landconflictsignal.go @@ -26,11 +26,11 @@ import ( "go.uber.org/zap" ) -// mergeConflictSignalController is the DLQ reconciler for the -// mergeconflictsignal topic. Its payload carries a runway +// landConflictSignalController is the DLQ reconciler for the +// landconflictsignal topic. Its payload carries a runway // MergeResult whose id is the request id echoed back, so // reconciliation fails that request directly via failRequest. -type mergeConflictSignalController struct { +type landConflictSignalController struct { logger *zap.SugaredLogger metricsScope tally.Scope stores storage.Factory @@ -39,12 +39,12 @@ type mergeConflictSignalController struct { consumerGroup string } -// Verify mergeConflictSignalController implements consumer.Controller at compile time. -var _ consumer.Controller = (*mergeConflictSignalController)(nil) +// Verify landConflictSignalController implements consumer.Controller at compile time. +var _ consumer.Controller = (*landConflictSignalController)(nil) -// NewDLQMergeConflictSignalController builds a DLQ controller for the -// mergeconflictsignal topic. -func NewDLQMergeConflictSignalController( +// NewDLQLandConflictSignalController builds a DLQ controller for the +// landconflictsignal topic. +func NewDLQLandConflictSignalController( logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, @@ -53,7 +53,7 @@ func NewDLQMergeConflictSignalController( consumerGroup string, ) consumer.Controller { name := string(topicKey) + "_controller" - return &mergeConflictSignalController{ + return &landConflictSignalController{ logger: logger.Named(name), metricsScope: scope.SubScope(name), stores: stores, @@ -63,8 +63,8 @@ func NewDLQMergeConflictSignalController( } } -// Process reconciles a single DLQ delivery for the mergeconflictsignal topic. -func (c *mergeConflictSignalController) Process(ctx context.Context, delivery consumer.Delivery) error { +// Process reconciles a single DLQ delivery for the landconflictsignal topic. +func (c *landConflictSignalController) Process(ctx context.Context, delivery consumer.Delivery) error { const opName = "process" msg := delivery.Message() @@ -72,7 +72,7 @@ func (c *mergeConflictSignalController) Process(ctx context.Context, delivery co result := &runwaymq.MergeResult{} if err := runwaymq.Unmarshal(msg.Payload, result); err != nil { metrics.NamedCounter(c.metricsScope, opName, "deserialize_errors", 1) - return fmt.Errorf("failed to decode merge conflict check result from dlq payload: %w", err) + return fmt.Errorf("failed to decode land conflict check result from dlq payload: %w", err) } store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) @@ -101,16 +101,16 @@ func (c *mergeConflictSignalController) Process(ctx context.Context, delivery co } // Name returns the controller name for logging and metrics. -func (c *mergeConflictSignalController) Name() string { +func (c *landConflictSignalController) Name() string { return string(c.topicKey) } // TopicKey returns the topic key this controller subscribes to. -func (c *mergeConflictSignalController) TopicKey() consumer.TopicKey { +func (c *landConflictSignalController) TopicKey() consumer.TopicKey { return c.topicKey } // ConsumerGroup returns the consumer group for offset tracking. -func (c *mergeConflictSignalController) ConsumerGroup() string { +func (c *landConflictSignalController) ConsumerGroup() string { return c.consumerGroup } diff --git a/submitqueue/orchestrator/controller/dlq/mergeconflictsignal_test.go b/submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go similarity index 71% rename from submitqueue/orchestrator/controller/dlq/mergeconflictsignal_test.go rename to submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go index bf23a1d6a..74acba067 100644 --- a/submitqueue/orchestrator/controller/dlq/mergeconflictsignal_test.go +++ b/submitqueue/orchestrator/controller/dlq/landconflictsignal_test.go @@ -29,19 +29,19 @@ import ( "go.uber.org/zap/zaptest" ) -func TestDLQMergeConflictSignalController_InterfaceAndAccessors(t *testing.T) { +func TestDLQLandConflictSignalController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQMergeConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-mergeconflictsignal-dlq") + c := NewDLQLandConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-landconflictsignal-dlq") assert.Equal(t, "merge-conflict-check-signal_dlq", c.Name()) assert.Equal(t, consumer.TopicKey("merge-conflict-check-signal_dlq"), c.TopicKey()) - assert.Equal(t, "orchestrator-mergeconflictsignal-dlq", c.ConsumerGroup()) + assert.Equal(t, "orchestrator-landconflictsignal-dlq", c.ConsumerGroup()) } -func TestDLQMergeConflictSignalController_Process_ReconcilesRequest(t *testing.T) { +func TestDLQLandConflictSignalController_Process_ReconcilesRequest(t *testing.T) { ctrl := gomock.NewController(t) requestStore := storagemock.NewMockRequestStore(ctrl) @@ -59,7 +59,7 @@ func TestDLQMergeConflictSignalController_Process_ReconcilesRequest(t *testing.T store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() - c := NewDLQMergeConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-mergeconflictsignal-dlq") + c := NewDLQLandConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-landconflictsignal-dlq") payload, err := runwaymq.Marshal(&runwaymq.MergeResult{Id: "q/1", Outcome: runwaypb.Outcome_FAILED, Reason: "boom"}) require.NoError(t, err) @@ -68,12 +68,12 @@ func TestDLQMergeConflictSignalController_Process_ReconcilesRequest(t *testing.T require.NoError(t, c.Process(context.Background(), delivery)) } -func TestDLQMergeConflictSignalController_Process_MalformedPayloadFails(t *testing.T) { +func TestDLQLandConflictSignalController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQMergeConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-mergeconflictsignal-dlq") + c := NewDLQLandConflictSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeConflictCheckSignal), "orchestrator-landconflictsignal-dlq") delivery := newMockDelivery(ctrl, []byte("garbage")) require.Error(t, c.Process(context.Background(), delivery)) diff --git a/submitqueue/orchestrator/controller/dlq/mergesignal.go b/submitqueue/orchestrator/controller/dlq/landsignal.go similarity index 77% rename from submitqueue/orchestrator/controller/dlq/mergesignal.go rename to submitqueue/orchestrator/controller/dlq/landsignal.go index 8eb3d9e4f..412f51bb3 100644 --- a/submitqueue/orchestrator/controller/dlq/mergesignal.go +++ b/submitqueue/orchestrator/controller/dlq/landsignal.go @@ -26,11 +26,11 @@ import ( "go.uber.org/zap" ) -// mergeSignalController is the DLQ reconciler for the mergesignal topic. Its -// payload carries a runway MergeResult whose id is the batch id echoed back, so +// landSignalController is the DLQ reconciler for the landsignal topic. Its +// payload carries a Runway MergeResult whose id is the batch id echoed back, so // reconciliation fails that batch directly via failBatch (which also fans out // to the member requests). -type mergeSignalController struct { +type landSignalController struct { logger *zap.SugaredLogger metricsScope tally.Scope stores storage.Factory @@ -39,11 +39,11 @@ type mergeSignalController struct { consumerGroup string } -// Verify mergeSignalController implements consumer.Controller at compile time. -var _ consumer.Controller = (*mergeSignalController)(nil) +// Verify landSignalController implements consumer.Controller at compile time. +var _ consumer.Controller = (*landSignalController)(nil) -// NewDLQMergeSignalController builds a DLQ controller for the mergesignal topic. -func NewDLQMergeSignalController( +// NewDLQLandSignalController builds a DLQ controller for the landsignal topic. +func NewDLQLandSignalController( logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, @@ -52,7 +52,7 @@ func NewDLQMergeSignalController( consumerGroup string, ) consumer.Controller { name := string(topicKey) + "_controller" - return &mergeSignalController{ + return &landSignalController{ logger: logger.Named(name), metricsScope: scope.SubScope(name), stores: stores, @@ -62,8 +62,8 @@ func NewDLQMergeSignalController( } } -// Process reconciles a single DLQ delivery for the mergesignal topic. -func (c *mergeSignalController) Process(ctx context.Context, delivery consumer.Delivery) error { +// Process reconciles a single DLQ delivery for the landsignal topic. +func (c *landSignalController) Process(ctx context.Context, delivery consumer.Delivery) error { const opName = "process" msg := delivery.Message() @@ -71,7 +71,7 @@ func (c *mergeSignalController) Process(ctx context.Context, delivery consumer.D result := &runwaymq.MergeResult{} if err := runwaymq.Unmarshal(msg.Payload, result); err != nil { metrics.NamedCounter(c.metricsScope, opName, "deserialize_errors", 1) - return fmt.Errorf("failed to decode merge result from dlq payload: %w", err) + return fmt.Errorf("failed to decode land result from dlq payload: %w", err) } store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) @@ -100,16 +100,16 @@ func (c *mergeSignalController) Process(ctx context.Context, delivery consumer.D } // Name returns the controller name for logging and metrics. -func (c *mergeSignalController) Name() string { +func (c *landSignalController) Name() string { return string(c.topicKey) } // TopicKey returns the topic key this controller subscribes to. -func (c *mergeSignalController) TopicKey() consumer.TopicKey { +func (c *landSignalController) TopicKey() consumer.TopicKey { return c.topicKey } // ConsumerGroup returns the consumer group for offset tracking. -func (c *mergeSignalController) ConsumerGroup() string { +func (c *landSignalController) ConsumerGroup() string { return c.consumerGroup } diff --git a/submitqueue/orchestrator/controller/dlq/mergesignal_test.go b/submitqueue/orchestrator/controller/dlq/landsignal_test.go similarity index 76% rename from submitqueue/orchestrator/controller/dlq/mergesignal_test.go rename to submitqueue/orchestrator/controller/dlq/landsignal_test.go index 69a122ae4..ef3e5efe5 100644 --- a/submitqueue/orchestrator/controller/dlq/mergesignal_test.go +++ b/submitqueue/orchestrator/controller/dlq/landsignal_test.go @@ -29,27 +29,27 @@ import ( "go.uber.org/zap/zaptest" ) -func TestDLQMergeSignalController_InterfaceAndAccessors(t *testing.T) { +func TestDLQLandSignalController_InterfaceAndAccessors(t *testing.T) { ctrl := gomock.NewController(t) store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQMergeSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-mergesignal-dlq") + c := NewDLQLandSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-landsignal-dlq") assert.Equal(t, "merge-signal_dlq", c.Name()) assert.Equal(t, consumer.TopicKey("merge-signal_dlq"), c.TopicKey()) - assert.Equal(t, "orchestrator-mergesignal-dlq", c.ConsumerGroup()) + assert.Equal(t, "orchestrator-landsignal-dlq", c.ConsumerGroup()) } // The payload id is the batch id echoed back, so reconciliation fails the batch // and fans out to its member requests via failBatch. -func TestDLQMergeSignalController_Process_ReconcilesBatch(t *testing.T) { +func TestDLQLandSignalController_Process_ReconcilesBatch(t *testing.T) { ctrl := gomock.NewController(t) batchStore := storagemock.NewMockBatchStore(ctrl) batch := entity.Batch{ ID: "q/batch/1", Queue: "q", Contains: []string{"q/1"}, - State: entity.BatchStateMerging, Version: 2, + State: entity.BatchStateLanding, Version: 2, } batchStore.EXPECT().Get(gomock.Any(), "q/batch/1").Return(batch, nil) batchStore.EXPECT().Update(gomock.Any(), batchWithState(batch, entity.BatchStateFailed), int32(2), int32(3)).Return(nil) @@ -70,7 +70,7 @@ func TestDLQMergeSignalController_Process_ReconcilesBatch(t *testing.T) { store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() store.EXPECT().GetRequestStore().Return(requestStore).AnyTimes() - c := NewDLQMergeSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-mergesignal-dlq") + c := NewDLQLandSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, registry, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-landsignal-dlq") payload, err := runwaymq.Marshal(&runwaymq.MergeResult{Id: "q/batch/1", Outcome: runwaypb.Outcome_FAILED, Reason: "boom"}) require.NoError(t, err) @@ -79,12 +79,12 @@ func TestDLQMergeSignalController_Process_ReconcilesBatch(t *testing.T) { require.NoError(t, c.Process(context.Background(), delivery)) } -func TestDLQMergeSignalController_Process_MalformedPayloadFails(t *testing.T) { +func TestDLQLandSignalController_Process_MalformedPayloadFails(t *testing.T) { ctrl := gomock.NewController(t) store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetQueueBatchStateStore().Return(newQueueBatchStateStore(ctrl)).AnyTimes() - c := NewDLQMergeSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-mergesignal-dlq") + c := NewDLQLandSignalController(zaptest.NewLogger(t).Sugar(), testScope(), staticStorageFactory{store: store}, consumer.TopicRegistry{}, TopicKey(runwaymq.TopicKeyMergeSignal), "orchestrator-landsignal-dlq") delivery := newMockDelivery(ctrl, []byte("garbage")) require.Error(t, c.Process(context.Background(), delivery)) diff --git a/submitqueue/orchestrator/controller/dlq/request_test.go b/submitqueue/orchestrator/controller/dlq/request_test.go index adc34d064..f25269d21 100644 --- a/submitqueue/orchestrator/controller/dlq/request_test.go +++ b/submitqueue/orchestrator/controller/dlq/request_test.go @@ -213,7 +213,7 @@ func TestDLQRequestController_Process_SkipsRequestOwnedByLiveBatch(t *testing.T) for _, state := range []entity.BatchState{ entity.BatchStateCreated, entity.BatchStateSpeculating, - entity.BatchStateMerging, + entity.BatchStateLanding, entity.BatchStateCancelling, } { t.Run(string(state), func(t *testing.T) { diff --git a/submitqueue/orchestrator/controller/merge/BUILD.bazel b/submitqueue/orchestrator/controller/land/BUILD.bazel similarity index 96% rename from submitqueue/orchestrator/controller/merge/BUILD.bazel rename to submitqueue/orchestrator/controller/land/BUILD.bazel index a0eb8359a..806647872 100644 --- a/submitqueue/orchestrator/controller/merge/BUILD.bazel +++ b/submitqueue/orchestrator/controller/land/BUILD.bazel @@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["merge.go"], - importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/merge", + srcs = ["land.go"], + importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/land", visibility = ["//visibility:public"], deps = [ "//api/base/change/protopb:go_default_library", @@ -23,7 +23,7 @@ go_library( go_test( name = "go_default_test", - srcs = ["merge_test.go"], + srcs = ["land_test.go"], embed = [":go_default_library"], deps = [ "//api/base/mergestrategy/protopb:go_default_library", diff --git a/submitqueue/orchestrator/controller/merge/merge.go b/submitqueue/orchestrator/controller/land/land.go similarity index 75% rename from submitqueue/orchestrator/controller/merge/merge.go rename to submitqueue/orchestrator/controller/land/land.go index a9657457b..5c3dfaa99 100644 --- a/submitqueue/orchestrator/controller/merge/merge.go +++ b/submitqueue/orchestrator/controller/land/land.go @@ -12,14 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package merge implements the trigger stage for the asynchronous merge. It -// consumes a batch ready to land, builds the full merge request from the +// Package land implements the trigger stage for the asynchronous land. It +// consumes a batch ready to land, builds the full land request from the // batch's member requests (one step per request, in Contains order), and -// publishes it to runway's merge queue using the batch id as the client-owned -// correlation id. Runway performs the merge out of process and publishes the -// result to the merge-signal queue, which the mergesignal stage consumes and +// publishes it to Runway's merge queue using the batch id as the client-owned +// correlation id. Runway executes the request as a merge and publishes the +// result to the merge-signal queue, which the landsignal stage consumes and // correlates back to the batch by that id. -package merge +package land import ( "context" @@ -29,7 +29,7 @@ import ( "go.uber.org/zap" changepb "github.com/uber/submitqueue/api/base/change/protopb" - strategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" + mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" runwaymq "github.com/uber/submitqueue/api/runway/messagequeue" "github.com/uber/submitqueue/platform/base/mergestrategy" "github.com/uber/submitqueue/platform/consumer" @@ -40,13 +40,13 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/storage" ) -// Controller handles merge queue messages. Implements consumer.Controller. +// Controller handles land queue messages. Implements consumer.Controller. // -// It loads the batch and its member requests, assembles the full merge request +// It loads the batch and its member requests, assembles the full land request // (one step per member request, in Contains order, each carrying that request's -// change and land strategy), and publishes it to runway's merge queue. Runway -// performs the merge out of process and returns the result on the merge-signal -// queue; the mergesignal stage consumes it and transitions the batch. This +// change and land strategy), and publishes it to Runway's merge queue. Runway +// executes the request as a merge and returns the result on the merge-signal +// queue; the landsignal stage consumes it and transitions the batch. This // controller therefore performs no state transition itself. type Controller struct { logger *zap.SugaredLogger @@ -61,8 +61,8 @@ type Controller struct { // Verify Controller implements consumer.Controller interface at compile time. var _ consumer.Controller = (*Controller)(nil) -// NewController creates a new merge controller for the orchestrator. -// runwayTopicKey is the runway-owned topic this controller publishes merge +// NewController creates a new land controller for the orchestrator. +// runwayTopicKey is the runway-owned topic this controller publishes land // requests to (TopicKeyMerge). func NewController( logger *zap.SugaredLogger, @@ -74,8 +74,8 @@ func NewController( consumerGroup string, ) *Controller { return &Controller{ - logger: logger.Named("merge_controller"), - metricsScope: scope.SubScope("merge_controller"), + logger: logger.Named("land_controller"), + metricsScope: scope.SubScope("land_controller"), stores: stores, registry: registry, runwayTopicKey: runwayTopicKey, @@ -84,12 +84,12 @@ func NewController( } } -// Process publishes the full merge request to runway. Returns nil to ack +// Process publishes the full Runway merge request for this land. Returns nil to ack // (success), or error to nack/reject. // // Error classification: deserialize and storage failures are non-retryable // (reject to DLQ). The publish to runway is retryable — it is the hand-off that -// keeps the merge alive, so a transient enqueue blip should replay rather than +// keeps the land alive, so a transient enqueue blip should replay rather than // strand the batch. func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) error { const opName = "process" @@ -122,7 +122,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("payload queue %q does not match queue %q of batch %s", bid.Queue, batch.Queue, batch.ID) } - c.logger.Infow("received merge event", + c.logger.Infow("received land event", "batch_id", batch.ID, "queue", batch.Queue, "state", string(batch.State), @@ -131,27 +131,27 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er "partition_key", msg.PartitionKey, ) - // Short-circuit halted batches (terminal or cancelling): no merge should be + // Short-circuit halted batches (terminal or cancelling): no land should be // kicked off for a batch that will not proceed. Unlike the old synchronous - // merge there is no terminal re-fan-out here — the mergesignal stage owns the + // land there is no terminal re-fan-out here — the landsignal stage owns the // state transition and fan-out once runway's result returns, so a redelivery // at this stage simply acks. if entity.IsBatchStateHalted(batch.State) { metrics.NamedCounter(c.metricsScope, opName, "skipped_halted", 1) - c.logger.Infow("skipping merge for halted batch", + c.logger.Infow("skipping land for halted batch", "batch_id", batch.ID, "state", string(batch.State), ) return nil } - // Build the full payload runway needs to perform the merge. The batch id is + // Build the full merge payload Runway needs to execute the land. The batch id is // the client-owned correlation id, so a redelivery republishes the same id // and runway dedupes on it; the result is matched straight back to the batch. - req, err := c.buildMergeRequest(ctx, store, batch) + req, err := c.buildLandRequest(ctx, store, batch) if err != nil { metrics.NamedCounter(c.metricsScope, opName, "storage_errors", 1) - return fmt.Errorf("failed to build merge request for batch %s: %w", batch.ID, err) + return fmt.Errorf("failed to build land request for batch %s: %w", batch.ID, err) } // Report that the members are landing before the request goes out, so a @@ -169,7 +169,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("failed to publish to runway merge: %w", err) } - c.logger.Infow("published merge to runway", + c.logger.Infow("published merge request to Runway", "batch_id", batch.ID, "steps", len(req.Steps), "topic_key", c.runwayTopicKey, @@ -178,10 +178,10 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return nil // Success - message will be acked } -// buildMergeRequest loads the batch's member requests and assembles the runway -// merge request: one MergeStep per request, in Contains order, attributed by +// buildLandRequest loads the batch's member requests and assembles the runway +// MergeRequest: one MergeStep per request, in Contains order, attributed by // request id and carrying that request's change and land strategy. -func (c *Controller) buildMergeRequest(ctx context.Context, store storage.Storage, batch entity.Batch) (*runwaymq.MergeRequest, error) { +func (c *Controller) buildLandRequest(ctx context.Context, store storage.Storage, batch entity.Batch) (*runwaymq.MergeRequest, error) { steps := make([]*runwaymq.MergeStep, 0, len(batch.Contains)) for _, requestID := range batch.Contains { request, err := store.GetRequestStore().Get(ctx, requestID) @@ -204,29 +204,31 @@ func (c *Controller) buildMergeRequest(ctx context.Context, store storage.Storag // toProtoStrategy maps the shared mergestrategy.MergeStrategy entity to the // proto Strategy enum carried on the wire. An unknown strategy maps to DEFAULT, // letting runway apply the queue's configured default. -func toProtoStrategy(s mergestrategy.MergeStrategy) strategypb.Strategy { +func toProtoStrategy(s mergestrategy.MergeStrategy) mergestrategypb.Strategy { switch s { case mergestrategy.MergeStrategyRebase: - return strategypb.Strategy_REBASE + return mergestrategypb.Strategy_REBASE case mergestrategy.MergeStrategySquashRebase: - return strategypb.Strategy_SQUASH_REBASE + return mergestrategypb.Strategy_SQUASH_REBASE case mergestrategy.MergeStrategyMerge: - return strategypb.Strategy_MERGE + return mergestrategypb.Strategy_MERGE + case mergestrategy.MergeStrategyPromote: + return mergestrategypb.Strategy_PROMOTE default: - return strategypb.Strategy_DEFAULT + return mergestrategypb.Strategy_DEFAULT } } -// publish serializes the runway merge request and publishes it to the given +// publish serializes the Runway merge request and publishes it to the given // topic key, partitioned by queue. // -// The correlation ID is the message ID with no cause: a batch is asked to merge +// The correlation ID is the message ID with no cause: a batch is asked to land // once, so a redelivery that re-asks is meant to dedup rather than have Runway // merge the same batch twice. func (c *Controller) publish(ctx context.Context, key consumer.TopicKey, req *runwaymq.MergeRequest, partitionKey string) error { payload, err := runwaymq.Marshal(req) if err != nil { - return fmt.Errorf("failed to serialize merge request: %w", err) + return fmt.Errorf("failed to serialize land request: %w", err) } if err := publish.Message(ctx, c.registry, key, publish.IntentID(req.GetId()), payload, partitionKey); err != nil { @@ -238,7 +240,7 @@ func (c *Controller) publish(ctx context.Context, key consumer.TopicKey, req *ru // Name returns the controller name for logging and metrics. func (c *Controller) Name() string { - return "merge" + return "land" } // TopicKey returns the topic key this controller subscribes to. diff --git a/submitqueue/orchestrator/controller/merge/merge_test.go b/submitqueue/orchestrator/controller/land/land_test.go similarity index 90% rename from submitqueue/orchestrator/controller/merge/merge_test.go rename to submitqueue/orchestrator/controller/land/land_test.go index 90c63cd6c..557c1721a 100644 --- a/submitqueue/orchestrator/controller/merge/merge_test.go +++ b/submitqueue/orchestrator/controller/land/land_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package merge +package land import ( "context" @@ -53,6 +53,26 @@ func batchIDPayload(t *testing.T, id string) []byte { return payload } +func TestToProtoStrategy(t *testing.T) { + tests := []struct { + name string + in mergestrategy.MergeStrategy + want strategypb.Strategy + }{ + {name: "default", in: mergestrategy.MergeStrategyUnknown, want: strategypb.Strategy_DEFAULT}, + {name: "rebase", in: mergestrategy.MergeStrategyRebase, want: strategypb.Strategy_REBASE}, + {name: "squash rebase", in: mergestrategy.MergeStrategySquashRebase, want: strategypb.Strategy_SQUASH_REBASE}, + {name: "merge", in: mergestrategy.MergeStrategyMerge, want: strategypb.Strategy_MERGE}, + {name: "promote", in: mergestrategy.MergeStrategyPromote, want: strategypb.Strategy_PROMOTE}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, toProtoStrategy(tt.in)) + }) + } +} + func newDelivery(t *testing.T, ctrl *gomock.Controller, batchID, partitionKey string) *consumermock.MockDelivery { msg := entityqueue.NewMessage(batchID, batchIDPayload(t, batchID), partitionKey, nil) delivery := consumermock.NewMockDelivery(ctrl) @@ -68,14 +88,14 @@ func newController(t *testing.T, store *storagemock.MockStorage, registry consum staticStorageFactory{store: store}, registry, runwaymq.TopicKeyMerge, - topickey.TopicKeyMerge, - "orchestrator-merge", + topickey.TopicKeyLand, + "orchestrator-land", ) } // publishes records what a controller run published, in order and by topic. The // controller writes to two topics now — the request-log fan-out and the runway -// merge request — so tests need to tell them apart and to see which came first. +// land request — so tests need to tell them apart and to see which came first. type publishes struct { inOrder []string byTopic map[string][]entityqueue.Message @@ -122,9 +142,9 @@ func TestNewController(t *testing.T) { c := newController(t, store, registry) require.NotNil(t, c) - assert.Equal(t, topickey.TopicKeyMerge, c.TopicKey()) - assert.Equal(t, "orchestrator-merge", c.ConsumerGroup()) - assert.Equal(t, "merge", c.Name()) + assert.Equal(t, topickey.TopicKeyLand, c.TopicKey()) + assert.Equal(t, "orchestrator-land", c.ConsumerGroup()) + assert.Equal(t, "land", c.Name()) var _ consumer.Controller = c } @@ -148,7 +168,7 @@ func TestProcess_PublishesFullPayloadToRunway(t *testing.T) { ID: batchID, Queue: "test-queue", Contains: []string{req1.ID, req2.ID}, - State: entity.BatchStateMerging, + State: entity.BatchStateLanding, Version: 4, } @@ -221,7 +241,7 @@ func TestProcess_HaltedBatchSkips(t *testing.T) { batchStore.EXPECT().Get(gomock.Any(), batchID).Return(batch, nil) // No request-store reads and no publish for a halted batch: the - // members are told nothing and runway is not asked to merge. + // members are told nothing and runway is not asked to land. store := storagemock.NewMockStorage(ctrl) store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes() @@ -236,7 +256,7 @@ func TestProcess_HaltedBatchSkips(t *testing.T) { // TestProcess_ReportsLandingBeforeDispatch covers the request-log half of this // stage: every member of the batch is told it is landing, and it is told before -// the merge request goes out. The ordering is what makes a lost log entry +// the land request goes out. The ordering is what makes a lost log entry // recoverable — a failure here nacks with nothing announced to runway, and the // redelivery re-publishes under the same occurrence, which the queue dedupes. func TestProcess_ReportsLandingBeforeDispatch(t *testing.T) { @@ -248,7 +268,7 @@ func TestProcess_ReportsLandingBeforeDispatch(t *testing.T) { batch := entity.Batch{ ID: batchID, Queue: "test-queue", Contains: []string{req1.ID, req2.ID}, - State: entity.BatchStateMerging, Version: 2, + State: entity.BatchStateLanding, Version: 2, } batchStore := storagemock.NewMockBatchStore(ctrl) @@ -298,7 +318,7 @@ func TestProcess_PublishFailureReturnsError(t *testing.T) { const batchID = "test-queue/batch/2" req := entity.Request{ID: "test-queue/1", Queue: "test-queue", LandStrategy: mergestrategy.MergeStrategyRebase} - batch := entity.Batch{ID: batchID, Queue: "test-queue", Contains: []string{req.ID}, State: entity.BatchStateMerging, Version: 1} + batch := entity.Batch{ID: batchID, Queue: "test-queue", Contains: []string{req.ID}, State: entity.BatchStateLanding, Version: 1} batchStore := storagemock.NewMockBatchStore(ctrl) batchStore.EXPECT().Get(gomock.Any(), batchID).Return(batch, nil) @@ -315,7 +335,7 @@ func TestProcess_PublishFailureReturnsError(t *testing.T) { require.Error(t, c.Process(context.Background(), newDelivery(t, ctrl, batchID, batch.Queue))) // A failed log publish must stop the run before runway hears about - // the merge, so the redelivery can repair the log entry. + // the land, so the redelivery can repair the log entry. if tt.failTopic == "log" { assert.Empty(t, rec.byTopic["runway-merge"]) } diff --git a/submitqueue/orchestrator/controller/mergeconflictsignal/BUILD.bazel b/submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel similarity index 93% rename from submitqueue/orchestrator/controller/mergeconflictsignal/BUILD.bazel rename to submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel index 94f0c2827..2a7edbab5 100644 --- a/submitqueue/orchestrator/controller/mergeconflictsignal/BUILD.bazel +++ b/submitqueue/orchestrator/controller/landconflictsignal/BUILD.bazel @@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["mergeconflictsignal.go"], - importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/mergeconflictsignal", + srcs = ["landconflictsignal.go"], + importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/landconflictsignal", visibility = ["//visibility:public"], deps = [ "//api/runway/messagequeue:go_default_library", @@ -22,7 +22,7 @@ go_library( go_test( name = "go_default_test", - srcs = ["mergeconflictsignal_test.go"], + srcs = ["landconflictsignal_test.go"], embed = [":go_default_library"], deps = [ "//api/runway/messagequeue:go_default_library", diff --git a/submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal.go b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go similarity index 86% rename from submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal.go rename to submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go index dbfb69b5c..ab3e6fe12 100644 --- a/submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal.go +++ b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go @@ -12,12 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package mergeconflictsignal consumes merge-conflict check results from runway's -// signal queue, correlates them to the request by the echoed id, and either -// advances the request to the batch stage (mergeable) or fails it (conflicted). -// Unlike buildsignal it is purely result-driven — runway pushes the result, so +// Package landconflictsignal consumes merge-conflict results from Runway's +// signal queue, correlates them to the land request by the echoed id, and either +// advances the request to the batch stage (landable) or fails it (conflicted). +// Unlike buildsignal it is purely result-driven — Runway pushes the result, so // there is no poll loop or self-reschedule. -package mergeconflictsignal +package landconflictsignal import ( "context" @@ -36,7 +36,7 @@ import ( "go.uber.org/zap" ) -// Controller handles mergeconflictsignal queue messages. Implements consumer.Controller. +// Controller handles landconflictsignal queue messages. Implements consumer.Controller. type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope @@ -49,7 +49,7 @@ type Controller struct { // Verify Controller implements consumer.Controller interface at compile time. var _ consumer.Controller = (*Controller)(nil) -// NewController creates a new mergeconflictsignal controller for the orchestrator. +// NewController creates a new landconflictsignal controller for the orchestrator. func NewController( logger *zap.SugaredLogger, scope tally.Scope, @@ -59,8 +59,8 @@ func NewController( consumerGroup string, ) *Controller { return &Controller{ - logger: logger.Named("mergeconflictsignal_controller"), - metricsScope: scope.SubScope("mergeconflictsignal_controller"), + logger: logger.Named("landconflictsignal_controller"), + metricsScope: scope.SubScope("landconflictsignal_controller"), stores: stores, registry: registry, topicKey: topicKey, @@ -71,7 +71,7 @@ func NewController( // Process consumes a runway check result and advances or fails the request. // Returns nil to ack, or error to nack/reject. // -// A not-mergeable verdict is an expected outcome of the check, not a failure: +// A not-landable verdict is an expected outcome of the check, not a failure: // the request is driven to terminal Error inline and the message is acked. Only // infrastructure faults — deserialize, storage, the terminal transition, and the // batch publish — return an error and reject to the DLQ, where the request is @@ -86,7 +86,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er result := &runwaymq.MergeResult{} if err := runwaymq.Unmarshal(msg.Payload, result); err != nil { metrics.NamedCounter(c.metricsScope, opName, "deserialize_errors", 1) - return fmt.Errorf("failed to deserialize merge conflict check result: %w", err) + return fmt.Errorf("failed to deserialize land conflict check result: %w", err) } store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) @@ -102,9 +102,9 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("failed to get request %s: %w", result.Id, err) } - c.logger.Infow("received mergeconflict signal", + c.logger.Infow("received landconflict signal", "request_id", request.ID, - "mergeable", result.Outcome == runwaypb.Outcome_SUCCEEDED, + "landable", result.Outcome == runwaypb.Outcome_SUCCEEDED, "attempt", delivery.Attempt(), "partition_key", msg.PartitionKey, ) @@ -112,7 +112,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // Short-circuit halted requests: the cancel path owns driving them terminal. if entity.IsRequestStateHalted(request.State) { metrics.NamedCounter(c.metricsScope, opName, "skipped_halted", 1) - c.logger.Infow("skipping mergeconflict signal for halted request", + c.logger.Infow("skipping landconflict signal for halted request", "request_id", request.ID, "state", string(request.State), ) @@ -120,8 +120,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } if result.Outcome != runwaypb.Outcome_SUCCEEDED { - metrics.NamedCounter(c.metricsScope, opName, "not_mergeable", 1) - c.logger.Infow("request not mergeable", + metrics.NamedCounter(c.metricsScope, opName, "not_landable", 1) + c.logger.Infow("request not landable", "request_id", request.ID, "reason", result.Reason, ) @@ -132,7 +132,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return nil } - // Advance the request to Validated now that the merge-conflict check passed. + // Advance the request to Validated now that the land-conflict check passed. newVersion := request.Version + 1 request.State = entity.RequestStateValidated if err := store.GetRequestStore().Update(ctx, request, request.Version, newVersion); err != nil { @@ -161,7 +161,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } // failRequest drives the request to terminal RequestStateError and records the -// conflict reason on the request log. A not-mergeable verdict is an expected +// conflict reason on the request log. A not-landable verdict is an expected // terminal outcome of the check, so the request is concluded here directly. // // Idempotent under at-least-once delivery: a redelivery whose request is already @@ -216,7 +216,7 @@ func (c *Controller) publishRequestID(ctx context.Context, key consumer.TopicKey // Name returns the controller name for logging and metrics. func (c *Controller) Name() string { - return "mergeconflictsignal" + return "landconflictsignal" } // TopicKey returns the topic key this controller subscribes to. diff --git a/submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal_test.go b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go similarity index 93% rename from submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal_test.go rename to submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go index c71dd9fb7..853a21deb 100644 --- a/submitqueue/orchestrator/controller/mergeconflictsignal/mergeconflictsignal_test.go +++ b/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package mergeconflictsignal +package landconflictsignal import ( "context" @@ -65,7 +65,7 @@ const ( testQueue = "test-queue" ) -func TestProcess_MergeablePublishesToBatch(t *testing.T) { +func TestProcess_LandablePublishesToBatch(t *testing.T) { ctrl := gomock.NewController(t) reqStore := storagemock.NewMockRequestStore(ctrl) @@ -97,7 +97,7 @@ func TestProcess_MergeablePublishesToBatch(t *testing.T) { require.NoError(t, err) controller := NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, registry, - runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-mergeconflictsignal") + runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-landconflictsignal") res := runwaymq.MergeResult{Id: testRequestID, Outcome: runwaypb.Outcome_SUCCEEDED} msg := entityqueue.NewMessage(testRequestID, resultPayload(t, res), testQueue, nil) @@ -119,7 +119,7 @@ func TestProcess_MergeablePublishesToBatch(t *testing.T) { assert.Equal(t, testRequestID, rid.ID) } -func TestProcess_NotMergeableMarksRequestError(t *testing.T) { +func TestProcess_NotLandableMarksRequestError(t *testing.T) { ctrl := gomock.NewController(t) reqStore := storagemock.NewMockRequestStore(ctrl) @@ -154,11 +154,11 @@ func TestProcess_NotMergeableMarksRequestError(t *testing.T) { require.NoError(t, err) controller := NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, registry, - runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-mergeconflictsignal") + runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-landconflictsignal") res := runwaymq.MergeResult{Id: testRequestID, Outcome: runwaypb.Outcome_FAILED, Reason: "conflict in foo.go"} msg := entityqueue.NewMessage(testRequestID, resultPayload(t, res), testQueue, nil) - // Not-mergeable is an expected terminal outcome, so Process acks (no error). + // Not-landable is an expected terminal outcome, so Process acks (no error). require.NoError(t, controller.Process(context.Background(), newDelivery(ctrl, msg))) // The single publish is the terminal log entry carrying the conflict reason. @@ -181,7 +181,7 @@ func TestFailRequest_UpdateFailureLeavesRequestUnchanged(t *testing.T) { store.EXPECT().GetRequestStore().Return(reqStore) controller := NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, consumer.TopicRegistry{}, - runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-mergeconflictsignal") + runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-landconflictsignal") err := controller.failRequest(context.Background(), store, request, "conflict") require.Error(t, err) @@ -209,7 +209,7 @@ func TestProcess_HaltedRequestSkips(t *testing.T) { require.NoError(t, err) controller := NewController(zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, registry, - runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-mergeconflictsignal") + runwaymq.TopicKeyMergeConflictCheckSignal, "orchestrator-landconflictsignal") res := runwaymq.MergeResult{Id: testRequestID, Outcome: runwaypb.Outcome_SUCCEEDED} msg := entityqueue.NewMessage(testRequestID, resultPayload(t, res), testQueue, nil) diff --git a/submitqueue/orchestrator/controller/mergesignal/BUILD.bazel b/submitqueue/orchestrator/controller/landsignal/BUILD.bazel similarity index 94% rename from submitqueue/orchestrator/controller/mergesignal/BUILD.bazel rename to submitqueue/orchestrator/controller/landsignal/BUILD.bazel index 9163af3db..74e0e7f04 100644 --- a/submitqueue/orchestrator/controller/mergesignal/BUILD.bazel +++ b/submitqueue/orchestrator/controller/landsignal/BUILD.bazel @@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["mergesignal.go"], - importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/mergesignal", + srcs = ["landsignal.go"], + importpath = "github.com/uber/submitqueue/submitqueue/orchestrator/controller/landsignal", visibility = ["//visibility:public"], deps = [ "//api/runway/messagequeue:go_default_library", @@ -22,7 +22,7 @@ go_library( go_test( name = "go_default_test", - srcs = ["mergesignal_test.go"], + srcs = ["landsignal_test.go"], embed = [":go_default_library"], deps = [ "//api/runway/messagequeue:go_default_library", diff --git a/submitqueue/orchestrator/controller/mergesignal/mergesignal.go b/submitqueue/orchestrator/controller/landsignal/landsignal.go similarity index 82% rename from submitqueue/orchestrator/controller/mergesignal/mergesignal.go rename to submitqueue/orchestrator/controller/landsignal/landsignal.go index 7611dd837..350dd1160 100644 --- a/submitqueue/orchestrator/controller/mergesignal/mergesignal.go +++ b/submitqueue/orchestrator/controller/landsignal/landsignal.go @@ -12,14 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package mergesignal consumes merge results from runway's merge-signal queue, +// Package landsignal consumes merge results from Runway's merge-signal queue, // correlates them to the batch by the echoed id, and transitions the batch to a -// terminal state — Succeeded when runway merged the batch, Failed when it could -// not — then fans the batch out to conclude (so member requests pick up the -// outcome) and speculate (so dependents can re-plan). Like mergeconflictsignal -// it is purely result-driven — runway pushes the result, so there is no poll +// terminal state — Succeeded when Runway's merge completed the land, Failed +// when it could not — then fans the batch out to conclude (so member requests pick up the +// outcome) and speculate (so dependents can re-plan). Like landconflictsignal +// it is purely result-driven — Runway pushes the result, so there is no poll // loop or self-reschedule. -package mergesignal +package landsignal import ( "context" @@ -38,7 +38,7 @@ import ( "go.uber.org/zap" ) -// Controller handles mergesignal queue messages. Implements consumer.Controller. +// Controller handles landsignal queue messages. Implements consumer.Controller. type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope @@ -51,7 +51,7 @@ type Controller struct { // Verify Controller implements consumer.Controller interface at compile time. var _ consumer.Controller = (*Controller)(nil) -// NewController creates a new mergesignal controller for the orchestrator. +// NewController creates a new landsignal controller for the orchestrator. func NewController( logger *zap.SugaredLogger, scope tally.Scope, @@ -61,8 +61,8 @@ func NewController( consumerGroup string, ) *Controller { return &Controller{ - logger: logger.Named("mergesignal_controller"), - metricsScope: scope.SubScope("mergesignal_controller"), + logger: logger.Named("landsignal_controller"), + metricsScope: scope.SubScope("landsignal_controller"), stores: stores, registry: registry, topicKey: topicKey, @@ -70,10 +70,10 @@ func NewController( } } -// Process consumes a runway merge result and advances or fails the batch. +// Process consumes a runway land result and advances or fails the batch. // Returns nil to ack, or error to nack/reject. // -// A not-merged verdict is an expected outcome of the merge, not a failure: the +// A not-landed verdict is an expected outcome of the land, not a failure: the // batch is driven to terminal Failed inline and the message is acked. Only // infrastructure faults — deserialize, storage, the state transition, and the // fan-out publishes — return an error and reject to the DLQ, where the batch is @@ -88,7 +88,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er result := &runwaymq.MergeResult{} if err := runwaymq.Unmarshal(msg.Payload, result); err != nil { metrics.NamedCounter(c.metricsScope, opName, "deserialize_errors", 1) - return fmt.Errorf("failed to deserialize merge result: %w", err) + return fmt.Errorf("failed to deserialize land result: %w", err) } store, err := c.stores.For(storage.Config{QueueName: result.GetQueueName()}) @@ -104,9 +104,9 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("failed to get batch %s: %w", result.Id, err) } - c.logger.Infow("received merge signal", + c.logger.Infow("received land signal", "batch_id", batch.ID, - "merged", result.Outcome == runwaypb.Outcome_SUCCEEDED, + "landed", result.Outcome == runwaypb.Outcome_SUCCEEDED, "state", string(batch.State), "version", batch.Version, "attempt", delivery.Attempt(), @@ -115,22 +115,22 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // Cancelling: the cancel path (via speculate) owns the terminal write and the // downstream fan-out for a batch the user asked to cancel. Silently ack — do - // not transition (a racing terminal merge result must not override the + // not transition (a racing terminal land result must not override the // cancel) and do not fan out. if batch.State == entity.BatchStateCancelling { metrics.NamedCounter(c.metricsScope, opName, "skipped_cancelling", 1) return nil } - // A merge failure's reason travels to conclude on the fan-out message, not on + // A land failure's reason travels to conclude on the fan-out message, not on // the batch, so it reaches the request's terminal log without becoming durable - // batch state. Empty on the merged path. Computed before the idempotency check + // batch state. Empty on the landed path. Computed before the idempotency check // so a redelivered failed batch re-fans-out with its reason intact. var failureReason string if result.Outcome != runwaypb.Outcome_SUCCEEDED { failureReason = result.Reason if failureReason == "" { - failureReason = "merge failed" + failureReason = "land failed" } } @@ -150,14 +150,14 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er var newState entity.BatchState if result.Outcome == runwaypb.Outcome_SUCCEEDED { newState = entity.BatchStateSucceeded - c.logger.Infow("merged batch", + c.logger.Infow("landed batch", "batch_id", batch.ID, "steps", result.Steps, ) } else { - metrics.NamedCounter(c.metricsScope, opName, "not_merged", 1) + metrics.NamedCounter(c.metricsScope, opName, "not_landed", 1) newState = entity.BatchStateFailed - c.logger.Warnw("batch merge failed", + c.logger.Warnw("batch land failed", "batch_id", batch.ID, "reason", result.Reason, ) @@ -175,7 +175,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er // fanout publishes the batch ID to conclude (so requests are updated) and to // speculate (so dependents can re-evaluate now that this batch is done). // -// Both messages name the merge as their cause. Without it the speculate +// Both messages name the land as their cause. Without it the speculate // publish would reuse the bare batch ID, which the batch controller already // published at creation, and the queue would drop this one as a duplicate for // as long as that row survives — leaving dependents unwoken. Conclude is @@ -187,11 +187,11 @@ func (c *Controller) fanout(ctx context.Context, batchID, queue, failureReason s if failureReason != "" { concludeMeta = map[string]string{topickey.MetadataKeyFailureReason: failureReason} } - if err := c.publish(ctx, topickey.TopicKeyConclude, publish.IntentID(batchID, "conclude", "merged"), batchID, queue, concludeMeta); err != nil { + if err := c.publish(ctx, topickey.TopicKeyConclude, publish.IntentID(batchID, "conclude", "landed"), batchID, queue, concludeMeta); err != nil { metrics.NamedCounter(c.metricsScope, "process", "publish_conclude_errors", 1) return fmt.Errorf("failed to publish to conclude: %w", err) } - if err := c.publish(ctx, topickey.TopicKeySpeculate, publish.IntentID(batchID, "merged"), batchID, queue, nil); err != nil { + if err := c.publish(ctx, topickey.TopicKeySpeculate, publish.IntentID(batchID, "landed"), batchID, queue, nil); err != nil { metrics.NamedCounter(c.metricsScope, "process", "publish_speculate_errors", 1) return fmt.Errorf("failed to publish to speculate: %w", err) } @@ -216,7 +216,7 @@ func (c *Controller) publish(ctx context.Context, key consumer.TopicKey, msgID, // Name returns the controller name for logging and metrics. func (c *Controller) Name() string { - return "mergesignal" + return "landsignal" } // TopicKey returns the topic key this controller subscribes to. diff --git a/submitqueue/orchestrator/controller/mergesignal/mergesignal_test.go b/submitqueue/orchestrator/controller/landsignal/landsignal_test.go similarity index 95% rename from submitqueue/orchestrator/controller/mergesignal/mergesignal_test.go rename to submitqueue/orchestrator/controller/landsignal/landsignal_test.go index 73b78c3e3..54626b89e 100644 --- a/submitqueue/orchestrator/controller/mergesignal/mergesignal_test.go +++ b/submitqueue/orchestrator/controller/landsignal/landsignal_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package mergesignal +package landsignal import ( "context" @@ -100,7 +100,7 @@ func newController(t *testing.T, store *storagemock.MockStorage, registry consum staticStorageFactory{store: store}, registry, runwaymq.TopicKeyMergeSignal, - "orchestrator-mergesignal", + "orchestrator-landsignal", ) } @@ -112,12 +112,12 @@ func TestNewController(t *testing.T) { c := newController(t, store, recordingRegistry(t, ctrl, &got)) assert.Equal(t, consumer.TopicKey(runwaymq.TopicKeyMergeSignal), c.TopicKey()) - assert.Equal(t, "orchestrator-mergesignal", c.ConsumerGroup()) - assert.Equal(t, "mergesignal", c.Name()) + assert.Equal(t, "orchestrator-landsignal", c.ConsumerGroup()) + assert.Equal(t, "landsignal", c.Name()) var _ consumer.Controller = c } -func TestProcess_MergedAdvancesBatch(t *testing.T) { +func TestProcess_LandedAdvancesBatch(t *testing.T) { ctrl := gomock.NewController(t) batchStore := storagemock.NewMockBatchStore(ctrl) @@ -126,7 +126,7 @@ func TestProcess_MergedAdvancesBatch(t *testing.T) { Queue: testQueue, Contains: []string{"test-queue/1"}, Dependencies: []string{"test-queue/batch/0"}, - State: entity.BatchStateMerging, + State: entity.BatchStateLanding, Version: 1, } batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return(batch, nil) @@ -152,7 +152,7 @@ func TestProcess_MergedAdvancesBatch(t *testing.T) { assert.ElementsMatch(t, []string{"conclude", "speculate"}, got) } -// The fan-out after a merge must not reuse the bare batch ID. +// The fan-out after a land must not reuse the bare batch ID. // // The batch's own announcement to speculate uses exactly that ID, and the // queue deduplicates on (topic, partition key, message ID) against every row it @@ -169,7 +169,7 @@ func TestProcess_FanoutDoesNotCollideWithTheBatchAnnouncement(t *testing.T) { Queue: testQueue, Contains: []string{"test-queue/1"}, Dependencies: []string{"test-queue/batch/0"}, - State: entity.BatchStateMerging, + State: entity.BatchStateLanding, Version: 1, } batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return(batch, nil) @@ -205,7 +205,7 @@ func TestProcess_FanoutDoesNotCollideWithTheBatchAnnouncement(t *testing.T) { assert.NotEqual(t, byTopic["speculate"], byTopic["conclude"]) } -func TestProcess_NotMergedMarksBatchFailed(t *testing.T) { +func TestProcess_NotLandedMarksBatchFailed(t *testing.T) { ctrl := gomock.NewController(t) batchStore := storagemock.NewMockBatchStore(ctrl) @@ -214,7 +214,7 @@ func TestProcess_NotMergedMarksBatchFailed(t *testing.T) { Queue: testQueue, Contains: []string{"test-queue/1"}, Dependencies: []string{"test-queue/batch/0"}, - State: entity.BatchStateMerging, + State: entity.BatchStateLanding, Version: 3, } batchStore.EXPECT().Get(gomock.Any(), testBatchID).Return(batch, nil) @@ -244,12 +244,12 @@ func TestProcess_NotMergedMarksBatchFailed(t *testing.T) { res := runwaymq.MergeResult{Id: testBatchID, Outcome: runwaypb.Outcome_FAILED, Reason: "conflict in foo.go"} msg := entityqueue.NewMessage(testBatchID, resultPayload(t, res), testQueue, nil) - // Not-merged is an expected terminal outcome, so Process acks (no error). + // Not-landed is an expected terminal outcome, so Process acks (no error). require.NoError(t, newController(t, store, registry).Process(context.Background(), newDelivery(ctrl, msg))) require.Contains(t, byTopic, "conclude") require.Contains(t, byTopic, "speculate") - // The merge reason rides the conclude message so conclude can stamp it on + // The land failure reason rides the conclude message so conclude can stamp it on // the request's terminal log; the speculate wake-up carries none. assert.Equal(t, "conflict in foo.go", byTopic["conclude"].Metadata[topickey.MetadataKeyFailureReason]) assert.Empty(t, byTopic["speculate"].Metadata[topickey.MetadataKeyFailureReason]) diff --git a/submitqueue/orchestrator/controller/speculate/check_test.go b/submitqueue/orchestrator/controller/speculate/check_test.go index ee2e25f5f..3700ff56a 100644 --- a/submitqueue/orchestrator/controller/speculate/check_test.go +++ b/submitqueue/orchestrator/controller/speculate/check_test.go @@ -82,9 +82,9 @@ func TestFilterProposals_Rejects(t *testing.T) { want: rejectUnknownHead, }, { - name: "head already merging", + name: "head already landing", proposal: entity.Speculation{Path: valid, Action: entity.PathActionBuild}, - snap: checkSnapshot(entity.BatchStateMerging), + snap: checkSnapshot(entity.BatchStateLanding), want: rejectHeadNotSpeculating, }, { diff --git a/submitqueue/orchestrator/controller/speculate/finalize.go b/submitqueue/orchestrator/controller/speculate/finalize.go index d823c93e5..0b08f8fa3 100644 --- a/submitqueue/orchestrator/controller/speculate/finalize.go +++ b/submitqueue/orchestrator/controller/speculate/finalize.go @@ -33,11 +33,11 @@ import ( // snap.speculating holding the heads still open to new work. // // Everything here is a fact, not a choice: a path a resolved dependency ruled -// out is dead, a head whose passed builds establish a merge verdict merges, +// out is dead, a head whose passed builds establish a land verdict lands, // and a batch the user cancelled is finished once its last build stops. // Finalizing before the Speculator is asked is what keeps its work from // being wasted — asked first, it would propose builds for a head that is -// already merging. +// already landing. // // Outcomes cascade, and the head loop runs to a fixed point to collapse a // whole cascade into this one run: @@ -55,7 +55,7 @@ import ( // Deciding the whole cascade up front and writing afterwards would enact // dependents of an outcome whose own write then lost its compare-and-swap — // and the loser of that race is not always benign: a cancellation loses -// precisely to a merge that got there first, which leaves the batch +// precisely to a land that got there first, which leaves the batch // *succeeded*, after its dependents were already failed on the assumption it // was cancelled. Committing per generation costs no extra reads — the // snapshot is read once, and the writes are ones this run makes anyway. @@ -95,11 +95,11 @@ func (c *Controller) finalize(ctx context.Context, snap *snapshot) error { } bypassed := false - if decision == outcomeMerge { + if decision == outcomeLand { // The winning path carries the head out of the queue; its // siblings cannot help it any more and are still holding CI // slots the rest of the queue could use. - winner, ok := mergeablePath(set, *snap) + winner, ok := landablePath(set, *snap) if !ok { winner, bypassed = bypassablePath(batch, set, *snap) } @@ -159,8 +159,8 @@ func (c *Controller) reportSpeculation( ) error { after, hasPassed := livePassedPath(set, snap) - // A merge is decided on the same live passed path, so an ungated report - // would claim a wait on every head that merges straight through. + // A land is decided on the same live passed path, so an ungated report + // would claim a wait on every head that lands straight through. event, path := entity.RequestEventWaiting, after switch { case hasPassed && decision == outcomeWait: @@ -195,7 +195,7 @@ func (c *Controller) reportSpeculation( // moment it is asked to be — its builds hold their CI slots until they // actually stop — so the run marks the paths, the poll loop asks the runner // to stop them, and whichever later run sees them stopped finishes the -// job. That is why cancellation is best effort, and why a merge that wins the +// job. That is why cancellation is best effort, and why a land that wins the // race still prevails. func (c *Controller) finalizeCancellations(ctx context.Context, snap *snapshot, nowMs int64) error { // TODO(respeculate-collateral): re-enqueue Land for every request in batch.Contains @@ -283,9 +283,9 @@ func (c *Controller) commitOutcome(ctx context.Context, snap *snapshot, batch en // durable — see commitOutcome — because everything concluded about the // batches stacked on this one is derived from it. // -// Only a terminal outcome is recorded. Merging is not terminal — a head +// Only a terminal outcome is recorded. Landing is not terminal — a head // stacked on this one assumed it would *succeed*, and it has not yet — so a -// merge outcome resolves nothing for anybody else. +// land outcome resolves nothing for anybody else. func (c *Controller) recordOutcome(snap *snapshot, batchID string, decision outcome) { state, terminal := decision.terminalState() if !terminal { @@ -307,8 +307,8 @@ func (c *Controller) applyOutcome(ctx context.Context, store storage.Storage, ba var state entity.BatchState switch decision { - case outcomeMerge: - state = entity.BatchStateMerging + case outcomeLand: + state = entity.BatchStateLanding case outcomeFail, outcomeCancel: state, _ = decision.terminalState() @@ -347,8 +347,8 @@ func (c *Controller) applyOutcome(ctx context.Context, store storage.Storage, ba ) switch decision { - case outcomeMerge: - if err := c.dispatchMerge(ctx, batch); err != nil { + case outcomeLand: + if err := c.dispatchLand(ctx, batch); err != nil { return true, err } @@ -362,7 +362,7 @@ func (c *Controller) applyOutcome(ctx context.Context, store storage.Storage, ba } // Named for the run that decided it, so a redelivery re-deriving the // same outcome does not conclude the batch twice, and so it stays - // distinct from the conclude mergesignal sends for a merged batch. + // distinct from the conclude landsignal sends for a landed batch. // A conclude that goes missing is recovered by fanout, which is // deliberately un-deduplicated. if err := c.publishBatchIDWithMetadata(ctx, topickey.TopicKeyConclude, publish.IntentID(batch.ID, "conclude", "speculate"), batch.ID, batch.Queue, batch.Queue, concludeMeta); err != nil { @@ -373,11 +373,11 @@ func (c *Controller) applyOutcome(ctx context.Context, store storage.Storage, ba return true, nil } -// dispatchMerge reports speculation finished and hands the batch to the merge -// stage. The stable ID means a redelivery or the Merging self-heal dedupes -// against the request already sent instead of merging twice; the status goes +// dispatchLand reports speculation finished and hands the batch to the land +// stage. The stable ID means a redelivery or the Landing self-heal dedupes +// against the request already sent instead of landing twice; the status goes // first so it cannot be timestamped after the landing the dispatch triggers. -func (c *Controller) dispatchMerge(ctx context.Context, batch entity.Batch) error { +func (c *Controller) dispatchLand(ctx context.Context, batch entity.Batch) error { if err := corerequest.PublishBatchLogs(ctx, c.registry, batch.Queue, batch.Contains, entity.RequestStatusSpeculated, batch.ID, map[string]string{"batch_id": batch.ID}, ); err != nil { @@ -385,9 +385,9 @@ func (c *Controller) dispatchMerge(ctx context.Context, batch entity.Batch) erro return fmt.Errorf("failed to publish request logs for batch %s: %w", batch.ID, err) } - if err := c.publishBatchID(ctx, topickey.TopicKeyMerge, publish.IntentID(batch.ID, "merge-dispatch"), batch.ID, batch.Queue, batch.Queue); err != nil { + if err := c.publishBatchID(ctx, topickey.TopicKeyLand, publish.IntentID(batch.ID, "land-dispatch"), batch.ID, batch.Queue, batch.Queue); err != nil { metrics.NamedCounter(c.metricsScope, opName, "publish_errors", 1) - return fmt.Errorf("failed to publish batch %s to merge: %w", batch.ID, err) + return fmt.Errorf("failed to publish batch %s to land: %w", batch.ID, err) } return nil } @@ -399,7 +399,7 @@ func (c *Controller) dispatchMerge(ctx context.Context, batch entity.Batch) erro // A batch named by a message is repaired through it: a redelivery re-publishes // from one of Process's self-heal branches, and a persistent failure // dead-letters by name. A cascade-decided batch has neither, and finalize only -// walks heads still speculating — so a merged one would never reach Runway, +// walks heads still speculating — so a batch in Landing would never reach Runway, // and a terminal one would leave its requests unreconciled. // // Distinct per publish: the guarantee being bought is that a message exists at @@ -456,7 +456,7 @@ func cancelBrokenPathsInSet(set *entity.SpeculationPathSet, snap snapshot, nowMs }) } -// supersede stops every path other than the winner once the head can merge. +// supersede stops every path other than the winner once the head can land. // Its live siblings cannot help any more but still hold CI slots the rest of // the queue could use. func supersede(set *entity.SpeculationPathSet, winnerID string, nowMs int64) bool { diff --git a/submitqueue/orchestrator/controller/speculate/outcome.go b/submitqueue/orchestrator/controller/speculate/outcome.go index cd6c0c2c7..c7a64a525 100644 --- a/submitqueue/orchestrator/controller/speculate/outcome.go +++ b/submitqueue/orchestrator/controller/speculate/outcome.go @@ -25,10 +25,10 @@ type outcome string const ( // outcomeWait means the batch's outcome is not decided yet. outcomeWait outcome = "wait" - // outcomeMerge means the head can be handed to the merge stage: either a + // outcomeLand means the head can be handed to the land stage: either a // passed path's assumptions have all come true, or passed paths cover every // possible outcome of its unsettled dependencies. - outcomeMerge outcome = "merge" + outcomeLand outcome = "land" // outcomeFail means no future remains in which the head could pass. outcomeFail outcome = "fail" // outcomeCancel means a batch the user asked to cancel has had every path @@ -37,8 +37,8 @@ const ( ) // terminalState returns the batch state an outcome writes, and whether the -// outcome leaves the batch terminal. Merge is the odd one out: it hands the -// batch to the merge stage, which owns the terminal write that follows. +// outcome leaves the batch terminal. Land is the odd one out: it hands the +// batch to the land stage, which owns the terminal write that follows. func (v outcome) terminalState() (entity.BatchState, bool) { switch v { case outcomeFail: @@ -52,11 +52,11 @@ func (v outcome) terminalState() (entity.BatchState, bool) { // decide returns the run's outcome on one open head, from the snapshot alone. func decide(head entity.Batch, set entity.SpeculationPathSet, snap snapshot) outcome { - if _, ok := mergeablePath(set, snap); ok { - return outcomeMerge + if _, ok := landablePath(set, snap); ok { + return outcomeLand } if _, ok := bypassablePath(head, set, snap); ok { - return outcomeMerge + return outcomeLand } if hasNoViableFuture(head, set, snap) { return outcomeFail @@ -64,23 +64,23 @@ func decide(head entity.Batch, set entity.SpeculationPathSet, snap snapshot) out return outcomeWait } -// mergeablePath returns a passed path whose merge preconditions are met: every +// landablePath returns a passed path whose land preconditions are met: every // guess it made about a dependency has been borne out by that dependency's // actual state. // // This is what makes speculation pay — not by shortening the list the head // waits on, but by having already done the work. The build ran against the // guess while the dependencies were still resolving, so when they land the way -// the path assumed there is nothing left to run and the head merges at once. +// the path assumed there is nothing left to run and the head lands at once. // -// A guess that has not been settled yet is not a licence to merge, whichever +// A guess that has not been settled yet is not a licence to land, whichever // way it points. A path that assumed a dependency would fail was built without // that dependency's changes, so landing it while the dependency is still live // puts a combination on the trunk that no build ever validated — which is the -// one thing the queue exists to prevent. The dependency merging is not enough -// either: a merge can fail, so "on its way in" is still an open question, and +// one thing the queue exists to prevent. The dependency landing is not enough +// either: a land can fail, so "on its way in" is still an open question, and // the head waits for the answer. -func mergeablePath(set entity.SpeculationPathSet, snap snapshot) (entity.SpeculationPathEntry, bool) { +func landablePath(set entity.SpeculationPathSet, snap snapshot) (entity.SpeculationPathEntry, bool) { for _, entry := range set.Paths { if entry.Status != entity.SpeculationPathStatusPassed { continue @@ -115,10 +115,10 @@ func passedEntry(set entity.SpeculationPathSet) (entity.SpeculationPathEntry, bo // consistent with how its dependencies are resolving, whether or not they have // finished resolving. // -// It is mergeablePath without the settled requirement, and the difference +// It is landablePath without the settled requirement, and the difference // between the two is exactly the head's waiting room: work this head had to do // is done, and all that is left is other batches finishing. Reported rather -// than acted on — nothing may merge on a path this loose, and decide is +// than acted on — nothing may land on a path this loose, and decide is // deliberately not built on it. func livePassedPath(set entity.SpeculationPathSet, snap snapshot) (entity.SpeculationPathEntry, bool) { for _, entry := range set.Paths { diff --git a/submitqueue/orchestrator/controller/speculate/outcome_test.go b/submitqueue/orchestrator/controller/speculate/outcome_test.go index f1b6fd591..e14dced6e 100644 --- a/submitqueue/orchestrator/controller/speculate/outcome_test.go +++ b/submitqueue/orchestrator/controller/speculate/outcome_test.go @@ -30,10 +30,10 @@ func setOf(entries ...entity.SpeculationPathEntry) entity.SpeculationPathSet { return entity.SpeculationPathSet{Head: head, Paths: entries} } -// The payoff case: a head merges as soon as the dependencies its passed build +// The payoff case: a head lands as soon as the dependencies its passed build // was stacked on have landed, without waiting for the ones it was built // without or told to ignore. -func TestMergeablePath(t *testing.T) { +func TestLandablePath(t *testing.T) { const ( succeeds = entity.DependencyAssumptionSucceeds fails = entity.DependencyAssumptionFails @@ -49,23 +49,23 @@ func TestMergeablePath(t *testing.T) { // dep2 is settled the way its assumption expects throughout, so dep1 // is the only thing under test. { - name: "waits for an assumed-succeeding dependency to merge", + name: "waits for an assumed-succeeding dependency to land", assumption: [2]entity.DependencyAssumption{succeeds, fails}, dep1State: entity.BatchStateSpeculating, dep2State: entity.BatchStateFailed, want: false, }, { - name: "merges once it has", + name: "lands once it has", assumption: [2]entity.DependencyAssumption{succeeds, fails}, dep1State: entity.BatchStateSucceeded, dep2State: entity.BatchStateFailed, want: true, }, { - name: "an assumed-succeeding dependency waits out its merge", + name: "an assumed-succeeding dependency waits out its land", assumption: [2]entity.DependencyAssumption{succeeds, fails}, - dep1State: entity.BatchStateMerging, + dep1State: entity.BatchStateLanding, dep2State: entity.BatchStateFailed, want: false, }, @@ -77,9 +77,9 @@ func TestMergeablePath(t *testing.T) { want: false, }, { - name: "still waits while that dependency is merging", + name: "still waits while that dependency is landing", assumption: [2]entity.DependencyAssumption{fails, fails}, - dep1State: entity.BatchStateMerging, + dep1State: entity.BatchStateLanding, dep2State: entity.BatchStateFailed, want: false, }, @@ -91,14 +91,14 @@ func TestMergeablePath(t *testing.T) { want: false, }, { - name: "merges once it has failed", + name: "lands once it has failed", assumption: [2]entity.DependencyAssumption{fails, fails}, dep1State: entity.BatchStateFailed, dep2State: entity.BatchStateFailed, want: true, }, { - name: "merges once it has been cancelled", + name: "lands once it has been cancelled", assumption: [2]entity.DependencyAssumption{fails, fails}, dep1State: entity.BatchStateCancelled, dep2State: entity.BatchStateFailed, @@ -120,14 +120,14 @@ func TestMergeablePath(t *testing.T) { dep2 = entity.BatchStateSpeculating } set := setOf(passedPath(tt.assumption[0], tt.assumption[1])) - _, ok := mergeablePath(set, snapWith(tt.dep1State, dep2)) + _, ok := landablePath(set, snapWith(tt.dep1State, dep2)) assert.Equal(t, tt.want, ok) }) } } // A path that has not passed cannot carry the head out of the queue. -func TestMergeablePath_IgnoresUnpassedPaths(t *testing.T) { +func TestLandablePath_IgnoresUnpassedPaths(t *testing.T) { for _, status := range []entity.SpeculationPathStatus{ entity.SpeculationPathStatusPending, entity.SpeculationPathStatusBuilding, @@ -138,19 +138,19 @@ func TestMergeablePath_IgnoresUnpassedPaths(t *testing.T) { t.Run(string(status), func(t *testing.T) { set := setOf(entryFor( pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionSucceeds), status)) - _, ok := mergeablePath(set, snapWith(entity.BatchStateSucceeded, entity.BatchStateSucceeded)) + _, ok := landablePath(set, snapWith(entity.BatchStateSucceeded, entity.BatchStateSucceeded)) assert.False(t, ok) }) } } // A passed build whose assumptions reality has since contradicted is not a -// licence to merge — it verified a world that did not happen. -func TestMergeablePath_ExcludesBrokenPassedPath(t *testing.T) { +// licence to land — it verified a world that did not happen. +func TestLandablePath_ExcludesBrokenPassedPath(t *testing.T) { set := setOf(passedPath(entity.DependencyAssumptionFails, entity.DependencyAssumptionFails)) // The path was built without dep1, but dep1 landed after all. - _, ok := mergeablePath(set, snapWith(entity.BatchStateSucceeded, entity.BatchStateSpeculating)) + _, ok := landablePath(set, snapWith(entity.BatchStateSucceeded, entity.BatchStateSpeculating)) assert.False(t, ok) } @@ -180,7 +180,7 @@ func TestBypassablePath(t *testing.T) { head: headBatch, set: setOf(allPaths...), dep1State: entity.BatchStateSpeculating, - dep2State: entity.BatchStateMerging, + dep2State: entity.BatchStateLanding, want: true, }, { @@ -266,7 +266,7 @@ func TestBypassablePath(t *testing.T) { dep2State: entity.BatchStateSpeculating, }, { - name: "leaves fully settled dependencies to strict merge", + name: "leaves fully settled dependencies to strict land", head: headBatch, set: setOf(allPaths...), dep1State: entity.BatchStateSucceeded, @@ -285,10 +285,10 @@ func TestBypassablePath(t *testing.T) { } } -// livePassedPath is mergeablePath without the settled requirement, and the gap +// livePassedPath is landablePath without the settled requirement, and the gap // between the two is the head's waiting room: its own work is done and all that // remains is other batches finishing. That window is reported to the members, -// so it has to be recognised while mergeablePath still says no. +// so it has to be recognised while landablePath still says no. func TestLivePassedPath(t *testing.T) { const ( succeeds = entity.DependencyAssumptionSucceeds @@ -404,12 +404,12 @@ func TestDecide(t *testing.T) { pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionFails), entity.SpeculationPathStatusBuilding) - assert.Equal(t, outcomeMerge, decide(headBatch, setOf(passed), allResolved)) + assert.Equal(t, outcomeLand, decide(headBatch, setOf(passed), allResolved)) assert.Equal(t, outcomeFail, decide(headBatch, setOf(failed), allResolved)) assert.Equal(t, outcomeWait, decide(headBatch, setOf(building), allResolved)) // A passed path wins over a failed sibling: one way through is enough. - assert.Equal(t, outcomeMerge, decide(headBatch, setOf(failed, passed), allResolved)) + assert.Equal(t, outcomeLand, decide(headBatch, setOf(failed, passed), allResolved)) allUnresolved := snapWith(entity.BatchStateSpeculating, entity.BatchStateSpeculating) fullCoverage := setOf( @@ -418,7 +418,7 @@ func TestDecide(t *testing.T) { passedPath(entity.DependencyAssumptionFails, entity.DependencyAssumptionSucceeds), passedPath(entity.DependencyAssumptionFails, entity.DependencyAssumptionFails), ) - assert.Equal(t, outcomeMerge, decide(headBatch, fullCoverage, allUnresolved)) + assert.Equal(t, outcomeLand, decide(headBatch, fullCoverage, allUnresolved)) } // Once a path has passed, its siblings cannot help the head but are still diff --git a/submitqueue/orchestrator/controller/speculate/run_test.go b/submitqueue/orchestrator/controller/speculate/run_test.go index 2c96b74e8..29afa7745 100644 --- a/submitqueue/orchestrator/controller/speculate/run_test.go +++ b/submitqueue/orchestrator/controller/speculate/run_test.go @@ -187,7 +187,7 @@ func newRunHarness(t *testing.T, ctrl *gomock.Controller, spec *scriptedSpeculat registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{ {Key: topickey.TopicKeyBuild, Name: "build", Queue: q}, - {Key: topickey.TopicKeyMerge, Name: "submitqueue-merge", Queue: q}, + {Key: topickey.TopicKeyLand, Name: "submitqueue-land", Queue: q}, {Key: topickey.TopicKeyConclude, Name: "conclude", Queue: q}, {Key: topickey.TopicKeySpeculate, Name: "speculate", Queue: q}, {Key: topickey.TopicKeyLog, Name: "log", Queue: q}, @@ -279,20 +279,20 @@ func TestRun_PassesSnapshotToSpeculator(t *testing.T) { ctrl := gomock.NewController(t) spec := &scriptedSpeculator{} - merging := entity.Batch{ID: "q/batch/merging", Queue: "q", State: entity.BatchStateMerging, Version: 1} - h := newRunHarness(t, ctrl, spec, []entity.Batch{speculatingHead(), merging}) + landing := entity.Batch{ID: "q/batch/landing", Queue: "q", State: entity.BatchStateLanding, Version: 1} + h := newRunHarness(t, ctrl, spec, []entity.Batch{speculatingHead(), landing}) h.noBuildsDispatched() h.batches.EXPECT().Get(gomock.Any(), dep1).Return(entity.Batch{ID: dep1, State: entity.BatchStateSucceeded}, nil) h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateSpeculating}, nil) existing := entity.SpeculationPathSet{Head: head, Version: 3} h.pathSets.EXPECT().Get(gomock.Any(), head).Return(existing, nil) - h.pathSets.EXPECT().Get(gomock.Any(), merging.ID).Return(entity.SpeculationPathSet{}, storage.ErrNotFound) + h.pathSets.EXPECT().Get(gomock.Any(), landing.ID).Return(entity.SpeculationPathSet{}, storage.ErrNotFound) require.NoError(t, h.run(head)) require.Equal(t, 1, spec.calls) - assert.ElementsMatch(t, []string{dep1, dep2, head, merging.ID}, h.speculatedOver(), + assert.ElementsMatch(t, []string{dep1, dep2, head, landing.ID}, h.speculatedOver(), "every batch the run read, in no particular order") require.Len(t, spec.gotSets, 1) assert.Equal(t, int32(3), spec.gotSets[0].Version) @@ -525,7 +525,7 @@ func TestRun_DoesNotReReadFinishedPaths(t *testing.T) { spec := &scriptedSpeculator{} h := newRunHarness(t, ctrl, spec, []entity.Batch{speculatingHead()}) - // dep1 is unresolved, so the passed path cannot merge — this test is about + // dep1 is unresolved, so the passed path cannot land — this test is about // observation being skipped, not about outcomes. h.batches.EXPECT().Get(gomock.Any(), dep1).Return(entity.Batch{ID: dep1, State: entity.BatchStateSpeculating}, nil) h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateFailed}, nil) @@ -633,7 +633,7 @@ func TestRun_DecidedHeadIsNotSpeculatedOn(t *testing.T) { h := newRunHarness(t, ctrl, spec, []entity.Batch{speculatingHead()}) h.noBuildsDispatched() - // Both dependencies resolved the way the passed path assumed, so it merges. + // Both dependencies resolved the way the passed path assumed, so it lands. h.batches.EXPECT().Get(gomock.Any(), dep1).Return(entity.Batch{ID: dep1, State: entity.BatchStateSucceeded}, nil) h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateFailed}, nil) h.pathSets.EXPECT().Get(gomock.Any(), head).Return(entity.SpeculationPathSet{ @@ -642,26 +642,26 @@ func TestRun_DecidedHeadIsNotSpeculatedOn(t *testing.T) { Version: 1, }, nil) - // The head moves to merging. Its set is untouched: the only path is the + // The head moves to landing. Its set is untouched: the only path is the // winner, and no proposal was applied. h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)). + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)). Return(nil) require.NoError(t, h.run(head)) assert.Zero(t, spec.calls, "a decided head leaves nothing to speculate about") - assert.Equal(t, []string{"submitqueue-merge"}, h.published) - assert.Contains(t, h.filed, entity.QueueBatchState{Queue: "q", State: entity.BatchStateMerging, BatchID: head}, + assert.Equal(t, []string{"submitqueue-land"}, h.published) + assert.Contains(t, h.filed, entity.QueueBatchState{Queue: "q", State: entity.BatchStateLanding, BatchID: head}, "the outcome must file the head under its new state") assert.Contains(t, h.unfiled, entity.QueueBatchState{State: entity.BatchStateSpeculating, BatchID: head}, "and drop the record under the state it left") } -// The churn this ordering removes: a mergeable head must not gain a funded path +// The churn this ordering removes: a landable head must not gain a funded path // that the same run immediately cancels — the dispatch would have started CI // for work nothing waits for. -func TestRun_MergeableHeadGainsNoNewPath(t *testing.T) { +func TestRun_LandableHeadGainsNoNewPath(t *testing.T) { ctrl := gomock.NewController(t) passed := pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionFails) other := pathOver(entity.DependencyAssumptionFails, entity.DependencyAssumptionFails) @@ -689,13 +689,13 @@ func TestRun_MergeableHeadGainsNoNewPath(t *testing.T) { h.pathSets.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)). DoAndReturn(func(_ context.Context, s entity.SpeculationPathSet, _, _ int32) error { - require.Len(t, s.Paths, 2, "no path was funded for a head that is merging") + require.Len(t, s.Paths, 2, "no path was funded for a head that is landing") assert.Equal(t, entity.SpeculationPathStatusPassed, s.Paths[0].Status) assert.Equal(t, entity.SpeculationPathStatusCancelling, s.Paths[1].Status) return nil }) h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)). + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)). Return(nil) require.NoError(t, h.run(head)) @@ -705,7 +705,7 @@ func TestRun_MergeableHeadGainsNoNewPath(t *testing.T) { // The dispatch is what takes a batch out of this stage's hands, so sending it // before the write would let Runway act on an outcome a lost compare-and-swap // refused to record. -func TestRun_MergeableHeadDispatchesAfterTheStateWrite(t *testing.T) { +func TestRun_LandableHeadDispatchesAfterTheStateWrite(t *testing.T) { ctrl := gomock.NewController(t) passed := pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionSucceeds) @@ -721,21 +721,21 @@ func TestRun_MergeableHeadDispatchesAfterTheStateWrite(t *testing.T) { var publishedBeforeWrite []string h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)). + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)). DoAndReturn(func(context.Context, entity.Batch, int32, int32) error { publishedBeforeWrite = append([]string(nil), h.published...) return nil }) require.NoError(t, h.run(head)) - assert.Equal(t, []string{"submitqueue-merge"}, h.published) + assert.Equal(t, []string{"submitqueue-land"}, h.published) assert.Empty(t, publishedBeforeWrite, - "nothing may reach the merge stage before the state it acts on is written") + "nothing may reach the land stage before the state it acts on is written") } // The other half: a lost write means another writer owns the batch, so the // dispatch it would have justified is never sent. -func TestRun_MergeableHeadDispatchesNothingWhenTheStateCASLoses(t *testing.T) { +func TestRun_LandableHeadDispatchesNothingWhenTheStateCASLoses(t *testing.T) { ctrl := gomock.NewController(t) passed := pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionSucceeds) @@ -749,7 +749,7 @@ func TestRun_MergeableHeadDispatchesNothingWhenTheStateCASLoses(t *testing.T) { Version: 1, }, nil).AnyTimes() h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)). + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)). Return(storage.ErrVersionMismatch) require.NoError(t, h.run(head)) @@ -999,44 +999,44 @@ func TestRun_CancellingLostPathSetRaceSkipsTheTerminalWrite(t *testing.T) { } // The allocator rations a budget measured in occupied CI slots, and a path -// holds its slot until its build actually stops. A merging head's superseded +// holds its slot until its build actually stops. A landing head's superseded // siblings are still running, so hiding their set would let the allocator count // those slots as free and oversubscribe CI. func TestRun_SpeculatorSeesPathSetsOfNonOpenHeads(t *testing.T) { ctrl := gomock.NewController(t) spec := &scriptedSpeculator{} - merging := entity.Batch{ - ID: "q/batch/merging", Queue: "q", State: entity.BatchStateMerging, Version: 1, + landing := entity.Batch{ + ID: "q/batch/landing", Queue: "q", State: entity.BatchStateLanding, Version: 1, } open := entity.Batch{ID: head, Queue: "q", State: entity.BatchStateSpeculating, Version: 1} - h := newRunHarness(t, ctrl, spec, []entity.Batch{open, merging}) + h := newRunHarness(t, ctrl, spec, []entity.Batch{open, landing}) h.noBuildsDispatched() h.pathSets.EXPECT().Get(gomock.Any(), head). Return(entity.SpeculationPathSet{Head: head, Version: 1}, nil) - h.pathSets.EXPECT().Get(gomock.Any(), merging.ID).Return(entity.SpeculationPathSet{ - Head: merging.ID, + h.pathSets.EXPECT().Get(gomock.Any(), landing.ID).Return(entity.SpeculationPathSet{ + Head: landing.ID, Paths: []entity.SpeculationPathEntry{ {ID: "still-running", Status: entity.SpeculationPathStatusCancelling, Attempt: 1}, }, Version: 1, }, nil) - // The undispatched cancelling path is marked cancelled, so the merging head's set is + // The undispatched cancelling path is marked cancelled, so the landing head's set is // rewritten even though it is closed to new work. h.pathSets.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)). DoAndReturn(func(_ context.Context, s entity.SpeculationPathSet, _, _ int32) error { - assert.Equal(t, merging.ID, s.Head) + assert.Equal(t, landing.ID, s.Head) assert.Equal(t, entity.SpeculationPathStatusCancelled, s.Paths[0].Status) return nil }) require.NoError(t, h.run(head)) - assert.ElementsMatch(t, []entity.Batch{open, merging}, spec.gotBatches, + assert.ElementsMatch(t, []entity.Batch{open, landing}, spec.gotBatches, "a closed head is still a fact the open ones are planned against") require.Len(t, spec.gotSets, 2, "every in-flight path set counts against the budget") - assert.Equal(t, merging.ID, spec.gotSets[1].Head) + assert.Equal(t, landing.ID, spec.gotSets[1].Head) } // A queue whose only in-flight head is closed to new work still has to be @@ -1047,13 +1047,13 @@ func TestRun_PersistsObservationsWithNoOpenHead(t *testing.T) { ctrl := gomock.NewController(t) spec := &scriptedSpeculator{} - merging := entity.Batch{ - ID: "q/batch/merging", Queue: "q", State: entity.BatchStateMerging, Version: 1, + landing := entity.Batch{ + ID: "q/batch/landing", Queue: "q", State: entity.BatchStateLanding, Version: 1, } - h := newRunHarness(t, ctrl, spec, []entity.Batch{merging}) - h.pathSets.EXPECT().Get(gomock.Any(), merging.ID).Return(entity.SpeculationPathSet{ - Head: merging.ID, + h := newRunHarness(t, ctrl, spec, []entity.Batch{landing}) + h.pathSets.EXPECT().Get(gomock.Any(), landing.ID).Return(entity.SpeculationPathSet{ + Head: landing.ID, Paths: []entity.SpeculationPathEntry{ {ID: "p1", Status: entity.SpeculationPathStatusCancelling, Attempt: 1}, }, @@ -1110,9 +1110,9 @@ func TestFanout_MintsADistinctMessageIDPerPublish(t *testing.T) { assert.NotEqual(t, ids[0], ids[1]) } -// The merge dispatch is the mirror image: a batch merges once, so the ID has +// The land dispatch is the mirror image: a batch lands once, so the ID has // to be stable across the redelivery and the self-heal that both re-derive it. -func TestDispatchMerge_ReusesOneMessageIDPerBatch(t *testing.T) { +func TestDispatchLand_ReusesOneMessageIDPerBatch(t *testing.T) { ctrl := gomock.NewController(t) var ids []string @@ -1127,7 +1127,7 @@ func TestDispatchMerge_ReusesOneMessageIDPerBatch(t *testing.T) { q.EXPECT().Publisher().Return(pub).AnyTimes() registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{ - {Key: topickey.TopicKeyMerge, Name: "submitqueue-merge", Queue: q}, + {Key: topickey.TopicKeyLand, Name: "submitqueue-land", Queue: q}, }) require.NoError(t, err) @@ -1137,8 +1137,8 @@ func TestDispatchMerge_ReusesOneMessageIDPerBatch(t *testing.T) { ) batch := entity.Batch{ID: head, Queue: "q"} - require.NoError(t, c.dispatchMerge(context.Background(), batch)) - require.NoError(t, c.dispatchMerge(context.Background(), batch)) + require.NoError(t, c.dispatchLand(context.Background(), batch)) + require.NoError(t, c.dispatchLand(context.Background(), batch)) require.Len(t, ids, 2) assert.Equal(t, ids[0], ids[1]) @@ -1176,7 +1176,7 @@ func cascadePair(t *testing.T, ctrl *gomock.Controller, prerequisiteState entity // An outcome is only a fact once it commits. When the prerequisite's state write // loses its race, nothing derived from that outcome may be enacted — the winner // may have written something else entirely. A cancellation loses precisely to a -// merge that got there first, which leaves the batch succeeded, and a dependent +// land that got there first, which leaves the batch succeeded, and a dependent // broken by that success must not already have been failed on the assumption // it was cancelled. func TestRun_CascadeStopsWhenThePrerequisiteStateCASLoses(t *testing.T) { @@ -1343,26 +1343,26 @@ func TestRun_TriggerBatchNeedsNoRecoverySignal(t *testing.T) { assert.Equal(t, []string{"conclude"}, h.published) } -// Merging needs the same signal for the same reason: the write drops the batch +// Landing needs the same signal for the same reason: the write drops the batch // out of the speculating set, so nothing else would ever dispatch it. -func TestRun_CascadeDerivedBatchIsGivenARecoverySignalBeforeItMerges(t *testing.T) { +func TestRun_CascadeDerivedBatchIsGivenARecoverySignalBeforeItLands(t *testing.T) { ctrl := gomock.NewController(t) - merging := entity.Batch{ID: "q/batch/derived", Queue: "q", State: entity.BatchStateSpeculating, Version: 1} - h := newRunHarness(t, ctrl, &scriptedSpeculator{}, []entity.Batch{merging}) + landing := entity.Batch{ID: "q/batch/derived", Queue: "q", State: entity.BatchStateSpeculating, Version: 1} + h := newRunHarness(t, ctrl, &scriptedSpeculator{}, []entity.Batch{landing}) h.noBuildsDispatched() // No dependencies, so the passed path has nothing left to settle. - passedPath := entity.SpeculationPath{Head: merging.ID} - h.pathSets.EXPECT().Get(gomock.Any(), merging.ID).Return(entity.SpeculationPathSet{ - Head: merging.ID, + passedPath := entity.SpeculationPath{Head: landing.ID} + h.pathSets.EXPECT().Get(gomock.Any(), landing.ID).Return(entity.SpeculationPathSet{ + Head: landing.ID, Paths: []entity.SpeculationPathEntry{entryFor(passedPath, entity.SpeculationPathStatusPassed)}, Version: 1, }, nil).AnyTimes() var publishedBeforeWrite []string h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: merging.ID, state: entity.BatchStateMerging}, int32(1), int32(2)). + Update(gomock.Any(), updateTo{id: landing.ID, state: entity.BatchStateLanding}, int32(1), int32(2)). DoAndReturn(func(context.Context, entity.Batch, int32, int32) error { publishedBeforeWrite = append([]string(nil), h.published...) return nil @@ -1372,7 +1372,7 @@ func TestRun_CascadeDerivedBatchIsGivenARecoverySignalBeforeItMerges(t *testing. require.NoError(t, h.run(head)) assert.Equal(t, []string{"speculate"}, publishedBeforeWrite) - assert.Equal(t, []string{"speculate", "submitqueue-merge"}, h.published) + assert.Equal(t, []string{"speculate", "submitqueue-land"}, h.published) } // A cancelling path whose build is still running finishes only when CI actually @@ -1424,7 +1424,7 @@ func TestRun_ReportsPassedPathWhileWaiting(t *testing.T) { h := newRunHarness(t, ctrl, spec, []entity.Batch{memberHead()}) h.noBuildsDispatched() // dep1 has landed the way the path assumed; dep2 has not answered yet, so - // the head cannot merge but has nothing of its own left to run. + // the head cannot land but has nothing of its own left to run. h.batches.EXPECT().Get(gomock.Any(), dep1).Return(entity.Batch{ID: dep1, State: entity.BatchStateSucceeded}, nil) h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateSpeculating}, nil) @@ -1436,7 +1436,7 @@ func TestRun_ReportsPassedPathWhileWaiting(t *testing.T) { }, nil).AnyTimes() require.NoError(t, h.run(head)) - assert.Empty(t, h.published, "an unsettled head merges nowhere") + assert.Empty(t, h.published, "an unsettled head lands nowhere") require.Len(t, h.logs, 1) assert.Equal(t, "q/1", h.logs[0].RequestID) @@ -1524,10 +1524,10 @@ func TestRun_ReportsInvalidatedWhenTheDependencyFailsInTheSameRun(t *testing.T) assert.Contains(t, got, entity.RequestEventInvalidated) } -// A merge is decided on the same live passed path a wait would be reported +// A land is decided on the same live passed path a wait would be reported // from, so without the gate every landed request would carry a wait it never // had. -func TestRun_MergingHeadReportsSpeculatedAndNoWait(t *testing.T) { +func TestRun_LandingHeadReportsSpeculatedAndNoWait(t *testing.T) { ctrl := gomock.NewController(t) passed := pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionSucceeds) @@ -1541,7 +1541,7 @@ func TestRun_MergingHeadReportsSpeculatedAndNoWait(t *testing.T) { Version: 1, }, nil).AnyTimes() h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)).Return(nil) + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)).Return(nil) require.NoError(t, h.run(head)) @@ -1561,7 +1561,7 @@ func TestRun_BypassesUnsettledDependenciesWithFullCoverage(t *testing.T) { h := newRunHarness(t, ctrl, &scriptedSpeculator{}, []entity.Batch{memberHead()}) h.batches.EXPECT().Get(gomock.Any(), dep1).Return(entity.Batch{ID: dep1, State: entity.BatchStateSpeculating}, nil) - h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateMerging}, nil) + h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateLanding}, nil) h.pathSets.EXPECT().Get(gomock.Any(), head).Return(entity.SpeculationPathSet{ Head: head, Paths: []entity.SpeculationPathEntry{ @@ -1573,11 +1573,11 @@ func TestRun_BypassesUnsettledDependenciesWithFullCoverage(t *testing.T) { Version: 1, }, nil).AnyTimes() h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)).Return(nil) + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)).Return(nil) require.NoError(t, h.run(head)) - assert.Equal(t, []string{"submitqueue-merge"}, h.published) + assert.Equal(t, []string{"submitqueue-land"}, h.published) assert.Zero(t, h.spec.calls) require.Len(t, h.logs, 1) assert.Equal(t, entity.RequestStatusSpeculated, h.logs[0].Status) @@ -1587,15 +1587,15 @@ func TestRun_BypassesUnsettledDependenciesWithFullCoverage(t *testing.T) { assert.EqualValues(t, 1, counter.Value()) } -// The merge stage publishes landing as its first act on the dispatch. Both +// The land stage publishes landing as its first act on the dispatch. Both // statuses are non-terminal, so the summary is decided on timestamp alone and // a speculated sent afterwards would beat the landing it precedes. -func TestRun_SpeculatedIsReportedBeforeTheMergeDispatch(t *testing.T) { +func TestRun_SpeculatedIsReportedBeforeTheLandDispatch(t *testing.T) { ctrl := gomock.NewController(t) passed := pathOver(entity.DependencyAssumptionSucceeds, entity.DependencyAssumptionSucceeds) h := newRunHarness(t, ctrl, &scriptedSpeculator{}, []entity.Batch{memberHead()}) - h.failPublishTo("submitqueue-merge") + h.failPublishTo("submitqueue-land") h.noBuildsDispatched() h.batches.EXPECT().Get(gomock.Any(), dep1).Return(entity.Batch{ID: dep1, State: entity.BatchStateSucceeded}, nil) h.batches.EXPECT().Get(gomock.Any(), dep2).Return(entity.Batch{ID: dep2, State: entity.BatchStateSucceeded}, nil) @@ -1605,7 +1605,7 @@ func TestRun_SpeculatedIsReportedBeforeTheMergeDispatch(t *testing.T) { Version: 1, }, nil).AnyTimes() h.batches.EXPECT(). - Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateMerging}, int32(1), int32(2)).Return(nil) + Update(gomock.Any(), updateTo{id: head, state: entity.BatchStateLanding}, int32(1), int32(2)).Return(nil) require.Error(t, h.run(head)) diff --git a/submitqueue/orchestrator/controller/speculate/speculate.go b/submitqueue/orchestrator/controller/speculate/speculate.go index ec7c90425..9385e7ada 100644 --- a/submitqueue/orchestrator/controller/speculate/speculate.go +++ b/submitqueue/orchestrator/controller/speculate/speculate.go @@ -79,12 +79,12 @@ func NewController( // // - Created: the batch is admitted first, which makes it visible to the // Speculator (proposals may only target Speculating heads). Reaching an -// outcome on it in the same run is safe: a merge needs a passed path, and +// outcome on it in the same run is safe: a land needs a passed path, and // a head admitted this instant has no paths at all. // - Already terminal: its conclude publish is repeated in case a previous // one was lost — idempotent on the batch ID — and the run that follows is // how dependents learn of an outcome no run has seen yet (a batch -// finalized by another stage, e.g. the merge signal recording a landed +// finalized by another stage, e.g. the land signal recording a landed // push, was never seen breaking the paths that bet against it). // // Everything else — funding paths, cancelling broken ones, driving a @@ -137,11 +137,11 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } } - // A Merging batch has left the set finalize walks, so a message naming it + // A Landing batch has left the set finalize walks, so a message naming it // is the only thing that will look at it again. - if batch.State == entity.BatchStateMerging { - metrics.NamedCounter(c.metricsScope, opName, "self_heal_merging", 1) - if err := c.dispatchMerge(ctx, batch); err != nil { + if batch.State == entity.BatchStateLanding { + metrics.NamedCounter(c.metricsScope, opName, "self_heal_landing", 1) + if err := c.dispatchLand(ctx, batch); err != nil { return c.attributed(err, entity.BatchSubject(batch.ID)) } } @@ -209,7 +209,7 @@ func (c *Controller) fanout(ctx context.Context, batchID, queue string) error { // // Callers choose msgID, because this controller publishes for two different // kinds of reason. A hand-off that happens once in a batch's life — dispatching -// it to merge, concluding it — names its cause with publish.IntentID, so a +// it to land, concluding it — names its cause with publish.IntentID, so a // redelivery that re-derives the same decision is deduplicated instead of // enacting it twice. A repeat-until-effective nudge — a dispatch re-sent until // the build stage records it, a fan-out repeated in case an earlier one was diff --git a/submitqueue/orchestrator/controller/speculate/speculate_test.go b/submitqueue/orchestrator/controller/speculate/speculate_test.go index 1fe4ef271..19aaf5b35 100644 --- a/submitqueue/orchestrator/controller/speculate/speculate_test.go +++ b/submitqueue/orchestrator/controller/speculate/speculate_test.go @@ -155,7 +155,7 @@ func newProcHarness(t *testing.T, ctrl *gomock.Controller, publishErr error) *pr registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{ {Key: topickey.TopicKeyBuild, Name: "build", Queue: q}, - {Key: topickey.TopicKeyMerge, Name: "submitqueue-merge", Queue: q}, + {Key: topickey.TopicKeyLand, Name: "submitqueue-land", Queue: q}, {Key: topickey.TopicKeyConclude, Name: "conclude", Queue: q}, {Key: topickey.TopicKeySpeculate, Name: "speculate", Queue: q}, {Key: topickey.TopicKeyLog, Name: "log", Queue: q}, @@ -204,7 +204,7 @@ func TestProcess_AdmitsCreatedBatch(t *testing.T) { h.listsInFlight() require.NoError(t, h.process(t, ctrl, batch.ID)) - assert.Empty(t, h.published, "a batch cannot merge on the message that admitted it") + assert.Empty(t, h.published, "a batch cannot land on the message that admitted it") // Admission is the first thing a member hears after being batched: without // it the request reads "batched" for the whole of speculation. @@ -276,19 +276,19 @@ func TestProcess_TerminalReplansQueue(t *testing.T) { "the dependent must be re-planned against the terminal outcome, which it can only be weighed against if the terminal batch comes too") } -// A Merging batch has left the speculating set, so a message naming it is the +// A Landing batch has left the speculating set, so a message naming it is the // only thing that will look at it again: it re-sends the dispatch to repair // one lost after the state write. -func TestProcess_MergingSelfHeals(t *testing.T) { +func TestProcess_LandingSelfHeals(t *testing.T) { ctrl := gomock.NewController(t) h := newProcHarness(t, ctrl, nil) - batch := testBatch(entity.BatchStateMerging) + batch := testBatch(entity.BatchStateLanding) h.batches.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil) h.listsInFlight() require.NoError(t, h.process(t, ctrl, batch.ID)) - assert.Equal(t, []string{"submitqueue-merge"}, h.published) + assert.Equal(t, []string{"submitqueue-land"}, h.published) } func TestProcess_Errors(t *testing.T) { diff --git a/submitqueue/orchestrator/controller/start/start_test.go b/submitqueue/orchestrator/controller/start/start_test.go index 12b88450d..b485a3a33 100644 --- a/submitqueue/orchestrator/controller/start/start_test.go +++ b/submitqueue/orchestrator/controller/start/start_test.go @@ -176,6 +176,7 @@ func TestController_Process_AllStrategies(t *testing.T) { {"rebase", mergestrategy.MergeStrategyRebase}, {"squash rebase", mergestrategy.MergeStrategySquashRebase}, {"merge", mergestrategy.MergeStrategyMerge}, + {"promote", mergestrategy.MergeStrategyPromote}, } for _, tt := range tests { diff --git a/submitqueue/orchestrator/controller/validate/validate.go b/submitqueue/orchestrator/controller/validate/validate.go index 9eacab3cf..10bcd3575 100644 --- a/submitqueue/orchestrator/controller/validate/validate.go +++ b/submitqueue/orchestrator/controller/validate/validate.go @@ -22,7 +22,7 @@ import ( "github.com/uber-go/tally" changepb "github.com/uber/submitqueue/api/base/change/protopb" - strategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" + mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" runwaymq "github.com/uber/submitqueue/api/runway/messagequeue" "github.com/uber/submitqueue/platform/base/mergestrategy" "github.com/uber/submitqueue/platform/consumer" @@ -38,8 +38,8 @@ import ( // Controller handles validate queue messages. // It consumes requests, performs local validation checks (duplicate detection via the change store -// and change metadata fetch), then kicks off the asynchronous merge-conflict check by publishing the -// full check request to runway's merge-conflict-check queue. Validation logic is extensible to +// and change metadata fetch), then kicks off the asynchronous land-conflict check by publishing the +// full check request to Runway's merge-conflict-check queue. Validation logic is extensible to // support additional checks. Implements consumer.Controller. type Controller struct { logger *zap.SugaredLogger @@ -57,7 +57,7 @@ type Controller struct { var _ consumer.Controller = (*Controller)(nil) // NewController creates a new validate controller for the orchestrator. -// runwayTopicKey is the runway-owned topic the merge-conflict check request is +// runwayTopicKey is the runway-owned topic the land-conflict check request is // published to (TopicKeyMergeConflictCheck). // validators is an optional factory for custom validation checks; pass nil to skip. func NewController( @@ -86,7 +86,7 @@ func NewController( // Process processes a validate delivery from the queue. // Runs duplicate detection, change metadata fetch, and change claiming, then kicks off the -// asynchronous merge-conflict check by publishing the full check request to runway. +// asynchronous land-conflict check by publishing the full check request to runway. // Returns nil to ack (success or non-retryable rejection), error to nack (retry). func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) error { msg := delivery.Message() @@ -142,7 +142,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } // Report that validation has begun. This stage is not instantaneous — the - // merge-conflict check below is an async round trip to runway — so without + // land-conflict check below is an async round trip to runway — so without // this the request reads "started" for the whole of it. No occurrence: a // request is validated once, and a redelivery is a retry of that one event. logEntry := entity.NewRequestStatusLog(request.Queue, request.ID, entity.RequestStatusValidating, 0, "", nil) @@ -209,7 +209,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } // Claim each URI in the change store with its provider details. The claim is - // created here — after duplicate detection and the merge/provider checks — so a + // created here — after duplicate detection, change-provider lookup, and custom validation — so a // rejected request never leaves a claim, and the record is written once with its // details (immutable thereafter; no separate enrichment update). Create is // idempotent per (queue, uri, request_id), so redelivery is a no-op. @@ -218,8 +218,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er return fmt.Errorf("failed to claim change records for request %s: %w", request.ID, err) } - // Kick off the asynchronous merge-conflict check: hand the full check request - // to runway via its merge-conflict-check queue, keyed by the request id (the + // Kick off the asynchronous land-conflict check: hand the full check request + // to Runway via its merge-conflict-check queue, keyed by the request id (the // client-owned correlation id) so a redelivery republishes the same id and the // result correlates straight back. At validate time the check is a single step // (candidate vs target branch). @@ -234,12 +234,12 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er }, }, } - if err := c.publishMergeCheck(ctx, req); err != nil { + if err := c.publishLandConflictCheck(ctx, req); err != nil { coremetrics.NamedCounter(c.metricsScope, "process", "publish_errors", 1) return fmt.Errorf("failed to publish to runway merge-conflict-check: %w", err) } - c.logger.Infow("published merge conflict check to runway", + c.logger.Infow("published merge conflict check to Runway", "request_id", request.ID, "topic_key", c.runwayTopicKey, ) @@ -309,16 +309,16 @@ func (c *Controller) checkDuplicate(ctx context.Context, store storage.Storage, return "", nil } -// publishMergeCheck serializes the runway check request and publishes it to the -// runway merge-conflict-check topic, partitioned by queue. +// publishLandConflictCheck serializes the runway check request and publishes it to the +// Runway merge-conflict-check topic, partitioned by queue. // // The correlation ID is the message ID with no cause: a request is checked once, // so a redelivery that re-asks is meant to dedup rather than have Runway run the // same check twice. -func (c *Controller) publishMergeCheck(ctx context.Context, req *runwaymq.MergeRequest) error { +func (c *Controller) publishLandConflictCheck(ctx context.Context, req *runwaymq.MergeRequest) error { payload, err := runwaymq.Marshal(req) if err != nil { - return fmt.Errorf("failed to serialize merge conflict check request: %w", err) + return fmt.Errorf("failed to serialize land conflict check request: %w", err) } if err := publish.Message(ctx, c.registry, c.runwayTopicKey, publish.IntentID(req.GetId()), payload, req.GetQueueName()); err != nil { @@ -331,16 +331,18 @@ func (c *Controller) publishMergeCheck(ctx context.Context, req *runwaymq.MergeR // toProtoStrategy maps the shared mergestrategy.MergeStrategy entity to the proto // Strategy enum carried on the wire. An unknown strategy maps to DEFAULT, letting // runway apply the queue's configured default. -func toProtoStrategy(s mergestrategy.MergeStrategy) strategypb.Strategy { +func toProtoStrategy(s mergestrategy.MergeStrategy) mergestrategypb.Strategy { switch s { case mergestrategy.MergeStrategyRebase: - return strategypb.Strategy_REBASE + return mergestrategypb.Strategy_REBASE case mergestrategy.MergeStrategySquashRebase: - return strategypb.Strategy_SQUASH_REBASE + return mergestrategypb.Strategy_SQUASH_REBASE case mergestrategy.MergeStrategyMerge: - return strategypb.Strategy_MERGE + return mergestrategypb.Strategy_MERGE + case mergestrategy.MergeStrategyPromote: + return mergestrategypb.Strategy_PROMOTE default: - return strategypb.Strategy_DEFAULT + return mergestrategypb.Strategy_DEFAULT } } diff --git a/submitqueue/orchestrator/controller/validate/validate_test.go b/submitqueue/orchestrator/controller/validate/validate_test.go index 4778d408c..053200da2 100644 --- a/submitqueue/orchestrator/controller/validate/validate_test.go +++ b/submitqueue/orchestrator/controller/validate/validate_test.go @@ -48,6 +48,26 @@ type staticStorageFactory struct{ store storage.Storage } // For returns the fixed store aggregate for any queue. func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil } +func TestToProtoStrategy(t *testing.T) { + tests := []struct { + name string + in mergestrategy.MergeStrategy + want strategypb.Strategy + }{ + {name: "default", in: mergestrategy.MergeStrategyUnknown, want: strategypb.Strategy_DEFAULT}, + {name: "rebase", in: mergestrategy.MergeStrategyRebase, want: strategypb.Strategy_REBASE}, + {name: "squash rebase", in: mergestrategy.MergeStrategySquashRebase, want: strategypb.Strategy_SQUASH_REBASE}, + {name: "merge", in: mergestrategy.MergeStrategyMerge, want: strategypb.Strategy_MERGE}, + {name: "promote", in: mergestrategy.MergeStrategyPromote, want: strategypb.Strategy_PROMOTE}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, toProtoStrategy(tt.in)) + }) + } +} + func requestWithState(request entity.Request, state entity.RequestState) entity.Request { request.State = state return request @@ -180,7 +200,7 @@ func TestController_Process_Success(t *testing.T) { require.NoError(t, controller.Process(context.Background(), delivery)) } -// TestController_Process_PublishesCheckToRunway verifies the full merge-conflict +// TestController_Process_PublishesCheckToRunway verifies the full land-conflict // check request is published to runway's merge-conflict-check queue (keyed by // the request id, the client-owned correlation id) on the happy path. func TestController_Process_PublishesCheckToRunway(t *testing.T) { diff --git a/submitqueue/orchestrator/pipeline.go b/submitqueue/orchestrator/pipeline.go index 0e8732799..b85defbaf 100644 --- a/submitqueue/orchestrator/pipeline.go +++ b/submitqueue/orchestrator/pipeline.go @@ -43,9 +43,9 @@ import ( "github.com/uber/submitqueue/submitqueue/orchestrator/controller/conclude" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/dependencyanalysis" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/dlq" - "github.com/uber/submitqueue/submitqueue/orchestrator/controller/merge" - "github.com/uber/submitqueue/submitqueue/orchestrator/controller/mergeconflictsignal" - "github.com/uber/submitqueue/submitqueue/orchestrator/controller/mergesignal" + "github.com/uber/submitqueue/submitqueue/orchestrator/controller/land" + "github.com/uber/submitqueue/submitqueue/orchestrator/controller/landconflictsignal" + "github.com/uber/submitqueue/submitqueue/orchestrator/controller/landsignal" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/speculate" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/start" "github.com/uber/submitqueue/submitqueue/orchestrator/controller/validate" @@ -95,9 +95,9 @@ type Deps struct { // // Pipeline: // -// start → cancel → validate ⇢ (runway) ⇢ mergeconflictsignal → batch → speculate → build → buildsignal ─┐ +// start → cancel → validate ⇢ (runway) ⇢ landconflictsignal → batch → speculate → build → buildsignal ─┐ // ↑ ↘ ↻ poll │ -// │ merge → conclude │ +// │ land → conclude │ // │ │ │ // └─────┴───────────────────────┘ var Stages = []pipeline.Stage[Deps]{ @@ -139,10 +139,10 @@ var Stages = []pipeline.Stage[Deps]{ Name: "merge-conflict-check-signal", ConsumerGroup: "orchestrator", New: func(d Deps, sc pipeline.StageContext) (consumer.Controller, error) { - return mergeconflictsignal.NewController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil + return landconflictsignal.NewController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil }, DLQ: func(d Deps, sc pipeline.StageContext) (consumer.Controller, error) { - return dlq.NewDLQMergeConflictSignalController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil + return dlq.NewDLQLandConflictSignalController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil }, }, { @@ -201,11 +201,11 @@ var Stages = []pipeline.Stage[Deps]{ }, }, { - Key: topickey.TopicKeyMerge, - Name: "submitqueue-merge", + Key: topickey.TopicKeyLand, + Name: "submitqueue-land", ConsumerGroup: "orchestrator", New: func(d Deps, sc pipeline.StageContext) (consumer.Controller, error) { - return merge.NewController(d.Logger, d.Scope, d.Storage, sc.Registry, runwaymq.TopicKeyMerge, sc.TopicKey, sc.ConsumerGroup), nil + return land.NewController(d.Logger, d.Scope, d.Storage, sc.Registry, runwaymq.TopicKeyMerge, sc.TopicKey, sc.ConsumerGroup), nil }, DLQ: func(d Deps, sc pipeline.StageContext) (consumer.Controller, error) { return dlq.NewDLQBatchController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil @@ -216,10 +216,10 @@ var Stages = []pipeline.Stage[Deps]{ Name: "merge-signal", ConsumerGroup: "orchestrator", New: func(d Deps, sc pipeline.StageContext) (consumer.Controller, error) { - return mergesignal.NewController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil + return landsignal.NewController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil }, DLQ: func(d Deps, sc pipeline.StageContext) (consumer.Controller, error) { - return dlq.NewDLQMergeSignalController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil + return dlq.NewDLQLandSignalController(d.Logger, d.Scope, d.Storage, sc.Registry, sc.TopicKey, sc.ConsumerGroup), nil }, }, { @@ -257,9 +257,9 @@ var Stages = []pipeline.Stage[Deps]{ var PublishOnlyTopics = []pipeline.PublishOnlyTopic{ // Log: the orchestrator emits request-log entries; the gateway consumes them. {Key: topickey.TopicKeyLog, Name: "log"}, - // Merge-conflict check: the orchestrator publishes check requests to runway. + // Land-conflict check: the orchestrator publishes check requests to runway. {Key: runwaymq.TopicKeyMergeConflictCheck, Name: "merge-conflict-check"}, - // Merge: the orchestrator publishes merge requests to runway. + // Land: the orchestrator publishes land requests to runway. {Key: runwaymq.TopicKeyMerge, Name: "runway-merge"}, } diff --git a/test/e2e/submitqueue/git_suite_test.go b/test/e2e/submitqueue/git_suite_test.go index 321051ec0..4109c8f25 100644 --- a/test/e2e/submitqueue/git_suite_test.go +++ b/test/e2e/submitqueue/git_suite_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Hermetic end-to-end coverage of a *real* merge. +// Hermetic end-to-end coverage of a *real* land. // // The tier-1 suite (suite_test.go) runs Runway on the noop merger, so "landed" // there proves the pipeline's choreography and nothing about git. This suite @@ -21,10 +21,10 @@ // many ref updates. // // It needs no credential, no network, and no account anywhere, because none of -// that is what the merge machinery depends on — which is what lets these +// that is what the land machinery depends on — which is what lets these // assertions gate a pull request. What it deliberately cannot cover is the half // that is specific to a change provider: reading change metadata, that -// provider's CI, and a real change being marked merged. Those need a repository +// provider's CI, and a real change being marked landed. Those need a repository // and a credential, so they are exercised by hand — see doc/howto/QUICKSTART.md. package e2e_test @@ -59,7 +59,7 @@ const gitQueue = "e2e-git-queue" // configured remote, so this identifies the change rather than routing to it. const sandboxRemote = "git.example.com" -type GitMergeSuite struct { +type GitLandSuite struct { suite.Suite ctx context.Context log *testutil.TestLogger @@ -77,11 +77,11 @@ type GitMergeSuite struct { work string } -func TestGitMergeE2E(t *testing.T) { - suite.Run(t, new(GitMergeSuite)) +func TestGitLandE2E(t *testing.T) { + suite.Run(t, new(GitLandSuite)) } -func (s *GitMergeSuite) SetupSuite() { +func (s *GitLandSuite) SetupSuite() { t := s.T() s.ctx = context.Background() s.log = testutil.NewTestLogger(t) @@ -142,10 +142,10 @@ func (s *GitMergeSuite) SetupSuite() { require.NoError(t, err) s.gatewayClient = gatewaypb.NewSubmitQueueGatewayClient(conn) - s.log.Logf("git merge E2E suite ready (bare repo at %s)", s.bare) + s.log.Logf("git land E2E suite ready (bare repo at %s)", s.bare) } -func (s *GitMergeSuite) TearDownSuite() { +func (s *GitLandSuite) TearDownSuite() { if s.db != nil { s.db.Close() } @@ -156,7 +156,7 @@ func (s *GitMergeSuite) TearDownSuite() { // --- assertions against the repository itself --- -func (s *GitMergeSuite) TestLand_SingleChange_ReachesTheTargetBranch() { +func (s *GitLandSuite) TestLand_SingleChange_ReachesTheTargetBranch() { before := s.mainSHA() head := s.pushChange("feature/single", map[string]string{"single.txt": "single\n"}, "add single") @@ -171,8 +171,8 @@ func (s *GitMergeSuite) TestLand_SingleChange_ReachesTheTargetBranch() { s.Equal("single\n", s.fileOnMain("single.txt")) } -func (s *GitMergeSuite) TestLand_Stack_LandsInOrderInOneRefUpdate() { - // The property that distinguishes a submit queue from merging changes one +func (s *GitLandSuite) TestLand_Stack_LandsInOrderInOneRefUpdate() { + // The property that distinguishes a submit queue from landing changes one // at a time: a stack reaches the target as a single atomic ref update, so // no reader ever observes it half-landed. before := s.mainSHA() @@ -195,8 +195,8 @@ func (s *GitMergeSuite) TestLand_Stack_LandsInOrderInOneRefUpdate() { "the whole stack must reach the target in exactly one ref update") } -func (s *GitMergeSuite) TestLand_MovesEachChangeHeadBranchToItsLandedCommit() { - // What makes a provider mark a rebased change merged: its head branch is moved +func (s *GitLandSuite) TestLand_MovesEachChangeHeadBranchToItsLandedCommit() { + // What makes a provider mark a rebased change landed: its head branch is moved // to the commit the change became, so the head is reachable from the target. before := s.mainSHA() first := s.pushChange("feature/head-1", map[string]string{"h1.txt": "h1\n"}, "add h1") @@ -218,7 +218,7 @@ func (s *GitMergeSuite) TestLand_MovesEachChangeHeadBranchToItsLandedCommit() { s.True(s.isAncestorOfMain(s.branchSHA("feature/head-2"))) } -func (s *GitMergeSuite) TestLand_Conflict_FailsAndLeavesTheTargetUntouched() { +func (s *GitLandSuite) TestLand_Conflict_FailsAndLeavesTheTargetUntouched() { // Two changes editing the same line from the same base: the first lands, // the second cannot be replayed onto it. base := s.mainSHA() @@ -236,7 +236,7 @@ func (s *GitMergeSuite) TestLand_Conflict_FailsAndLeavesTheTargetUntouched() { s.Equal(loser, s.branchSHA("feature/conflict-b")) } -func (s *GitMergeSuite) TestLand_ResubmittedAfterLanding_IsRejectedAsStale() { +func (s *GitLandSuite) TestLand_ResubmittedAfterLanding_IsRejectedAsStale() { // Landing a change moves its head branch to the commit it became, so the // URI that was submitted no longer describes where that branch points. The // staleness check catches exactly that, which is what stops a change from @@ -259,7 +259,7 @@ func (s *GitMergeSuite) TestLand_ResubmittedAfterLanding_IsRejectedAsStale() { // land submits a request and returns its sqid. Repeated URIs are the stack, in // the order they must be applied. -func (s *GitMergeSuite) land(queue string, uris ...string) string { +func (s *GitLandSuite) land(queue string, uris ...string) string { resp, err := s.gatewayClient.Land(s.ctx, &gatewaypb.LandRequest{ Queue: queue, Change: &changepb.Change{Uris: uris}, @@ -272,7 +272,7 @@ func (s *GitMergeSuite) land(queue string, uris ...string) string { // requireStatus waits for the request to reach a terminal status and asserts // which one. Bazel's test timeout is the only deadline. -func (s *GitMergeSuite) requireStatus(sqid string, want entity.RequestStatus) { +func (s *GitLandSuite) requireStatus(sqid string, want entity.RequestStatus) { var got entity.RequestStatus pollUntil(persistPollInterval, func() bool { resp, err := s.gatewayClient.GetRequestSummaryByID(s.ctx, &gatewaypb.GetRequestSummaryByIDRequest{Sqid: sqid, Queue: gitQueue}) @@ -288,7 +288,7 @@ func (s *GitMergeSuite) requireStatus(sqid string, want entity.RequestStatus) { // uri builds the git:// change URI for a branch pinned at a commit. The ref is // percent-encoded so a branch name containing slashes stays one path segment. -func (s *GitMergeSuite) uri(branch, sha string) string { +func (s *GitLandSuite) uri(branch, sha string) string { ref := "refs/heads/" + branch return fmt.Sprintf("git://%s/sandbox/%s/%s", sandboxRemote, url.PathEscape(ref), sha) } @@ -297,7 +297,7 @@ func (s *GitMergeSuite) uri(branch, sha string) string { // stageProviderConfig copies the committed example configuration into a directory // the containers can bind-mount, and returns its path. -func (s *GitMergeSuite) stageProviderConfig() string { +func (s *GitLandSuite) stageProviderConfig() string { t := s.T() staged := t.TempDir() for _, name := range []string{"merge.yaml", "profiles.yaml"} { @@ -310,7 +310,7 @@ func (s *GitMergeSuite) stageProviderConfig() string { // seedRepository creates the bare repository Runway merges into, plus a working // clone the test authors changes in. -func (s *GitMergeSuite) seedRepository() { +func (s *GitLandSuite) seedRepository() { t := s.T() s.runGit(filepath.Dir(s.bare), "init", "--bare", "-b", "main", s.bare) // Bare repositories do not log ref updates by default, and the reflog is @@ -326,7 +326,7 @@ func (s *GitMergeSuite) seedRepository() { s.runGit(s.work, "push", "origin", "main") } -func (s *GitMergeSuite) configureWorkClone() { +func (s *GitLandSuite) configureWorkClone() { for _, kv := range [][2]string{ {"user.name", "E2E Author"}, {"user.email", "author@example.com"}, @@ -342,13 +342,13 @@ func (s *GitMergeSuite) configureWorkClone() { // pushChange authors a change branched off the current target tip and pushes // it, returning its head SHA — all a change URI ever carries. -func (s *GitMergeSuite) pushChange(branch string, files map[string]string, message string) string { +func (s *GitLandSuite) pushChange(branch string, files map[string]string, message string) string { return s.pushChangeOnto("origin/main", branch, files, message) } // pushChangeOnto is pushChange based at an explicit start point, for building a // change that stacks on another rather than on the target. -func (s *GitMergeSuite) pushChangeOnto(base, branch string, files map[string]string, message string) string { +func (s *GitLandSuite) pushChangeOnto(base, branch string, files map[string]string, message string) string { s.runGit(s.work, "fetch", "origin") s.runGit(s.work, "checkout", "-B", branch, base) for path, contents := range files { @@ -361,18 +361,18 @@ func (s *GitMergeSuite) pushChangeOnto(base, branch string, files map[string]str } // mainSHA is the current tip of the target branch on the bare repository. -func (s *GitMergeSuite) mainSHA() string { +func (s *GitLandSuite) mainSHA() string { return s.runGit(s.bare, "rev-parse", "refs/heads/main") } // branchSHA is the current tip of a change's head branch. -func (s *GitMergeSuite) branchSHA(branch string) string { +func (s *GitLandSuite) branchSHA(branch string) string { return s.runGit(s.bare, "rev-parse", "refs/heads/"+branch) } // shasSince lists the commits added to the target since a known point, oldest // first. -func (s *GitMergeSuite) shasSince(since string) []string { +func (s *GitLandSuite) shasSince(since string) []string { out := s.runGit(s.bare, "rev-list", "--reverse", since+"..refs/heads/main") return strings.Fields(out) } @@ -380,7 +380,7 @@ func (s *GitMergeSuite) shasSince(since string) []string { // subjectsSince lists the messages of the commits added to the target since a // known point, oldest first — the readable form of what landed and in what // order. -func (s *GitMergeSuite) subjectsSince(since string) []string { +func (s *GitLandSuite) subjectsSince(since string) []string { out := s.runGit(s.bare, "log", "--reverse", "--format=%s", since+"..refs/heads/main") var subjects []string for _, line := range strings.Split(out, "\n") { @@ -392,13 +392,13 @@ func (s *GitMergeSuite) subjectsSince(since string) []string { } // fileOnMain reads a file's contents at the target tip. -func (s *GitMergeSuite) fileOnMain(path string) string { +func (s *GitLandSuite) fileOnMain(path string) string { return s.runGit(s.bare, "show", "refs/heads/main:"+path) + "\n" } // isAncestorOfMain reports whether a commit is reachable from the target — the -// property a provider reads to decide a change has merged. -func (s *GitMergeSuite) isAncestorOfMain(sha string) bool { +// property a provider reads to decide a change has landed. +func (s *GitLandSuite) isAncestorOfMain(sha string) bool { cmd := exec.Command(s.git, "merge-base", "--is-ancestor", sha, "refs/heads/main") cmd.Dir = s.bare return cmd.Run() == nil @@ -407,7 +407,7 @@ func (s *GitMergeSuite) isAncestorOfMain(sha string) bool { // mainRefUpdateCount is how many times the target branch has been updated, // read from the bare repository's reflog. One land must cost exactly one, // however many changes it carried. -func (s *GitMergeSuite) mainRefUpdateCount() int { +func (s *GitLandSuite) mainRefUpdateCount() int { out := s.runGit(s.bare, "reflog", "show", "--format=%H", "refs/heads/main") count := 0 for _, line := range strings.Split(out, "\n") { @@ -420,7 +420,7 @@ func (s *GitMergeSuite) mainRefUpdateCount() int { // runGit runs the pinned git and returns its trimmed stdout, failing the test // on a non-zero exit. -func (s *GitMergeSuite) runGit(dir string, args ...string) string { +func (s *GitLandSuite) runGit(dir string, args ...string) string { s.T().Helper() cmd := exec.Command(s.git, args...) cmd.Dir = dir diff --git a/test/e2e/submitqueue/suite_test.go b/test/e2e/submitqueue/suite_test.go index 2ecc0973d..ca4b9b83b 100644 --- a/test/e2e/submitqueue/suite_test.go +++ b/test/e2e/submitqueue/suite_test.go @@ -255,7 +255,7 @@ func (s *E2EIntegrationSuite) TestConflictAnalyzerFailure_ReconcilesFromDLQ() { // TestLand_HappyPath_ReachesLanded drives a single request through the whole // pipeline to terminal success on the fully-hermetic e2e-test-queue (no // conflicts, fake build succeeds, noop runway signals SUCCEEDED for both the -// merge-conflict check and the merge). It asserts three views: the black-box +// land-conflict check and the land). It asserts three views: the black-box // terminal request summary, the public GetRequestHistoryByID timeline, and the internal RequestState // in the operating store. // @@ -315,31 +315,31 @@ func (s *E2EIntegrationSuite) TestLand_HappyPath_ReachesLanded() { "operating store should show request %s in terminal state landed", req.sqid) } -// TestDependentBatch_BypassesMergingDependency proves that a dependency still -// waiting on Runway is unresolved for strict merge but can be bypassed once +// TestDependentBatch_BypassesLandingDependency proves that a dependency still +// waiting on Runway is unresolved for strict land but can be bypassed once // passed paths cover both of its possible outcomes. -func (s *E2EIntegrationSuite) TestDependentBatch_BypassesMergingDependency() { +func (s *E2EIntegrationSuite) TestDependentBatch_BypassesLandingDependency() { t := s.T() const queue = "e2e-chain-queue" const gateGroup = "runway-merge" gateTopic := runwaymq.TopicKeyMerge.String() - s.closeGate(gateGroup, queue, "e2e: hold the lead merge so the dependent finishes building first") + s.closeGate(gateGroup, queue, "e2e: hold the lead land so the dependent finishes building first") // Reopen even if an assertion below fails, so teardown does not stop the // stack with a delivery still parked. Opening twice is a no-op. defer s.openGate(gateGroup, queue) lead := s.land(queue, "github://github.example.com/uber/e2e-chain/pull/1/abcdef0123456789abcdef0123456789abcdef01") - s.log.Logf("Landed lead request %s; awaiting its merge to park", lead.sqid) + s.log.Logf("Landed lead request %s; awaiting its land to park", lead.sqid) - // The merge request is keyed by batch, so name the batch to prove the - // parked delivery is this request's merge and not some other. + // The land request is keyed by batch, so name the batch to prove the + // parked delivery is this request's land and not some other. leadBatch := s.awaitBatchID(lead) parked := s.awaitParked(gateGroup, gateTopic, leadBatch) - assert.Equal(t, queue, parked.PartitionKey, "merge request should be partitioned by queue") + assert.Equal(t, queue, parked.PartitionKey, "land request should be partitioned by queue") - // The lead is provably stopped mid-merge. A request landed now serializes + // The lead is provably stopped mid-land. A request landed now serializes // behind it. dependent := s.land(queue, "github://github.example.com/uber/e2e-chain/pull/2/1234567890abcdef1234567890abcdef12345678") dependentBatch := s.awaitBatchID(dependent) @@ -352,13 +352,13 @@ func (s *E2EIntegrationSuite) TestDependentBatch_BypassesMergingDependency() { require.Contains(t, got.Dependencies, leadBatch, "batch %s must depend on the in-flight %s for this test to exercise anything", dependentBatch, leadBatch) - // Both paths pass while the lead is still parked. A Merging dependency is - // unresolved because its merge can fail, so the durable Merging state proves - // the dependent advanced through complete coverage rather than strict merge. - s.awaitBatchState(queue, dependentBatch, entity.BatchStateMerging) - s.log.Logf("Dependent %s bypassed merging batch %s", dependent.sqid, leadBatch) + // Both paths pass while the lead is still parked. A Landing dependency is + // unresolved because its land can fail, so the durable Landing state proves + // the dependent advanced through complete coverage rather than strict land. + s.awaitBatchState(queue, dependentBatch, entity.BatchStateLanding) + s.log.Logf("Dependent %s bypassed landing batch %s", dependent.sqid, leadBatch) - // Start: the lead merges, and its fan-out is now the only thing that can + // Start: the lead lands, and its fan-out is now the only thing that can // move the dependent. s.openGate(gateGroup, queue) s.awaitUnparked(gateGroup, gateTopic, leadBatch) @@ -370,7 +370,7 @@ func (s *E2EIntegrationSuite) TestDependentBatch_BypassesMergingDependency() { } // TestDependentBatch_BypassedHeadLandsFirst proves the bypass does not just -// dispatch a merge: the dependent lands while its dependency is still held +// dispatch a land: the dependent lands while its dependency is still held // mid-build, and only then does the dependency proceed. func (s *E2EIntegrationSuite) TestDependentBatch_BypassedHeadLandsFirst() { t := s.T() @@ -389,8 +389,8 @@ func (s *E2EIntegrationSuite) TestDependentBatch_BypassedHeadLandsFirst() { require.NotEqual(t, heldBatch, followerBatch) // The follower builds with and without the held leader while the leader's - // own build is parked; complete coverage hands it to the merge stage. - s.awaitBatchState(queue, followerBatch, entity.BatchStateMerging) + // own build is parked; complete coverage hands it to the land stage. + s.awaitBatchState(queue, followerBatch, entity.BatchStateLanding) s.awaitStatus(follower, entity.RequestStatusLanded) assert.Equal(t, entity.RequestStatusSpeculating, s.mustStatus(lead), "the follower must land while its dependency is still held") @@ -408,7 +408,7 @@ func (s *E2EIntegrationSuite) TestDependentBatch_BypassedHeadLandsFirst() { // TestDependentBatch_NoBypassWhenCoverageIsIncomplete proves the complement: // with only the "dependency succeeds" side passed, a head waits for its -// dependency to resolve and merges strictly, never dispatching ahead of it. +// dependency to resolve and lands strictly, never dispatching ahead of it. // // Partial coverage is seeded directly: the follower's batch is stranded in // Created (build held), its "succeeds" path is written as passed, and a @@ -444,15 +444,15 @@ func (s *E2EIntegrationSuite) TestDependentBatch_NoBypassWhenCoverageIsIncomplet s.awaitBatchID(trigger) // The run that admits the follower also reports the wait: its one passed - // path covers only one of the lead's two outcomes, so it cannot merge + // path covers only one of the lead's two outcomes, so it cannot land // until the lead resolves. s.awaitEvent(follower, entity.RequestEventWaiting) // "No bypass" must not be read off the follower's batch state: waiting is // a recorded event, not a moment, and the lead is ungated — it can land - // before this test looks, at which point the seeded path is mergeable - // *strictly* and the batch rightly moves to Merging. The bypass would - // show up differently: the follower merging while the lead is still + // before this test looks, at which point the seeded path is landable + // *strictly* and the batch rightly moves to Landing. The bypass would + // show up differently: the follower landing while the lead is still // unresolved. Assert the true invariant instead — the follower only lands // after the lead has landed. s.awaitStatus(lead, entity.RequestStatusLanded) @@ -677,7 +677,7 @@ func (s *E2EIntegrationSuite) TestCancel_CaughtPreBatch_NeverLands() { // A batch delivery that is retried after its ack was lost must not enrol the // request into a second batch. Both batches would be analyzed, promoted and -// admitted, and both would merge the same change. +// admitted, and both would land the same change. // // The build gate is what makes the redelivery land in the window that matters: // with the build for the first batch held, the request cannot reach a terminal diff --git a/test/integration/extension/messagequeue/mysql/queue_test.go b/test/integration/extension/messagequeue/mysql/queue_test.go index a87503019..b0fb4a6df 100644 --- a/test/integration/extension/messagequeue/mysql/queue_test.go +++ b/test/integration/extension/messagequeue/mysql/queue_test.go @@ -779,9 +779,9 @@ func (s *SQLQueueIntegrationSuite) TestDedupOutlivesConsumption() { // Naming the cause is what gets it through. require.NoError(t, publisher.Publish(s.ctx, topic, - entityqueue.NewMessage("batch-1/merged", []byte("woken"), "queue-1", nil))) + entityqueue.NewMessage("batch-1/landed", []byte("woken"), "queue-1", nil))) second := receive(t, deliveryChan) - assert.Equal(t, "batch-1/merged", second.Message().ID) + assert.Equal(t, "batch-1/landed", second.Message().ID) assert.Equal(t, []byte("woken"), second.Message().Payload) require.NoError(t, second.Ack(s.ctx)) } diff --git a/test/integration/submitqueue/extension/storage/suite.go b/test/integration/submitqueue/extension/storage/suite.go index ff40e3efb..9a3b104f7 100644 --- a/test/integration/submitqueue/extension/storage/suite.go +++ b/test/integration/submitqueue/extension/storage/suite.go @@ -319,7 +319,7 @@ func (s *StorageContractSuite) TestStorage_BatchUpdateReplacesAllNonKeyFields() emptyCollections := got emptyCollections.Contains = []string{} emptyCollections.Dependencies = []string{} - emptyCollections.State = entity.BatchStateMerging + emptyCollections.State = entity.BatchStateLanding require.NoError(t, store.Update(ctx, emptyCollections, 2, 3)) got, err = store.Get(ctx, batch.ID) @@ -329,7 +329,7 @@ func (s *StorageContractSuite) TestStorage_BatchUpdateReplacesAllNonKeyFields() assert.Empty(t, got.Contains) assert.NotNil(t, got.Dependencies) assert.Empty(t, got.Dependencies) - assert.Equal(t, entity.BatchStateMerging, got.State) + assert.Equal(t, entity.BatchStateLanding, got.State) assert.Equal(t, int32(3), got.Version) stale := got @@ -375,7 +375,7 @@ func (s *StorageContractSuite) TestStorage_QueueBatchStateRecordLifecycle() { assert.ElementsMatch(t, []entity.QueueBatchState{speculating}, got) // An empty bucket lists empty, not an error. - got, err = storeA.List(ctx, entity.BatchStateMerging) + got, err = storeA.List(ctx, entity.BatchStateLanding) require.NoError(t, err) assert.Empty(t, got) diff --git a/tool/linter/messageid/main_test.go b/tool/linter/messageid/main_test.go index 9fcc6282e..99018bafa 100644 --- a/tool/linter/messageid/main_test.go +++ b/tool/linter/messageid/main_test.go @@ -50,7 +50,7 @@ func f() { _ = messagequeue.NewMessage("id", nil, "part", nil) }`, name: "publishing through the helper passes", src: `package p import "github.com/uber/submitqueue/platform/publish" -func f() { _ = publish.IntentID("batch-1", "merged") }`, +func f() { _ = publish.IntentID("batch-1", "landed") }`, }, { name: "NewMessage of an unrelated package passes",