diff --git a/doc/rfc/stovepipe/request-log.md b/doc/rfc/stovepipe/request-log.md index 0b9075070..85ce1b64e 100644 --- a/doc/rfc/stovepipe/request-log.md +++ b/doc/rfc/stovepipe/request-log.md @@ -244,7 +244,7 @@ The `project_facts_recorded` and `record_abandoned` events project completion of 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. -No controller infers an outcome reason from `RequestStateFailed` alone or records DLQ as the domain reason. Record DLQ uses that persisted state to recover a missing broken repository fact; it does not infer project verdicts. +A terminal `Request` persists its outcome reason in the same compare-and-swap as its state, and every terminal state entry copies that reason. Only a Request made terminal before reasons were persisted falls back to the reason derived by the stage repairing its entry. No controller otherwise infers an outcome reason from `RequestStateFailed` alone or records DLQ as the domain reason. Record DLQ uses that persisted state to recover a missing broken repository fact; it does not infer project verdicts. ### Repair trigger requirement diff --git a/stovepipe/controller/buildsignal/buildsignal.go b/stovepipe/controller/buildsignal/buildsignal.go index 305ddaf0e..27ed8b780 100644 --- a/stovepipe/controller/buildsignal/buildsignal.go +++ b/stovepipe/controller/buildsignal/buildsignal.go @@ -255,7 +255,8 @@ func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, r return err } - if err := c.markOutcome(ctx, store, request, buildID, outcomeState(status)); err != nil { + state := outcomeState(status) + if err := c.markOutcome(ctx, store, request, buildID, state, buildOutcomeReason(state)); err != nil { metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...) return err } @@ -263,20 +264,13 @@ func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, r } func (c *Controller) persistOutcomeLog(ctx context.Context, store storage.Storage, request entity.Request) error { - var reason entity.RequestOutcomeReason - // The durable request is authoritative when duplicate builds race to record different outcomes. - switch request.State { - case entity.RequestStateSucceeded: - reason = entity.RequestOutcomeReasonBuildSucceeded - case entity.RequestStateFailed: - reason = entity.RequestOutcomeReasonBuildFailed - case entity.RequestStateCancelled: - reason = entity.RequestOutcomeReasonBuildCancelled - default: + if !request.State.HasBuildOutcome() { return fmt.Errorf("request %s has no build outcome to record", request.ID) } - log := requestlog.NewRequestStateLog(request, reason) + // The durable request is authoritative: a DLQ may have forced this outcome, or a duplicate + // build won it. The build-derived reason applies only to a request stored without one. + log := requestlog.NewRequestStateLog(request, buildOutcomeReason(request.State)) if request.TerminalBuildID != "" { log.Metadata[requestlog.MetadataKeyBuildID] = request.TerminalBuildID } @@ -300,11 +294,25 @@ func outcomeState(status entity.BuildStatus) entity.RequestState { } } -// markOutcome CAS-transitions request from processing to state, recording the -// winning build ID and retrying on version conflicts. First writer wins: once +// buildOutcomeReason is the outcome reason a build-established state records. +func buildOutcomeReason(state entity.RequestState) entity.RequestOutcomeReason { + switch state { + case entity.RequestStateSucceeded: + return entity.RequestOutcomeReasonBuildSucceeded + case entity.RequestStateFailed: + return entity.RequestOutcomeReasonBuildFailed + case entity.RequestStateCancelled: + return entity.RequestOutcomeReasonBuildCancelled + default: + return entity.RequestOutcomeReasonUnknown + } +} + +// markOutcome CAS-transitions request from processing to state and reason, recording +// the winning build ID and retrying on version conflicts. First writer wins: once // any outcome is recorded a later caller leaves it alone, so duplicate builds // for one request (which build.md accepts) cannot flip the verdict back and forth. -func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, buildID string, state entity.RequestState) error { +func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, buildID string, state entity.RequestState, reason entity.RequestOutcomeReason) error { reqStore := store.GetRequestStore() for { @@ -315,6 +323,7 @@ func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, req updated := *request updated.State = state updated.TerminalBuildID = buildID + updated.OutcomeReason = reason newVersion := request.Version + 1 if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil { if errors.Is(err, storage.ErrVersionMismatch) { diff --git a/stovepipe/controller/buildsignal/buildsignal_test.go b/stovepipe/controller/buildsignal/buildsignal_test.go index 5731831d7..597cab9ee 100644 --- a/stovepipe/controller/buildsignal/buildsignal_test.go +++ b/stovepipe/controller/buildsignal/buildsignal_test.go @@ -161,6 +161,11 @@ func requestWithState(state entity.RequestState) entity.Request { } if state.HasBuildOutcome() { request.TerminalBuildID = testBuildID + request.OutcomeReason = map[entity.RequestState]entity.RequestOutcomeReason{ + entity.RequestStateSucceeded: entity.RequestOutcomeReasonBuildSucceeded, + entity.RequestStateFailed: entity.RequestOutcomeReasonBuildFailed, + entity.RequestStateCancelled: entity.RequestOutcomeReasonBuildCancelled, + }[state] } return request } @@ -225,14 +230,9 @@ func expectFinish(m buildsignalMocks, state entity.RequestState) *gomock.Call { } func expectOutcomeLog(m buildsignalMocks, state entity.RequestState, version int32) *gomock.Call { - reason := map[entity.RequestState]entity.RequestOutcomeReason{ - entity.RequestStateSucceeded: entity.RequestOutcomeReasonBuildSucceeded, - entity.RequestStateFailed: entity.RequestOutcomeReasonBuildFailed, - entity.RequestStateCancelled: entity.RequestOutcomeReasonBuildCancelled, - }[state] request := requestWithState(state) request.Version = version - log := requestlog.NewRequestStateLog(request, reason) + log := requestlog.NewRequestStateLog(request, request.OutcomeReason) log.Metadata[requestlog.MetadataKeyBuildID] = request.TerminalBuildID return m.materializer.EXPECT().PersistLog( gomock.Any(), diff --git a/stovepipe/controller/dlq/build_test.go b/stovepipe/controller/dlq/build_test.go index c97686202..ad5772fc3 100644 --- a/stovepipe/controller/dlq/build_test.go +++ b/stovepipe/controller/dlq/build_test.go @@ -71,6 +71,7 @@ func TestBuildProcess(t *testing.T) { m.queueStore.EXPECT().Update(gomock.Any(), entity.Queue{Name: testQueue, Version: 5}, int32(5), int32(6)).Return(nil) updated := requestWithState(entity.RequestStateProcessing) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall) }, diff --git a/stovepipe/controller/dlq/buildsignal_test.go b/stovepipe/controller/dlq/buildsignal_test.go index 08c0f4aab..62a316ad2 100644 --- a/stovepipe/controller/dlq/buildsignal_test.go +++ b/stovepipe/controller/dlq/buildsignal_test.go @@ -110,6 +110,7 @@ func TestBuildSignalProcess(t *testing.T) { }, int32(5), int32(6)).Return(nil) updated := requestWithState(entity.RequestStateProcessing) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonBuildPollingExhausted updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) request := requestWithState(entity.RequestStateFailed) request.Version = 3 diff --git a/stovepipe/controller/dlq/dlq.go b/stovepipe/controller/dlq/dlq.go index 9d9a6c134..27f024bdc 100644 --- a/stovepipe/controller/dlq/dlq.go +++ b/stovepipe/controller/dlq/dlq.go @@ -102,8 +102,8 @@ func failRequest( if request.State.IsTerminal() { if request.State == entity.RequestStateFailed { - // The originating DLQ remains the retry trigger after the state write. Its stage-specific - // reason repairs that write's missing log; PersistLog rejects an already-retained conflict. + // The originating DLQ remains the retry trigger after the state write. The repaired log + // carries the reason persisted with that write, so it agrees with any log its writer retained. return persistFailureLog(ctx, store, materializer, request, reason) } logger.Infow("dlq reconcile: request already terminal, skipping", @@ -121,6 +121,7 @@ func failRequest( updated := request updated.State = entity.RequestStateFailed + updated.OutcomeReason = reason newVersion := request.Version + 1 if err := store.GetRequestStore().Update(ctx, updated, request.Version, newVersion); err != nil { return fmt.Errorf("failed to update request %s state to failed: %w", requestID, err) @@ -141,9 +142,9 @@ func persistFailureLog( store storage.Storage, materializer requestlog.Materializer, request entity.Request, - reason entity.RequestOutcomeReason, + fallbackReason entity.RequestOutcomeReason, ) error { - log := requestlog.NewRequestStateLog(request, reason) + log := requestlog.NewRequestStateLog(request, fallbackReason) if err := materializer.PersistLog(ctx, store, log); err != nil { return fmt.Errorf("failed to record failed state for request %s: %w", request.ID, err) } diff --git a/stovepipe/controller/dlq/dlq_test.go b/stovepipe/controller/dlq/dlq_test.go index eeb7c838c..8f536f67f 100644 --- a/stovepipe/controller/dlq/dlq_test.go +++ b/stovepipe/controller/dlq/dlq_test.go @@ -141,6 +141,7 @@ func TestProcess(t *testing.T) { m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateAccepted), nil) updated := requestWithState(entity.RequestStateAccepted) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall) }, @@ -157,6 +158,7 @@ func TestProcess(t *testing.T) { }, int32(5), int32(6)).Return(nil) updated := requestWithState(entity.RequestStateProcessing) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall) }, @@ -168,12 +170,21 @@ func TestProcess(t *testing.T) { }, }, { - name: "already failed repairs its missing log", + name: "already failed without a stored reason repairs its missing log with the stage reason", setup: func(m dlqMocks) { m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateFailed), nil) expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 2) }, }, + { + name: "already failed repairs its missing log with the stored reason, not the stage reason", + setup: func(m dlqMocks) { + request := requestWithState(entity.RequestStateFailed) + request.OutcomeReason = entity.RequestOutcomeReasonBuildFailed + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil) + expectFailureLog(m, entity.RequestOutcomeReasonBuildFailed, 2) + }, + }, { name: "request not found is a no-op", setup: func(m dlqMocks) { @@ -186,6 +197,7 @@ func TestProcess(t *testing.T) { m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateAccepted), nil) updated := requestWithState(entity.RequestStateAccepted) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(storage.ErrVersionMismatch) }, wantErr: true, @@ -208,6 +220,7 @@ func TestProcess(t *testing.T) { }, int32(6), int32(7)).Return(nil) updated := requestWithState(entity.RequestStateProcessing) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall) }, @@ -221,6 +234,7 @@ func TestProcess(t *testing.T) { }, nil) updated := requestWithState(entity.RequestStateProcessing) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall) }, @@ -232,6 +246,7 @@ func TestProcess(t *testing.T) { m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateAccepted), nil) updated := requestWithState(entity.RequestStateAccepted) updated.State = entity.RequestStateFailed + updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil) request := requestWithState(entity.RequestStateFailed) request.Version = 3 diff --git a/stovepipe/controller/process/process.go b/stovepipe/controller/process/process.go index 10edcf83c..1b6619c75 100644 --- a/stovepipe/controller/process/process.go +++ b/stovepipe/controller/process/process.go @@ -409,6 +409,7 @@ func (c *Controller) supersedeRequest(ctx context.Context, store storage.Storage updated := request updated.State = entity.RequestStateSuperseded + updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead newVersion := request.Version + 1 if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil { if errors.Is(err, storage.ErrVersionMismatch) { diff --git a/stovepipe/controller/process/process_test.go b/stovepipe/controller/process/process_test.go index fa87d2668..1eb39c721 100644 --- a/stovepipe/controller/process/process_test.go +++ b/stovepipe/controller/process/process_test.go @@ -770,6 +770,7 @@ func TestProcess(t *testing.T) { }, nil) updated := acceptedRequest(testOlderID) updated.State = entity.RequestStateSuperseded + updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil) loggedRequest := updated loggedRequest.Version = 2 @@ -789,6 +790,7 @@ func TestProcess(t *testing.T) { }, nil) updated := acceptedRequest(testOlderID) updated.State = entity.RequestStateSuperseded + updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil) request := entity.Request{ ID: testOlderID, Queue: testQueue, State: entity.RequestStateSuperseded, Version: 2, @@ -812,6 +814,7 @@ func TestProcess(t *testing.T) { }, nil) updated := acceptedRequest(testOlderID) updated.State = entity.RequestStateSuperseded + updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(storage.ErrVersionMismatch) reloaded := entity.Request{ ID: testOlderID, Queue: testQueue, State: entity.RequestStateSuperseded, Version: 2, @@ -936,6 +939,7 @@ func TestDelayedRegistrationDoesNotAdmitAnOlderRequest(t *testing.T) { m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(entity.Queue{Name: testQueue, LatestRequestID: "8", Version: 2}, nil) request := acceptedRequest(testID) request.State = entity.RequestStateSuperseded + request.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead m.reqStore.EXPECT().Update(gomock.Any(), request, int32(1), int32(2)).Return(nil) request.Version = 2 expectStateLog(t, m, request, entity.RequestOutcomeReasonSupersededByNewerHead) diff --git a/stovepipe/core/requestlog/materializer.go b/stovepipe/core/requestlog/materializer.go index 1034f84f1..6a96c4bbb 100644 --- a/stovepipe/core/requestlog/materializer.go +++ b/stovepipe/core/requestlog/materializer.go @@ -82,7 +82,13 @@ func NewMaterializer(scope tally.Scope) Materializer { } // NewRequestStateLog constructs the occurrence representing the request's current durable state. -func NewRequestStateLog(request entity.Request, outcomeReason entity.RequestOutcomeReason) entity.RequestLog { +// It records the request's persisted outcome reason. fallbackReason applies only when none is +// persisted: on a nonterminal request, or on one made terminal before reasons were persisted. +func NewRequestStateLog(request entity.Request, fallbackReason entity.RequestOutcomeReason) entity.RequestLog { + outcomeReason := request.OutcomeReason + if outcomeReason == entity.RequestOutcomeReasonUnknown { + outcomeReason = fallbackReason + } metadata := make(map[string]string) switch request.BuildStrategy { case entity.BuildStrategyFull: diff --git a/stovepipe/entity/request.go b/stovepipe/entity/request.go index 163aa5fa4..817dbba23 100644 --- a/stovepipe/entity/request.go +++ b/stovepipe/entity/request.go @@ -120,6 +120,9 @@ type Request struct { // TerminalBuildID is the build that established this request's terminal // state. It is empty when no build established the terminal state. TerminalBuildID string `json:"terminal_build_id"` + // OutcomeReason is the durable reason for the current terminal state. It is empty while + // the request is nonterminal and on requests made terminal before reasons were persisted. + OutcomeReason RequestOutcomeReason `json:"outcome_reason"` // Version is the version of the object. It is used for optimistic locking. // Versioning starts at 1 and is incremented for each change to the object. Version int32 `json:"version"` diff --git a/stovepipe/extension/storage/README.md b/stovepipe/extension/storage/README.md index c194e43bb..3eb70bd60 100644 --- a/stovepipe/extension/storage/README.md +++ b/stovepipe/extension/storage/README.md @@ -26,6 +26,10 @@ Same design space as [`submitqueue/extension/storage`](../../../submitqueue/exte `ProjectFactByDegreeStore` is the ordered lookup mapping for named project facts. Its immutable `(queue, URI, degree DESC, project ASC)` primary key stores the original degree and orders by descending degree and ascending bytewise project ID, so listing is a bounded key-range read. It holds no duplicate validation payload; readers hydrate facts by their original keys and verify the mapping. The record controller ensures mappings from persisted winning facts before recording project completion, including retries after partial writes. Create the table before deploying writers. The table starts empty: only results processed by index-aware writers receive mappings, with no historical population or read-time fallback. +## Requests + +Existing MySQL deployments must add `outcome_reason VARCHAR(64) NOT NULL DEFAULT ''` to `request` before deploying the new readers and writers. + ## Build issue observations `BuildIssueStore` accumulates observations identified by build, task, and unit within the bound queue. The build ID is supplied separately from the detected issue. Repeated observations return `ErrAlreadyExists`, which callers may treat as successful persistence. Build and task IDs must be nonempty; empty unit IDs represent task-wide observations. diff --git a/stovepipe/extension/storage/mysql/request_store.go b/stovepipe/extension/storage/mysql/request_store.go index 6741f004b..d00fd76b1 100644 --- a/stovepipe/extension/storage/mysql/request_store.go +++ b/stovepipe/extension/storage/mysql/request_store.go @@ -55,8 +55,8 @@ func (r *requestStore) Create(ctx context.Context, request entity.Request) (retE } _, err := r.db.ExecContext(ctx, - `INSERT INTO request (id, queue, uri, state, build_strategy, base_uri, build_id, version) - VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO request (id, queue, uri, state, build_strategy, base_uri, build_id, outcome_reason, version) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, request.ID, request.Queue, request.URI, @@ -64,6 +64,7 @@ func (r *requestStore) Create(ctx context.Context, request entity.Request) (retE request.BuildStrategy, request.BaseURI, request.BuildID, + request.OutcomeReason, request.Version, ) if err != nil { @@ -83,7 +84,7 @@ func (r *requestStore) Get(ctx context.Context, id string) (ret entity.Request, var req entity.Request err := r.db.QueryRowContext(ctx, - `SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE(terminal_build_id, ''), version + `SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE(terminal_build_id, ''), outcome_reason, version FROM request WHERE queue = ? AND id = ?`, r.queue, id, ).Scan( @@ -95,6 +96,7 @@ func (r *requestStore) Get(ctx context.Context, id string) (ret entity.Request, &req.BaseURI, &req.BuildID, &req.TerminalBuildID, + &req.OutcomeReason, &req.Version, ) @@ -109,10 +111,10 @@ func (r *requestStore) Get(ctx context.Context, id string) (ret entity.Request, } // Update persists the mutable fields of request (URI, state, build strategy, -// base URI, associated build ID, terminal build ID) if the oldVersion, writing newVersion. Returns -// ErrVersionMismatch if the stored version does not match (including when the -// request does not exist). This is a pure conditional write; the caller owns -// version arithmetic. +// base URI, associated build ID, terminal build ID, outcome reason) if the +// oldVersion, writing newVersion. Returns ErrVersionMismatch if the stored +// version does not match (including when the request does not exist). This is +// a pure conditional write; the caller owns version arithmetic. func (r *requestStore) Update(ctx context.Context, request entity.Request, oldVersion, newVersion int32) (retErr error) { op := metrics.Begin(r.scope, "update", metrics.StorageLatencyBuckets) defer func() { op.Complete(retErr) }() @@ -123,7 +125,7 @@ func (r *requestStore) Update(ctx context.Context, request entity.Request, oldVe result, err := r.db.ExecContext(ctx, `UPDATE request - SET uri = ?, state = ?, build_strategy = ?, base_uri = ?, build_id = ?, terminal_build_id = NULLIF(?, ''), version = ? + SET uri = ?, state = ?, build_strategy = ?, base_uri = ?, build_id = ?, terminal_build_id = NULLIF(?, ''), outcome_reason = ?, version = ? WHERE queue = ? AND id = ? AND version = ?`, request.URI, request.State, @@ -131,6 +133,7 @@ func (r *requestStore) Update(ctx context.Context, request entity.Request, oldVe request.BaseURI, request.BuildID, request.TerminalBuildID, + request.OutcomeReason, newVersion, request.Queue, request.ID, diff --git a/stovepipe/extension/storage/mysql/request_store_test.go b/stovepipe/extension/storage/mysql/request_store_test.go index 9c48fefe2..52ff40a4e 100644 --- a/stovepipe/extension/storage/mysql/request_store_test.go +++ b/stovepipe/extension/storage/mysql/request_store_test.go @@ -62,7 +62,7 @@ func TestRequestStore_Create(t *testing.T) { name: "success", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("INSERT INTO request"). - WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.Version). + WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.OutcomeReason, request.Version). WillReturnResult(sqlmock.NewResult(0, 1)) }, }, @@ -70,7 +70,7 @@ func TestRequestStore_Create(t *testing.T) { name: "duplicate id returns ErrAlreadyExists", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("INSERT INTO request"). - WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.Version). + WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.OutcomeReason, request.Version). WillReturnError(&mysql.MySQLError{Number: mysqlErrDuplicateEntry}) }, wantErr: true, @@ -80,7 +80,7 @@ func TestRequestStore_Create(t *testing.T) { name: "other exec error", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("INSERT INTO request"). - WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.Version). + WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.OutcomeReason, request.Version). WillReturnError(fmt.Errorf("connection reset")) }, wantErr: true, @@ -113,11 +113,12 @@ func TestRequestStore_Get(t *testing.T) { ID: "1", Queue: "monorepo/main", URI: "git://remote/monorepo/main/deadbeef", - State: entity.RequestStateProcessing, + State: entity.RequestStateFailed, BuildStrategy: entity.BuildStrategyFull, BaseURI: "", BuildID: "bk-1001", TerminalBuildID: "bk-1001", + OutcomeReason: entity.RequestOutcomeReasonBuildFailed, Version: 2, } @@ -133,9 +134,9 @@ func TestRequestStore_Get(t *testing.T) { name: "found", id: want.ID, setup: func(mock sqlmock.Sqlmock) { - rows := sqlmock.NewRows([]string{"id", "queue", "uri", "state", "build_strategy", "base_uri", "build_id", "terminal_build_id", "version"}). - AddRow(want.ID, want.Queue, want.URI, string(want.State), string(want.BuildStrategy), want.BaseURI, want.BuildID, want.TerminalBuildID, want.Version) - mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE\\(terminal_build_id, ''\\), version"). + rows := sqlmock.NewRows([]string{"id", "queue", "uri", "state", "build_strategy", "base_uri", "build_id", "terminal_build_id", "outcome_reason", "version"}). + AddRow(want.ID, want.Queue, want.URI, string(want.State), string(want.BuildStrategy), want.BaseURI, want.BuildID, want.TerminalBuildID, string(want.OutcomeReason), want.Version) + mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE\\(terminal_build_id, ''\\), outcome_reason, version"). WithArgs("monorepo/main", want.ID). WillReturnRows(rows) }, @@ -145,7 +146,7 @@ func TestRequestStore_Get(t *testing.T) { name: "not found", id: "missing", setup: func(mock sqlmock.Sqlmock) { - mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE\\(terminal_build_id, ''\\), version"). + mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE\\(terminal_build_id, ''\\), outcome_reason, version"). WithArgs("monorepo/main", "missing"). WillReturnError(sql.ErrNoRows) }, @@ -156,7 +157,7 @@ func TestRequestStore_Get(t *testing.T) { name: "query error", id: "bad", setup: func(mock sqlmock.Sqlmock) { - mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE\\(terminal_build_id, ''\\), version"). + mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, build_id, COALESCE\\(terminal_build_id, ''\\), outcome_reason, version"). WithArgs("monorepo/main", "bad"). WillReturnError(fmt.Errorf("connection reset")) }, @@ -191,11 +192,12 @@ func TestRequestStore_Update(t *testing.T) { ID: "1", Queue: "monorepo/main", URI: "git://remote/monorepo/main/deadbeef", - State: entity.RequestStateProcessing, + State: entity.RequestStateFailed, BuildStrategy: entity.BuildStrategyFull, BaseURI: "", BuildID: "bk-1001", TerminalBuildID: "bk-1001", + OutcomeReason: entity.RequestOutcomeReasonBuildFailed, } const oldVersion, newVersion = int32(1), int32(2) @@ -209,7 +211,7 @@ func TestRequestStore_Update(t *testing.T) { name: "success", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, request.OutcomeReason, newVersion, request.Queue, request.ID, oldVersion). WillReturnResult(sqlmock.NewResult(0, 1)) }, }, @@ -217,7 +219,7 @@ func TestRequestStore_Update(t *testing.T) { name: "version mismatch", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, request.OutcomeReason, newVersion, request.Queue, request.ID, oldVersion). WillReturnResult(sqlmock.NewResult(0, 0)) }, wantErr: true, @@ -227,7 +229,7 @@ func TestRequestStore_Update(t *testing.T) { name: "exec error", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, request.OutcomeReason, newVersion, request.Queue, request.ID, oldVersion). WillReturnError(fmt.Errorf("connection reset")) }, wantErr: true, @@ -236,7 +238,7 @@ func TestRequestStore_Update(t *testing.T) { name: "rows affected error", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.BuildID, request.TerminalBuildID, request.OutcomeReason, newVersion, request.Queue, request.ID, oldVersion). WillReturnResult(sqlmock.NewErrorResult(fmt.Errorf("driver error"))) }, wantErr: true, diff --git a/stovepipe/extension/storage/mysql/schema/request.sql b/stovepipe/extension/storage/mysql/schema/request.sql index f068a01fe..b6f111dc8 100644 --- a/stovepipe/extension/storage/mysql/schema/request.sql +++ b/stovepipe/extension/storage/mysql/schema/request.sql @@ -11,5 +11,6 @@ CREATE TABLE IF NOT EXISTS request ( version INT NOT NULL, terminal_build_id VARCHAR(255) NULL, build_id VARCHAR(255) NOT NULL DEFAULT '', + outcome_reason VARCHAR(64) NOT NULL DEFAULT '', PRIMARY KEY (queue, id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; diff --git a/test/integration/stovepipe/extension/storage/mysql/storage_test.go b/test/integration/stovepipe/extension/storage/mysql/storage_test.go index de593bd9c..f41595034 100644 --- a/test/integration/stovepipe/extension/storage/mysql/storage_test.go +++ b/test/integration/stovepipe/extension/storage/mysql/storage_test.go @@ -338,6 +338,16 @@ func (s *MySQLRequestStoreSuite) TestUpdateCAS() { // Stale CAS: oldVersion 1 no longer matches the stored version (2). err = s.store.Update(s.ctx, updated, 1, 2) require.ErrorIs(s.T(), err, storage.ErrVersionMismatch) + + failed := updated + failed.State = entity.RequestStateFailed + failed.OutcomeReason = entity.RequestOutcomeReasonBuildPollingExhausted + require.NoError(s.T(), s.store.Update(s.ctx, failed, 2, 3)) + failed.Version = 3 + + got, err = s.store.Get(s.ctx, req.ID) + require.NoError(s.T(), err) + require.Equal(s.T(), failed, got) } func (s *MySQLRequestStoreSuite) TestUpdateNotFoundIsVersionMismatch() {