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
7 changes: 5 additions & 2 deletions doc/rfc/stovepipe/request-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

## Summary

Stovepipe retains an append-only request log for each validation request. Its internal `RequestLog` is the counterpart of SubmitQueue's `RequestLog`: both retain request status changes and explanatory lifecycle events, while Stovepipe calls its materializer directly instead of sending records through a cross-service log topic. The public API presents these records as request history. The log records every durable `Request.State` transition plus asynchronous milestones needed to explain those transitions, the public verdict, and abandoned terminal-stage work:
Stovepipe retains an append-only request log for each validation request. Its internal `RequestLog` is the counterpart of SubmitQueue's `RequestLog`: both retain request status changes and explanatory lifecycle events, while Stovepipe calls its materializer directly instead of sending records through a cross-service log topic. The public API presents these records as request history. The log records every durable `Request.State` transition plus asynchronous milestones needed to explain those transitions, the public verdict, and abandoned stage work:

- `build_triggered`;
- `build_finished`;
- `validation_fact_recorded`;
- `project_facts_recorded`;
- `record_abandoned`.
- `record_abandoned`;
- `build_analysis_abandoned`.

The model deliberately follows SubmitQueue's distinction between statuses describing where a request is and events describing important activity that does not move it. It remains a bounded request-lifecycle log rather than a generic event bus or an audit of every correlated operation.

Expand Down Expand Up @@ -126,6 +127,7 @@ Immutable Request context such as URI, build strategy, base URI, and terminal bu
| `validation_fact_recorded` | The immutable whole-repository fact became durable. | Degree metadata and fact creation time |
| `project_facts_recorded` | All project validation facts returned for a request became durable. | Project-fact count metadata and completion time |
| `record_abandoned` | Record-stage work stopped after exhausting primary retries. | Event retention time |
| `build_analysis_abandoned` | Build analysis could not complete; issue observations may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time |

Build running and unchanged polls are not retained. Trigger and terminal result explain the request outcome without turning polling into an unbounded log. Individual project facts remain outside the vocabulary; one batch-completion event summarizes their durable recording.

Expand Down Expand Up @@ -214,6 +216,7 @@ Request creation, Build changes, and fact creation use the same source-write, lo
| Buildsignal | Persist terminal Build and retain `build_finished`; CAS the Request outcome and retain its terminal state. | Existing terminal Build and Request outcome each ensure their own entry before record publication. |
| Record | Create or verify the whole-repository fact, then retain `validation_fact_recorded`; after recording project facts, retain one `project_facts_recorded` event. | An identical whole-repository fact and project-fact batch ensure their events before bookmark or promotion work. |
| Record DLQ | Retain `record_abandoned`, then acknowledge the remaining record work. | The stable event ID makes history retention idempotent without replaying facts, bookmarks, promotion, or hooks. |
| Build analysis DLQ | Retain `build_analysis_abandoned` without rerunning detection or changing validation state. | One stable event per build deduplicates recovery deliveries; only state reads and history retention retry. |
| Reconciler | CAS an unrecoverable non-terminal Request to failed, then retain failed. | An existing terminal Request is repaired from its persisted outcome without relabeling it. |

Build running and unchanged polls create no entry. A failed runner trigger that creates no Build creates no event. Cancelled and superseded requests create no validation fact.
Expand Down
23 changes: 23 additions & 0 deletions doc/rfc/stovepipe/steps/buildanalysis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Build issue observation

Build analysis runs alongside status polling and recording. Hosts supply a queue-resolved detector factory. The reference service uses a no-op detector so the analysis stage is always wired without inspecting provider output; deployments replace the factory with their integration. No-op results do not establish build success or recovery.

## Start and completion notices

After persisting a Build and its trigger occurrence, build publishes to buildsignal and buildanalysis. On completion, buildsignal persists status, releases the build slot, records the Request outcome, and publishes to record and buildanalysis. Each pair of publications attempts both destinations even if one fails. Publication is not atomic: one destination can receive its message while the other publication fails.

Start and completion notices use distinct deterministic message IDs and the same build-ID partition. Retries of one notice deduplicate without suppressing the other. A held start delivery can delay the completion notice in that partition, but it also reloads Build status on every pass and can perform terminal analysis itself. Both notices may analyze the terminal Build; duplicate observations are safe.

Publication errors are returned through the producing stage's retry/DLQ policy. Analysis processing does not gate record, but notification publication is still part of the producing stage: a failed start publication can send the build-stage message to its existing fail-closed recovery path.

