Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions doc/rfc/stovepipe/steps/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ For a delivery carrying request id `R`:

```
1. Load Request R from the request store.
- ErrNotFound -> retryable (process/analyze write not visible yet; redelivery converges).
- ErrNotFound -> return raw; non-retryable (storage is read-after-write consistent; see [storage README](stovepipe/extension/storage/README.md)).
- other store error -> return raw; classifier decides.

2. If R.State is terminal (superseded / recorded-green / recorded-not-green): ack and return.
Expand Down Expand Up @@ -75,7 +75,8 @@ For a delivery carrying request id `R`:

Every branch is safe under at-least-once redelivery — with SubmitQueue's posture on duplicates adopted wholesale: `build` has no pre-trigger dedup check (there is no caller-derivable key to check by; see [Alternatives considered](#alternatives-considered-for-the-build-identity)), so a redelivery that reaches step 5 starts a second, independent build, and safety comes from downstream idempotency rather than from preventing the duplicate:

- **Request not found / strategy not yet visible** — retryable; the producing stage's write is not visible on this reader yet.
- **Request not found** — non-retryable; storage's read-after-write guarantee means a miss here is a storage defect, not a lag condition to retry through.
- **Strategy not yet visible** — retryable; the producing stage's write is not visible on this reader yet.
- **Request already terminal** (step 2) — ack, no build. A redelivery after `record` finished, or after `process` superseded the head, never starts a stale build.
- **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `record`'s terminal transition is CAS-guarded, so the second verdict is a no-op. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue.
- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, and the fail-closed reconciler drives the Request terminal (see [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)).
Expand All @@ -101,9 +102,11 @@ Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.m

| Failure | Disposition | Why |
|---|---|---|
| `Request` not found (`storage.ErrNotFound`) | retryable (`errs.NewRetryableError`) | On a primary-only read this shouldn't happen in practice — `process` commits before it publishes — but the check costs nothing when the row is already visible and gives free convergence on the rare chance it isn't, same posture as `process.go`. |
| `BuildStrategy` not yet visible (step 4) | retryable (`errs.NewRetryableError`) | The producing stage's write (`process`'s CAS) may not be visible on this reader yet; redelivery converges. |
| `Trigger` | raw error; classifier decides | Deliberately left open rather than fixed either way — a runner timeout/connection is transient, a bad URI is permanent, and only a backend classifier can tell them apart. |

`Request` not found (`storage.ErrNotFound`) is **not** in this table: storage is required to be read-after-write consistent (see [storage README](stovepipe/extension/storage/README.md)), so a miss here is already the correct default (non-retryable, straight to DLQ) rather than a departure worth overriding.

Everything else — factory lookup, a malformed message, a build-store error other than `ErrAlreadyExists`, and the publish to `buildsignal` — is returned raw with no override, because the default is already correct: none of them are worth automatically replaying (a queue with no registered builder is a config error, a broken payload will never parse, and storage/queue and publish failures dead-letter and let DLQ reconciliation recover).

## Orchestrator vs. Stovepipe build
Expand Down
4 changes: 2 additions & 2 deletions doc/rfc/stovepipe/steps/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ For a delivery carrying request id `R`:

```
1. Load Request R from the request store.
- not found yet -> retryable error (ingest write not visible; redelivery converges)
- not found -> non-retryable (storage is read-after-write consistent; see [storage README](../../../../stovepipe/extension/storage/README.md)).
2. If R.State is terminal (superseded / recorded-green / recorded-not-green):
- ack and return (idempotent no-op).
3. If R.State is processing (strategy already recorded):
Expand Down Expand Up @@ -163,7 +163,7 @@ On a crash between admit and `record`, the Request stays non-terminal; visibilit
- **Re-ingest of a superseded URI.** Ingest dedups on `(Queue, URI)` and returns the existing (now terminal `superseded`) id; `process` acks it as a no-op (step 2). Correct: a URI is only superseded for a *strictly newer* head, so re-validating it is never wanted.
- **Gate closed, no newer head.** The single latest head waits for a slot until the in-flight validation completes — the steady state, not an error.
- **Head equals last-green.** `IsAncestor(lastGreen, R.URI)` with `R.URI == lastGreen` is degenerate; treat as already-green, or (simpler) run an incremental build with an empty delta. Left to `build`.
- **Queue row missing.** First head for a Queue: ingest get-or-creates the row with defaults (`in_flight_count = 0`, empty `last_green_uri`). `process` treats a missing row as retryable (ingest write not yet visible).
- **Queue row missing.** First head for a Queue: ingest get-or-creates the row with defaults (`in_flight_count = 0`, empty `last_green_uri`). `process` treats a missing row as non-retryable — storage's read-after-write guarantee means ingest's write is already visible by the time `process` reads it, so a miss is a storage defect, not lag.

