[3/N] feat(request): materialize request log status #342
+367
−0
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| // 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 request | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| "github.com/uber/submitqueue/submitqueue/entity" | ||
| "github.com/uber/submitqueue/submitqueue/extension/storage" | ||
| ) | ||
|
|
||
| // Materializer appends request logs and projects the winning current status. | ||
| // Storage implementations remain mechanical; this type owns winner selection, optimistic concurrency, and projection repair. | ||
| type Materializer struct { | ||
| store storage.Storage | ||
| } | ||
|
|
||
| // NewMaterializer creates a request read-model materializer. | ||
| func NewMaterializer(store storage.Storage) *Materializer { | ||
| return &Materializer{store: store} | ||
| } | ||
|
|
||
| // PersistLog appends one audit log and materializes its winning state. | ||
| // Projection errors are returned so queue deliveries are retried rather than silently dropping the side write. | ||
| func (m *Materializer) PersistLog(ctx context.Context, log entity.RequestLog) error { | ||
| if err := m.store.GetRequestLogStore().Insert(ctx, log); err != nil { | ||
|
Collaborator
There was a problem hiding this comment. The salted log insert runs before up to five more fallible round trips, and every nack→redelivery re-inserts a new row (the salt guarantees no dedup). The test at The duplication is avoidable for retries of the same logical event: derive the row identity from the event instead of a random salt — e.g. key on |
||
| return fmt.Errorf("failed to insert request log request_id=%s: %w", log.RequestID, err) | ||
| } | ||
|
|
||
| for { | ||
| summary, err := m.store.GetRequestSummaryStore().Get(ctx, log.RequestID) | ||
|
Collaborator
There was a problem hiding this comment.
Separately: |
||
| if err != nil { | ||
| return fmt.Errorf("failed to get request summary request_id=%s: %w", log.RequestID, err) | ||
| } | ||
|
|
||
| if logWins(log, summary) { | ||
| oldVersion := summary.Version | ||
| newVersion := oldVersion + 1 | ||
| updated := summary | ||
| updated.Status = log.Status | ||
| updated.RequestVersion = log.RequestVersion | ||
| updated.StatusTimestampMs = log.TimestampMs | ||
| updated.LastError = log.LastError | ||
| updated.Metadata = cloneMetadata(log.Metadata) | ||
|
|
||
| if err := m.store.GetRequestSummaryStore().Update(ctx, updated, oldVersion, newVersion); err != nil { | ||
| if errors.Is(err, storage.ErrVersionMismatch) { | ||
| continue | ||
| } | ||
| return fmt.Errorf("failed to update request summary request_id=%s: %w", log.RequestID, err) | ||
| } | ||
| updated.Version = newVersion | ||
| summary = updated | ||
| } | ||
|
|
||
| if err := m.repairQueueSummary(ctx, summary); err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| } | ||
|
|
||
| func (m *Materializer) repairQueueSummary(ctx context.Context, authoritative entity.RequestSummary) error { | ||
| desired := queueSummaryFromSummary(authoritative) | ||
| for { | ||
| current, err := m.store.GetRequestQueueSummaryStore().Get(ctx, desired.Queue, desired.ReceivedAtMs, desired.RequestID) | ||
| if errors.Is(err, storage.ErrNotFound) { | ||
| if err := m.store.GetRequestQueueSummaryStore().Create(ctx, desired); err != nil { | ||
| if errors.Is(err, storage.ErrAlreadyExists) { | ||
| continue | ||
| } | ||
| return fmt.Errorf("failed to recreate queue summary request_id=%s: %w", desired.RequestID, err) | ||
| } | ||
| return nil | ||
| } | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get queue summary request_id=%s: %w", desired.RequestID, err) | ||
| } | ||
| if current.Version == desired.Version { | ||
| return nil | ||
| } | ||
| if current.Version > desired.Version { | ||
|
Collaborator
There was a problem hiding this comment.
|
||
| return fmt.Errorf("queue summary ahead of authoritative summary request_id=%s queue_version=%d summary_version=%d", desired.RequestID, current.Version, desired.Version) | ||
| } | ||
| if err := m.store.GetRequestQueueSummaryStore().Update(ctx, desired, current.Version, desired.Version); err != nil { | ||
| if errors.Is(err, storage.ErrVersionMismatch) { | ||
| continue | ||
| } | ||
| return fmt.Errorf("failed to update queue summary request_id=%s: %w", desired.RequestID, err) | ||
| } | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| func logWins(log entity.RequestLog, summary entity.RequestSummary) bool { | ||
|
Collaborator
There was a problem hiding this comment. probably deserves some comments |
||
| incomingTerminal := isVersionedTerminal(log) | ||
| currentTerminal := isVersionedTerminalSummary(summary) | ||
| if incomingTerminal != currentTerminal { | ||
| return incomingTerminal | ||
| } | ||
| if incomingTerminal { | ||
|
Collaborator
There was a problem hiding this comment. When both the incoming log and the summary are versioned-terminal with equal |
||
| if log.RequestVersion != summary.RequestVersion { | ||
| return log.RequestVersion > summary.RequestVersion | ||
| } | ||
| } | ||
| return log.TimestampMs > summary.StatusTimestampMs | ||
| } | ||
|
|
||
| func isVersionedTerminalSummary(summary entity.RequestSummary) bool { | ||
| return summary.RequestVersion > 0 && entity.IsRequestStateTerminal(entity.RequestState(summary.Status)) | ||
| } | ||
|
|
||
| func isVersionedTerminal(log entity.RequestLog) bool { | ||
| return log.RequestVersion > 0 && entity.IsRequestStateTerminal(entity.RequestState(log.Status)) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,235 @@ | ||
| // 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 request | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "github.com/uber/submitqueue/submitqueue/entity" | ||
| "github.com/uber/submitqueue/submitqueue/extension/storage" | ||
| storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock" | ||
| "go.uber.org/mock/gomock" | ||
| ) | ||
|
|
||
| func TestMaterializer_PersistLog(t *testing.T) { | ||
| base := testRequestSummary() | ||
| log := entity.RequestLog{RequestID: "q/1", TimestampMs: 20, Status: entity.RequestStatusLanded, RequestVersion: 2, Metadata: map[string]string{}} | ||
| t.Run("winning log updates both projections", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil) | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(base, nil) | ||
| summaryStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).DoAndReturn(func(_ context.Context, updated entity.RequestSummary, _, _ int32) error { | ||
| assert.Equal(t, entity.RequestStatusLanded, updated.Status) | ||
| assert.Equal(t, int32(2), updated.RequestVersion) | ||
| return nil | ||
| }) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(queueSummaryFromSummary(base), nil) | ||
| queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) | ||
| require.NoError(t, NewMaterializer(store).PersistLog(context.Background(), log)) | ||
| }) | ||
|
|
||
| t.Run("unversioned terminal status does not receive terminal precedence", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| current := base | ||
| current.Status = entity.RequestStatusLanded | ||
| current.RequestVersion = 0 | ||
| incoming := entity.RequestLog{RequestID: "q/1", TimestampMs: 20, Status: entity.RequestStatusProcessing, RequestVersion: 0, Metadata: map[string]string{}} | ||
| logStore.EXPECT().Insert(gomock.Any(), incoming).Return(nil) | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(current, nil) | ||
| summaryStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).DoAndReturn(func(_ context.Context, updated entity.RequestSummary, _, _ int32) error { | ||
| assert.Equal(t, entity.RequestStatusProcessing, updated.Status) | ||
| return nil | ||
| }) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(queueSummaryFromSummary(current), nil) | ||
| queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) | ||
| require.NoError(t, NewMaterializer(store).PersistLog(context.Background(), incoming)) | ||
| }) | ||
|
|
||
| t.Run("CAS conflict reloads and repairs winner", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil) | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(base, nil) | ||
| summaryStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(storage.ErrVersionMismatch) | ||
| advanced := base | ||
| advanced.Status = entity.RequestStatusLanded | ||
| advanced.RequestVersion = 2 | ||
| advanced.StatusTimestampMs = 20 | ||
| advanced.Version = 2 | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(advanced, nil) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(queueSummaryFromSummary(base), nil) | ||
| queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) | ||
| require.NoError(t, NewMaterializer(store).PersistLog(context.Background(), log)) | ||
| }) | ||
|
|
||
| t.Run("non-winning redelivery repairs stale queue projection", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil) | ||
| advanced := base | ||
| advanced.Status = entity.RequestStatusLanded | ||
| advanced.RequestVersion = 2 | ||
| advanced.StatusTimestampMs = 20 | ||
| advanced.Version = 2 | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(advanced, nil) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(queueSummaryFromSummary(base), nil) | ||
| queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) | ||
| require.NoError(t, NewMaterializer(store).PersistLog(context.Background(), log)) | ||
| }) | ||
|
|
||
| t.Run("missing queue projection is recreated", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil) | ||
| advanced := base | ||
| advanced.Status = entity.RequestStatusLanded | ||
| advanced.RequestVersion = 2 | ||
| advanced.StatusTimestampMs = 20 | ||
| advanced.Version = 2 | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(advanced, nil) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(entity.RequestQueueSummary{}, storage.ErrNotFound) | ||
| queueStore.EXPECT().Create(gomock.Any(), queueSummaryFromSummary(advanced)).Return(nil) | ||
| require.NoError(t, NewMaterializer(store).PersistLog(context.Background(), log)) | ||
| }) | ||
|
|
||
| t.Run("retry after projection failure appends another audit row", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| materializer := NewMaterializer(store) | ||
| advanced := base | ||
| advanced.Status = entity.RequestStatusLanded | ||
| advanced.RequestVersion = 2 | ||
| advanced.StatusTimestampMs = 20 | ||
| advanced.Version = 2 | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil).Times(2) | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(advanced, nil).Times(2) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(entity.RequestQueueSummary{}, errors.New("queue store down")) | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(queueSummaryFromSummary(advanced), nil) | ||
|
|
||
| require.Error(t, materializer.PersistLog(context.Background(), log)) | ||
| require.NoError(t, materializer.PersistLog(context.Background(), log)) | ||
| }) | ||
|
|
||
| t.Run("missing authoritative summary fails", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, _, logStore := materializerStores(ctrl) | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil) | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(entity.RequestSummary{}, storage.ErrNotFound) | ||
| require.Error(t, NewMaterializer(store).PersistLog(context.Background(), log)) | ||
| }) | ||
|
|
||
| t.Run("queue projection ahead fails fast", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| store, summaryStore, queueStore, logStore := materializerStores(ctrl) | ||
| logStore.EXPECT().Insert(gomock.Any(), log).Return(nil) | ||
| advanced := base | ||
| advanced.Status = entity.RequestStatusLanded | ||
| advanced.RequestVersion = 2 | ||
| advanced.StatusTimestampMs = 20 | ||
| advanced.Version = 2 | ||
| summaryStore.EXPECT().Get(gomock.Any(), "q/1").Return(advanced, nil) | ||
| queueAhead := queueSummaryFromSummary(advanced) | ||
| queueAhead.Version = 3 | ||
| queueStore.EXPECT().Get(gomock.Any(), "q", int64(10), "q/1").Return(queueAhead, nil) | ||
| require.Error(t, NewMaterializer(store).PersistLog(context.Background(), log)) | ||
| }) | ||
| } | ||
|
|
||
| func TestLogWins(t *testing.T) { | ||
| base := testRequestSummary() | ||
| tests := []struct { | ||
| name string | ||
| current entity.RequestSummary | ||
| incoming entity.RequestLog | ||
| want bool | ||
| }{ | ||
| { | ||
| name: "versioned terminal beats newer unversioned status", | ||
| current: entity.RequestSummary{Status: entity.RequestStatusProcessing, StatusTimestampMs: 200}, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusLanded, RequestVersion: 1, TimestampMs: 100, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "nonterminal cannot replace versioned terminal", | ||
| current: entity.RequestSummary{Status: entity.RequestStatusLanded, RequestVersion: 1, StatusTimestampMs: 100}, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusProcessing, TimestampMs: 200, | ||
| }, | ||
| }, | ||
| { | ||
| name: "higher terminal request version wins", | ||
| current: entity.RequestSummary{Status: entity.RequestStatusError, RequestVersion: 1, StatusTimestampMs: 200}, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusLanded, RequestVersion: 2, TimestampMs: 100, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "lower terminal request version loses", | ||
| current: entity.RequestSummary{Status: entity.RequestStatusLanded, RequestVersion: 2, StatusTimestampMs: 100}, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusError, RequestVersion: 1, TimestampMs: 200, | ||
| }, | ||
| }, | ||
| { | ||
| name: "equal terminal version uses later timestamp", | ||
| current: entity.RequestSummary{Status: entity.RequestStatusError, RequestVersion: 2, StatusTimestampMs: 100}, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusLanded, RequestVersion: 2, TimestampMs: 200, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "without terminal winner later timestamp wins", | ||
| current: base, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusStarted, TimestampMs: base.StatusTimestampMs + 1, | ||
| }, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "exact version and timestamp tie keeps current winner", | ||
| current: entity.RequestSummary{Status: entity.RequestStatusError, RequestVersion: 2, StatusTimestampMs: 200}, | ||
| incoming: entity.RequestLog{ | ||
| Status: entity.RequestStatusLanded, RequestVersion: 2, TimestampMs: 200, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| assert.Equal(t, tt.want, logWins(tt.incoming, tt.current)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func materializerStores(ctrl *gomock.Controller) (*storagemock.MockStorage, *storagemock.MockRequestSummaryStore, *storagemock.MockRequestQueueSummaryStore, *storagemock.MockRequestLogStore) { | ||
| store := storagemock.NewMockStorage(ctrl) | ||
| summaryStore := storagemock.NewMockRequestSummaryStore(ctrl) | ||
| queueStore := storagemock.NewMockRequestQueueSummaryStore(ctrl) | ||
| logStore := storagemock.NewMockRequestLogStore(ctrl) | ||
| store.EXPECT().GetRequestSummaryStore().Return(summaryStore).AnyTimes() | ||
| store.EXPECT().GetRequestQueueSummaryStore().Return(queueStore).AnyTimes() | ||
| store.EXPECT().GetRequestLogStore().Return(logStore).AnyTimes() | ||
| return store, summaryStore, queueStore, logStore | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking if there is better name for this like RequestReconciler or something