## Observations and polling

The [analysis controller](../../../../stovepipe/controller/buildanalysis/README.md) owns detection, per-build observation persistence, polling, and the stop condition for Requests that end without a recorded build result. It only accumulates historical observations; it does not maintain open issues or infer recovery from absence.

Status-polling DLQ recovery does not publish a build-completion notice. The analysis polling loop observes the ended Request through storage instead.

## Analysis errors and dead letters

Analysis errors are returned to the normal consumer whether the Build is running or terminal; errors do not use the two-minute polling hold. The reference service uses backend error classification and the extended retry configuration: a maximum of 10 attempts, with retry delays of 5, 10, 20, 40, then 60 seconds, capped at 60 seconds. A successful hold resets failure accounting. Non-retryable errors and exhausted retries send the message to the analysis DLQ, so final analysis is attempted rather than guaranteed to succeed.

The analysis DLQ uses the existing DLQ consumer, with its always-retryable processor and unlimited-retry subscription. The recovery controller records incomplete analysis in Request history without rerunning detection or changing the validation outcome. Only state reads and history persistence retry; unusable messages are logged and acknowledged. See [DLQ recovery](../../../../stovepipe/controller/buildanalysis/README.md#dlq-recovery) for the history identity and error-handling rules.
2 changes: 2 additions & 0 deletions doc/rfc/stovepipe/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ The pipeline is a queue-driven chain of small, single-purpose controllers, in th

> **poll for a new head → ingest → process the build strategy → build → buildsignal → record whole-repo greenness (and any project results the resolver returns) → hook.**

The [build issue observation](steps/buildanalysis.md) branch runs alongside this chain. It accumulates issues while builds run and attempts final analysis after build completion. Record does not wait for analysis to succeed. The reference host wires a no-op detector; deployments supply a real detector factory.

There is no `analyze` controller and no `analyze` topic. A later project-analysis stage that would map a target graph onto another `build` pass is design only, described under [Designed, not built](#designed-not-built-project-analysis). `record` resolves project results inline on the same delivery that writes the repository fact.

## What Stovepipe is agnostic about
Expand Down
4 changes: 4 additions & 0 deletions service/stovepipe/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_library(
"//service/stovepipe/server/handler:go_default_library",
"//stovepipe/controller:go_default_library",
"//stovepipe/controller/build:go_default_library",
"//stovepipe/controller/buildanalysis:go_default_library",
"//stovepipe/controller/buildsignal:go_default_library",
"//stovepipe/controller/dlq:go_default_library",
"//stovepipe/controller/process:go_default_library",
Expand All @@ -34,6 +35,8 @@ go_library(
"//stovepipe/core/queuelog:go_default_library",
"//stovepipe/core/queuestate:go_default_library",
"//stovepipe/core/requestlog:go_default_library",
"//stovepipe/extension/buildissue:go_default_library",
"//stovepipe/extension/buildissue/noop:go_default_library",
"//stovepipe/extension/buildrunner:go_default_library",
"//stovepipe/extension/buildrunner/fake:go_default_library",
"//stovepipe/extension/projectstatus/noop:go_default_library",
Expand Down Expand Up @@ -89,6 +92,7 @@ go_test(
"//stovepipe/controller/dlq:go_default_library",
"//stovepipe/core/messagequeue:go_default_library",
"//stovepipe/core/requestlog:go_default_library",
"//stovepipe/extension/buildissue/noop: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",
Expand Down
31 changes: 30 additions & 1 deletion service/stovepipe/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"github.com/uber/submitqueue/service/stovepipe/server/handler"
"github.com/uber/submitqueue/stovepipe/controller"
"github.com/uber/submitqueue/stovepipe/controller/build"
"github.com/uber/submitqueue/stovepipe/controller/buildanalysis"
"github.com/uber/submitqueue/stovepipe/controller/buildsignal"
"github.com/uber/submitqueue/stovepipe/controller/dlq"
"github.com/uber/submitqueue/stovepipe/controller/process"
Expand All @@ -56,6 +57,8 @@ import (
"github.com/uber/submitqueue/stovepipe/core/queuelog"
"github.com/uber/submitqueue/stovepipe/core/queuestate"
"github.com/uber/submitqueue/stovepipe/core/requestlog"
"github.com/uber/submitqueue/stovepipe/extension/buildissue"
buildissuenoop "github.com/uber/submitqueue/stovepipe/extension/buildissue/noop"
"github.com/uber/submitqueue/stovepipe/extension/buildrunner"
buildrunnerfake "github.com/uber/submitqueue/stovepipe/extension/buildrunner/fake"
projectstatusnoop "github.com/uber/submitqueue/stovepipe/extension/projectstatus/noop"
Expand Down Expand Up @@ -214,6 +217,7 @@ func run() error {
subscriberName = fmt.Sprintf("stovepipe-%d", time.Now().Unix())
}

detectorFactory := buildissuenoop.New()
registry, err := newTopicRegistry(mysqlQueue, subscriberName)
if err != nil {
return fmt.Errorf("failed to create topic registry: %w", err)
Expand Down Expand Up @@ -254,7 +258,7 @@ func run() error {
}
}
materializer := requestlog.NewMaterializer(scope)
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, storageFty, materializer, registry, sourceControl, brf, hookResolver{})
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, storageFty, materializer, registry, sourceControl, brf, hookResolver{}, detectorFactory)
if err != nil {
return err
}
Expand Down Expand Up @@ -373,6 +377,7 @@ func registerPrimaryControllers(
sourceControl sourcecontrol.Factory,
brf buildrunner.Factory,
hooks hookext.Hooks,
detectorFactory buildissue.DetectorFactory,
) (int, error) {
var count int

Expand Down Expand Up @@ -404,6 +409,12 @@ func registerPrimaryControllers(
}
count++

analysisController := buildanalysis.NewController(logger, scope, store, detectorFactory, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
if err := c.Register(analysisController); err != nil {
return count, fmt.Errorf("failed to register buildanalysis controller: %w", err)
}
count++

recordController := record.NewController(logger, scope, store, materializer, projectstatusnoop.New(), sourceControl, registry, stovepipemq.TopicKeyRecord, "stovepipe-record")
if err := c.Register(recordController); err != nil {
return count, fmt.Errorf("failed to register record controller: %w", err)
Expand Down Expand Up @@ -448,6 +459,12 @@ func registerDLQControllers(
}
count++

analysisDLQController := buildanalysis.NewDLQController(logger, scope, store, materializer, dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis), "stovepipe-buildanalysis-dlq")
if err := c.Register(analysisDLQController); err != nil {
return count, fmt.Errorf("failed to register buildanalysis dlq controller: %w", err)
}
count++

recordDLQController := record.NewDLQController(logger, scope, store, materializer, dlq.TopicKey(stovepipemq.TopicKeyRecord), "stovepipe-record-dlq")
if err := c.Register(recordDLQController); err != nil {
return count, fmt.Errorf("failed to register record dlq controller: %w", err)
Expand Down Expand Up @@ -544,6 +561,18 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
Queue: q,
Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-hook-dlq"),
},
{
Key: stovepipemq.TopicKeyBuildAnalysis,
Name: "buildanalysis",
Queue: q,
Subscription: extqueue.ExtendedSubscriptionConfig(subscriberName, "stovepipe-buildanalysis"),
},
{
Key: dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis),
Name: "buildanalysis_dlq",
Queue: q,
Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-buildanalysis-dlq"),
},
})
}

Expand Down
19 changes: 18 additions & 1 deletion service/stovepipe/server/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/uber/submitqueue/stovepipe/controller/dlq"
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
"github.com/uber/submitqueue/stovepipe/core/requestlog"
buildissuenoop "github.com/uber/submitqueue/stovepipe/extension/buildissue/noop"
"go.uber.org/zap/zaptest"
)

Expand Down Expand Up @@ -60,7 +61,7 @@ func registeredControllers(t *testing.T) (consumer.TopicRegistry, []consumer.Con
deadLetter := &recordingConsumer{}

_, err = registerPrimaryControllers(primary, logger, tally.NoopScope, store, requestlog.NewMaterializer(tally.NoopScope), registry,
fakeSourceControlFactory{}, fakeBuildRunnerFactory{}, hookResolver{})
fakeSourceControlFactory{}, fakeBuildRunnerFactory{}, hookResolver{}, buildissuenoop.New())
require.NoError(t, err)

_, err = registerDLQControllers(deadLetter, logger, tally.NoopScope, store, requestlog.NewMaterializer(tally.NoopScope))
Expand Down Expand Up @@ -129,6 +130,7 @@ func TestPipelineSubscriptionRetryPolicies(t *testing.T) {
{name: "process", topicKey: stovepipemq.TopicKeyProcess, consumerGroup: "stovepipe-process", wantRetry: defaultRetry},
{name: "build", topicKey: stovepipemq.TopicKeyBuild, consumerGroup: "stovepipe-build", wantRetry: defaultRetry},
{name: "buildsignal", topicKey: stovepipemq.TopicKeyBuildSignal, consumerGroup: "stovepipe-buildsignal", wantRetry: extendedRetry},
{name: "buildanalysis", topicKey: stovepipemq.TopicKeyBuildAnalysis, consumerGroup: "stovepipe-buildanalysis", wantRetry: extendedRetry},
{name: "record", topicKey: stovepipemq.TopicKeyRecord, consumerGroup: "stovepipe-record", wantRetry: extendedRetry},
{name: "hook", topicKey: basehook.TopicKeyHook, consumerGroup: "stovepipe-hook", wantRetry: defaultRetry},
}
Expand All @@ -154,3 +156,18 @@ func TestDLQSubscriptionRetryPoliciesRemainUnlimited(t *testing.T) {
})
}
}