## Entity model

Expand Down
43 changes: 43 additions & 0 deletions stovepipe/controller/build/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
load("@rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = ["build.go"],
importpath = "github.com/uber/submitqueue/stovepipe/controller/build",
visibility = ["//visibility:public"],
deps = [
"//platform/base/messagequeue:go_default_library",
"//platform/consumer:go_default_library",
"//platform/errs:go_default_library",
"//platform/metrics:go_default_library",
"//stovepipe/core/messagequeue:go_default_library",
"//stovepipe/entity:go_default_library",
"//stovepipe/extension/buildrunner:go_default_library",
"//stovepipe/extension/storage:go_default_library",
"@com_github_uber_go_tally//:go_default_library",
"@org_uber_go_zap//:go_default_library",
],
)

go_test(
name = "go_default_test",
srcs = ["build_test.go"],
embed = [":go_default_library"],
deps = [
"//platform/base/messagequeue:go_default_library",
"//platform/consumer:go_default_library",
"//platform/errs:go_default_library",
"//platform/extension/messagequeue/mock:go_default_library",
"//stovepipe/core/messagequeue:go_default_library",
"//stovepipe/entity:go_default_library",
"//stovepipe/extension/buildrunner:go_default_library",
"//stovepipe/extension/buildrunner/mock:go_default_library",
"//stovepipe/extension/storage:go_default_library",
"//stovepipe/extension/storage/mock:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
"@com_github_uber_go_tally//:go_default_library",
"@org_uber_go_mock//gomock:go_default_library",
"@org_uber_go_zap//:go_default_library",
],
)
193 changes: 193 additions & 0 deletions stovepipe/controller/build/build.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// Copyright (c) 2025 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Package build holds the build-stage queue controller. It consumes BuildRequest
// messages (a request id), reloads the Request, triggers the build-runner for
// the scope process already decided, persists a Build row, and publishes the
// build id to buildsignal.
package build

import (
"context"
"errors"
"fmt"

"github.com/uber-go/tally"
entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
"github.com/uber/submitqueue/platform/consumer"
"github.com/uber/submitqueue/platform/errs"
"github.com/uber/submitqueue/platform/metrics"
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
"github.com/uber/submitqueue/stovepipe/entity"
"github.com/uber/submitqueue/stovepipe/extension/buildrunner"
"github.com/uber/submitqueue/stovepipe/extension/storage"
"go.uber.org/zap"
)

// Controller consumes BuildRequest messages, reloads the referenced Request,
// triggers a build for its already-decided scope, and publishes the resulting
// build id to buildsignal. Implements consumer.Controller.
type Controller struct {
logger *zap.SugaredLogger
metricsScope tally.Scope
store storage.Storage
buildRunners buildrunner.Factory
registry consumer.TopicRegistry
topicKey consumer.TopicKey
consumerGroup string
}

// Verify Controller implements consumer.Controller interface at compile time.
var _ consumer.Controller = (*Controller)(nil)

// _opName is the metric operation name shared by every emit in this file.
const _opName = "build"

// NewController creates a new build controller.
func NewController(
logger *zap.SugaredLogger,
scope tally.Scope,
store storage.Storage,
buildRunners buildrunner.Factory,
registry consumer.TopicRegistry,
topicKey consumer.TopicKey,
consumerGroup string,
) *Controller {
return &Controller{
logger: logger.Named("build_controller"),
metricsScope: scope.SubScope("build_controller"),
store: store,
buildRunners: buildRunners,
registry: registry,
topicKey: topicKey,
consumerGroup: consumerGroup,
}
}

