Skip to content

Commit b27d20c

Browse files
committed
feat(stovepipe): advance queue latest_request_id on ingest
After ensureRequest succeeds, CAS-update the queue row's latest_request_id when the request id is newer. CompareRequestID makes the advance a no-op on dedup re-ingests; optimistic-lock retries handle concurrent ingests.
1 parent 90cbeef commit b27d20c

3 files changed

Lines changed: 81 additions & 12 deletions

File tree

stovepipe/controller/ingest.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ func (c *IngestController) Ingest(ctx context.Context, req *pb.IngestRequest) (r
126126
return nil, err
127127
}
128128

129+
if err := c.advanceQueueLatestRequestID(ctx, queue, id); err != nil {
130+
return nil, err
131+
}
132+
129133
// Publish while the request is still pre-pipeline (Accepted). The process consumer is
130134
// idempotent (keyed on the request id, at-least-once), so re-publishing on a retry or a
131135
// duplicate report is safe and closes the "request created but publish failed" gap. Once
@@ -211,6 +215,39 @@ func (c *IngestController) ensureRequest(ctx context.Context, id, queue, uri str
211215
return request, nil
212216
}
213217

218+
// advanceQueueLatestRequestID CAS-updates queue.latest_request_id to id when id is newer.
219+
// Retries on optimistic-lock conflicts so concurrent ingests converge.
220+
func (c *IngestController) advanceQueueLatestRequestID(ctx context.Context, queue, id string) error {
221+
queueStore := c.store.GetQueueStore()
222+
223+
for {
224+
queueRow, err := queueStore.GetOrCreate(ctx, queue, entity.Queue{Version: 1})
225+
if err != nil {
226+
return fmt.Errorf("IngestController failed to load queue %s: %w", queue, err)
227+
}
228+
if queueRow.LatestRequestID != "" {
229+
cmp, err := entity.CompareRequestID(queue, id, queueRow.LatestRequestID)
230+
if err != nil {
231+
return fmt.Errorf("IngestController failed to compare request ids for queue %s: %w", queue, err)
232+
}
233+
if cmp <= 0 {
234+
return nil
235+
}
236+
}
237+
238+
updated := queueRow
239+
updated.LatestRequestID = id
240+
newVersion := queueRow.Version + 1
241+
if err := queueStore.Update(ctx, updated, queueRow.Version, newVersion); err != nil {
242+
if errors.Is(err, storage.ErrVersionMismatch) {
243+
continue
244+
}
245+
return fmt.Errorf("IngestController failed to update queue %s latest_request_id: %w", queue, err)
246+
}
247+
return nil
248+
}
249+
}
250+
214251
// publishProcess publishes the request ID to the process stage, partitioned by queue so a
215252
// queue's requests stay ordered.
216253
func (c *IngestController) publishProcess(ctx context.Context, id, queue string) error {

stovepipe/controller/ingest_test.go

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,29 +43,32 @@ const (
4343

4444
// ingestMocks bundles the mocks an Ingest test case wires expectations on.
4545
type ingestMocks struct {
46-
counter *countermock.MockCounter
47-
factory *scmock.MockFactory
48-
sc *scmock.MockSourceControl
49-
reqStore *storagemock.MockRequestStore
50-
uriStore *storagemock.MockRequestURIStore
51-
publisher *mqmock.MockPublisher
46+
counter *countermock.MockCounter
47+
factory *scmock.MockFactory
48+
sc *scmock.MockSourceControl
49+
reqStore *storagemock.MockRequestStore
50+
uriStore *storagemock.MockRequestURIStore
51+
queueStore *storagemock.MockQueueStore
52+
publisher *mqmock.MockPublisher
5253
}
5354

5455
func newIngestController(t *testing.T, ctrl *gomock.Controller) (*IngestController, ingestMocks) {
5556
t.Helper()
5657

5758
m := ingestMocks{
58-
counter: countermock.NewMockCounter(ctrl),
59-
factory: scmock.NewMockFactory(ctrl),
60-
sc: scmock.NewMockSourceControl(ctrl),
61-
reqStore: storagemock.NewMockRequestStore(ctrl),
62-
uriStore: storagemock.NewMockRequestURIStore(ctrl),
63-
publisher: mqmock.NewMockPublisher(ctrl),
59+
counter: countermock.NewMockCounter(ctrl),
60+
factory: scmock.NewMockFactory(ctrl),
61+
sc: scmock.NewMockSourceControl(ctrl),
62+
reqStore: storagemock.NewMockRequestStore(ctrl),
63+
uriStore: storagemock.NewMockRequestURIStore(ctrl),
64+
queueStore: storagemock.NewMockQueueStore(ctrl),
65+
publisher: mqmock.NewMockPublisher(ctrl),
6466
}
6567

6668
store := storagemock.NewMockStorage(ctrl)
6769
store.EXPECT().GetRequestStore().Return(m.reqStore).AnyTimes()
6870
store.EXPECT().GetRequestURIStore().Return(m.uriStore).AnyTimes()
71+
store.EXPECT().GetQueueStore().Return(m.queueStore).AnyTimes()
6972

7073
queue := mqmock.NewMockQueue(ctrl)
7174
queue.EXPECT().Publisher().Return(m.publisher).AnyTimes()
@@ -85,6 +88,25 @@ func expectResolve(m ingestMocks) {
8588
m.sc.EXPECT().Latest(gomock.Any()).Return(testURI, nil)
8689
}
8790

91+
// expectAdvanceLatestRequestID wires GetOrCreate + Update for queue.latest_request_id.
92+
func expectAdvanceLatestRequestID(m ingestMocks, queue, id string) {
93+
m.queueStore.EXPECT().GetOrCreate(gomock.Any(), queue, entity.Queue{Version: 1}).Return(entity.Queue{
94+
Name: queue,
95+
Version: 1,
96+
}, nil)
97+
updated := entity.Queue{Name: queue, LatestRequestID: id, Version: 1}
98+
m.queueStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil)
99+
}
100+
101+
// expectAdvanceLatestRequestIDNoOp wires GetOrCreate when latest_request_id is already at id.
102+
func expectAdvanceLatestRequestIDNoOp(m ingestMocks, queue, id string) {
103+
m.queueStore.EXPECT().GetOrCreate(gomock.Any(), queue, entity.Queue{Version: 1}).Return(entity.Queue{
104+
Name: queue,
105+
LatestRequestID: id,
106+
Version: 1,
107+
}, nil)
108+
}
109+
88110
func TestIngestController_Ingest(t *testing.T) {
89111
tests := []struct {
90112
name string
@@ -104,6 +126,7 @@ func TestIngestController_Ingest(t *testing.T) {
104126
m.uriStore.EXPECT().Create(gomock.Any(), testQueue, testURI, "request/monorepo/main/7").Return(nil)
105127
m.reqStore.EXPECT().Get(gomock.Any(), "request/monorepo/main/7").Return(entity.Request{}, storage.ErrNotFound)
106128
m.reqStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil)
129+
expectAdvanceLatestRequestID(m, testQueue, "request/monorepo/main/7")
107130
m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(nil)
108131
},
109132
wantID: "request/monorepo/main/7",
@@ -115,6 +138,7 @@ func TestIngestController_Ingest(t *testing.T) {
115138
expectResolve(m)
116139
m.uriStore.EXPECT().GetIDByURI(gomock.Any(), testQueue, testURI).Return("request/monorepo/main/3", nil)
117140
m.reqStore.EXPECT().Get(gomock.Any(), "request/monorepo/main/3").Return(entity.Request{ID: "request/monorepo/main/3", State: entity.RequestStateAccepted}, nil)
141+
expectAdvanceLatestRequestIDNoOp(m, testQueue, "request/monorepo/main/3")
118142
m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(nil)
119143
},
120144
wantID: "request/monorepo/main/3",
@@ -128,6 +152,7 @@ func TestIngestController_Ingest(t *testing.T) {
128152
m.uriStore.EXPECT().GetIDByURI(gomock.Any(), testQueue, testURI).Return("request/monorepo/main/3", nil)
129153
m.reqStore.EXPECT().Get(gomock.Any(), "request/monorepo/main/3").Return(entity.Request{}, storage.ErrNotFound)
130154
m.reqStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil)
155+
expectAdvanceLatestRequestID(m, testQueue, "request/monorepo/main/3")
131156
m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(nil)
132157
},
133158
wantID: "request/monorepo/main/3",
@@ -142,6 +167,7 @@ func TestIngestController_Ingest(t *testing.T) {
142167
m.uriStore.EXPECT().Create(gomock.Any(), testQueue, testURI, "request/monorepo/main/7").Return(storage.ErrAlreadyExists)
143168
m.uriStore.EXPECT().GetIDByURI(gomock.Any(), testQueue, testURI).Return("request/monorepo/main/3", nil)
144169
m.reqStore.EXPECT().Get(gomock.Any(), "request/monorepo/main/3").Return(entity.Request{ID: "request/monorepo/main/3", State: entity.RequestStateAccepted}, nil)
170+
expectAdvanceLatestRequestIDNoOp(m, testQueue, "request/monorepo/main/3")
145171
m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(nil)
146172
},
147173
wantID: "request/monorepo/main/3",
@@ -205,6 +231,7 @@ func TestIngestController_Ingest(t *testing.T) {
205231
m.uriStore.EXPECT().Create(gomock.Any(), testQueue, testURI, gomock.Any()).Return(nil)
206232
m.reqStore.EXPECT().Get(gomock.Any(), gomock.Any()).Return(entity.Request{}, storage.ErrNotFound)
207233
m.reqStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil)
234+
expectAdvanceLatestRequestID(m, testQueue, "request/monorepo/main/7")
208235
m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(errors.New("queue down"))
209236
},
210237
wantErr: true,

test/integration/stovepipe/suite_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,11 @@ func (s *StovepipeIntegrationSuite) TestIngestAPI() {
136136
resp2, err := s.client.Ingest(s.ctx, &pb.IngestRequest{Queue: queue})
137137
require.NoError(t, err, "second Ingest failed")
138138
assert.Equal(t, id, resp2.Id, "re-ingest of the same head should dedup to the same id")
139+
140+
// latest_request_id on the queue row.
141+
var latestRequestID string
142+
require.NoError(t, s.db.QueryRow("SELECT latest_request_id FROM queue WHERE name = ?", queue).Scan(&latestRequestID))
143+
assert.Equal(t, id, latestRequestID, "queue latest_request_id should point at the minted request")
139144
}
140145

141146
// TestIngestEmptyQueue verifies the request-validation error surfaces over gRPC.

0 commit comments

Comments
 (0)