func TestBuildAnalysisRegistration(t *testing.T) {
registry, primary, deadLetter := registeredControllers(t)
assert.Contains(t, topicKeys(primary), stovepipemq.TopicKeyBuildAnalysis)
assert.Contains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis))

dlqName, ok := registry.TopicName(dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis))
require.True(t, ok)
assert.Equal(t, "buildanalysis_dlq", dlqName)

cfg, ok := registry.SubscriptionConfig(stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
require.True(t, ok)
assert.True(t, cfg.DLQ.Enabled)
assert.Equal(t, "_dlq", cfg.DLQ.TopicSuffix)
}
11 changes: 10 additions & 1 deletion stovepipe/controller/build/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,28 @@ go_library(

go_test(
name = "go_default_test",
srcs = ["build_test.go"],
srcs = [
"build_analysis_test.go",
"build_test.go",
],
embed = [":go_default_library"],
deps = [
"//platform/base/messagequeue:go_default_library",
"//platform/consumer:go_default_library",
"//platform/consumer/mock:go_default_library",
"//platform/errs:go_default_library",
"//platform/errs/mysql:go_default_library",
"//platform/extension/messagequeue/mock:go_default_library",
"//platform/metrics:go_default_library",
"//platform/publish:go_default_library",
"//stovepipe/controller/buildanalysis:go_default_library",
"//stovepipe/controller/buildsignal:go_default_library",
"//stovepipe/core/messagequeue:go_default_library",
"//stovepipe/core/requestlog:go_default_library",
"//stovepipe/core/requestlog/mock:go_default_library",
"//stovepipe/entity:go_default_library",
"//stovepipe/extension/buildissue:go_default_library",
"//stovepipe/extension/buildissue/mock:go_default_library",
"//stovepipe/extension/buildrunner:go_default_library",
"//stovepipe/extension/buildrunner/mock:go_default_library",
"//stovepipe/extension/storage:go_default_library",
Expand Down
37 changes: 30 additions & 7 deletions stovepipe/controller/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// 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.
// build id to buildsignal and buildanalysis.
package build

import (
Expand All @@ -39,7 +39,7 @@ import (

// 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.
// build id to status polling and analysis. Implements consumer.Controller.
type Controller struct {
logger *zap.SugaredLogger
metricsScope tally.Scope
Expand All @@ -51,9 +51,6 @@ type Controller struct {
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"

Expand Down Expand Up @@ -155,8 +152,14 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
return err
}

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

analysisErr := c.publishBuildAnalysis(ctx, build.ID, request.Queue)
if err := errs.Group(signalErr, analysisErr); err != nil {
return err
}

c.logger.Debugw("triggered build",
Expand Down Expand Up @@ -219,3 +222,23 @@ func (c *Controller) TopicKey() consumer.TopicKey {
func (c *Controller) ConsumerGroup() string {
return c.consumerGroup
}

func (c *Controller) publishBuildAnalysis(ctx context.Context, buildID, queue string) error {
payload, err := stovepipemq.Marshal(&stovepipemq.BuildAnalysis{Id: buildID, QueueName: queue})
if err != nil {
return fmt.Errorf("failed to serialize build analysis: %w", err)
}

if err := publish.Message(ctx, c.registry, stovepipemq.TopicKeyBuildAnalysis, publish.MessageParams{
Tenant: queue,
ID: publish.IntentID(buildID, "started"),
Payload: payload,
PartitionKey: buildID,
}); err != nil {
return fmt.Errorf("failed to publish build analysis for %s: %w", buildID, err)
}

return nil
}

var _ consumer.Controller = (*Controller)(nil)
Loading
Loading