// Process reloads the request referenced by the delivery, triggers a build for
// its decided scope, and publishes the build id to buildsignal. Returns nil to
// ack (success) or an error to nack (retry) / reject (DLQ).
func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (retErr error) {
op := metrics.Begin(c.metricsScope, _opName)
defer func() { op.Complete(retErr) }()

msg := delivery.Message()

br := &stovepipemq.BuildRequest{}
if err := stovepipemq.Unmarshal(msg.Payload, br); err != nil {
metrics.NamedCounter(c.metricsScope, _opName, "deserialize_errors", 1)
// Non-retryable: a malformed message will never succeed regardless of retries.
return fmt.Errorf("failed to deserialize build request: %w", err)
}

request, err := c.loadRequest(ctx, br.Id)
if err != nil {
metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1)
return err
}

// A redelivery after record already finished, or after process superseded
// the head, must not start a fresh build.
if request.State.IsTerminal() {
return nil
}

buildRunner, err := c.buildRunners.For(buildrunner.Config{QueueName: request.Queue})
if err != nil {
// A queue with no registered builder is a config error.
return fmt.Errorf("BuildController failed to resolve build runner for queue %s: %w", request.Queue, err)
}

// process decided the scope; build never re-derives incremental-vs-full.
if request.BuildStrategy == entity.BuildStrategyUnknown {
metrics.NamedCounter(c.metricsScope, _opName, "strategy_not_visible", 1)
return errs.NewRetryableError(fmt.Errorf("request %s has no build strategy yet", request.ID))
}
baseURI := ""
if request.BuildStrategy == entity.BuildStrategyIncrementalSinceGreen {
baseURI = request.BaseURI
}

buildID, err := buildRunner.Trigger(ctx, baseURI, request.URI, nil)
if err != nil {
return fmt.Errorf("BuildController failed to trigger build for request %s: %w", request.ID, err)
}

build := entity.Build{
Comment thread
roychying marked this conversation as resolved.
ID: buildID.ID,
RequestID: request.ID,
Status: entity.BuildStatusAccepted,
Version: 1,
}
if err := c.store.GetBuildStore().Create(ctx, build); err != nil && !errors.Is(err, storage.ErrAlreadyExists) {
return fmt.Errorf("BuildController failed to persist build %s: %w", build.ID, err)
}

if err := c.publishBuildSignal(ctx, build.ID); err != nil {
return fmt.Errorf("BuildController failed to publish build signal for %s: %w", build.ID, err)
}

c.logger.Debugw("triggered build",
"request_id", request.ID,
"build_id", build.ID,
"queue", request.Queue,
"base_uri", baseURI,
)
return nil
}

// loadRequest returns the request for id.
func (c *Controller) loadRequest(ctx context.Context, id string) (entity.Request, error) {
got, err := c.store.GetRequestStore().Get(ctx, id)
if err != nil {
return entity.Request{}, fmt.Errorf("BuildController failed to load request %s: %w", id, err)
}
return got, nil
}

// publishBuildSignal publishes buildID to the buildsignal stage, partitioned by
// build id so each build's poll loop runs in its own partition.
func (c *Controller) publishBuildSignal(ctx context.Context, buildID string) error {
payload, err := stovepipemq.Marshal(&stovepipemq.BuildSignal{Id: buildID})
if err != nil {
return fmt.Errorf("failed to serialize build signal: %w", err)
}

msg := entityqueue.NewMessage(buildID, payload, buildID, nil)

q, ok := c.registry.Queue(stovepipemq.TopicKeyBuildSignal)
if !ok {
return fmt.Errorf("no queue registered for topic key %s", stovepipemq.TopicKeyBuildSignal)
}
topicName, ok := c.registry.TopicName(stovepipemq.TopicKeyBuildSignal)
if !ok {
return fmt.Errorf("no topic name registered for topic key %s", stovepipemq.TopicKeyBuildSignal)
}
return q.Publisher().Publish(ctx, topicName, msg)
}

// Name returns the controller name for logging and metrics.
func (c *Controller) Name() string {
return "build"
}

// TopicKey returns the topic key this controller subscribes to.
func (c *Controller) TopicKey() consumer.TopicKey {
return c.topicKey
}

// ConsumerGroup returns the consumer group for offset tracking.
func (c *Controller) ConsumerGroup() string {
return c.consumerGroup
}
Loading
Loading