Skip to content

Commit 12bd6c8

Browse files
committed
feat(stovepipe): admit latest head through processing transition
1 parent 16d8723 commit 12bd6c8

2 files changed

Lines changed: 120 additions & 15 deletions

File tree

stovepipe/controller/process/process.go

Lines changed: 99 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414

1515
// Package process holds the process-stage queue controller. It consumes request
1616
// ids from ingest, reloads the Request from storage, coalesces older heads, and
17-
// (in later changes) gates concurrency, decides build strategy, and admits
18-
// winners to build.
17+
// admits the latest head when a build slot is open. Build queue publish lands in
18+
// a follow-up PR.
1919
package process
2020

2121
import (
@@ -35,7 +35,8 @@ import (
3535
)
3636

3737
// Controller consumes ProcessRequest messages from the process stage, reloads the
38-
// referenced Request from storage, and coalesces older heads. Implements consumer.Controller.
38+
// referenced Request from storage, coalesces older heads, and admits the latest when
39+
// a slot is open. Implements consumer.Controller.
3940
type Controller struct {
4041
logger *zap.SugaredLogger
4142
metricsScope tally.Scope
@@ -70,8 +71,8 @@ func NewController(
7071
}
7172
}
7273

73-
// Process reloads the request referenced by the delivery and coalesces older heads.
74-
// Returns nil to ack (success) or an error to nack (retry).
74+
// Process reloads the request referenced by the delivery, coalesces older heads,
75+
// and admits the latest when a slot is open. Returns nil to ack (success) or an error to nack (retry).
7576
func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (retErr error) {
7677
op := metrics.Begin(c.metricsScope, _opName)
7778
defer func() { op.Complete(retErr) }()
@@ -109,8 +110,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
109110
}
110111
}
111112

112-
// processAccepted coalesces older heads against queue.latest_request_id, then resolves
113-
// per-queue config for the concurrency gate. Admit lands in a follow-up PR.
113+
// processAccepted coalesces older heads against queue.latest_request_id, then admits
114+
// the latest head when a build slot is available.
114115
func (c *Controller) processAccepted(ctx context.Context, request entity.Request) error {
115116
queueRow, err := c.loadQueue(ctx, request.Queue)
116117
if err != nil {
@@ -121,7 +122,7 @@ func (c *Controller) processAccepted(ctx context.Context, request entity.Request
121122
}
122123

123124
if queueRow.LatestRequestID == "" {
124-
c.logger.Infow("latest head awaiting admit",
125+
c.logger.Infow("latest head awaiting queue.latest_request_id stamp from ingest",
125126
"request_id", request.ID,
126127
"queue", request.Queue,
127128
"uri", request.URI,
@@ -155,23 +156,111 @@ func (c *Controller) processAccepted(ctx context.Context, request entity.Request
155156
}
156157

157158
if queueRow.InFlightCount >= cfg.MaxConcurrent {
159+
// TODO: re-enqueue the request via PublishAfter on the process topic with GateWaitDelayMs
158160
c.logger.Infow("latest head awaiting build slot",
159161
"request_id", request.ID,
160162
"queue", request.Queue,
163+
"uri", request.URI,
161164
"in_flight_count", queueRow.InFlightCount,
162165
)
163166
return nil
164167
}
165168

166-
c.logger.Infow("latest head awaiting admit",
169+
return c.admitRequestToBuild(ctx, request, queueRow, cfg.MaxConcurrent)
170+
}
171+
172+
// admitRequestToBuild runs the admit workflow: claim a build slot on the queue row,
173+
// mark the request processing with build strategy, and publish the request to build.
174+
func (c *Controller) admitRequestToBuild(ctx context.Context, request entity.Request, queueRow entity.Queue, maxConcurrent int32) error {
175+
for {
176+
err := c.claimBuildSlot(ctx, &queueRow)
177+
if err == nil {
178+
break
179+
}
180+
if errors.Is(err, storage.ErrVersionMismatch) {
181+
// claimBuildSlot reloaded queueRow; another admit may have taken the last slot.
182+
if queueRow.InFlightCount >= maxConcurrent {
183+
return fmt.Errorf("ProcessController gate closed for queue %s", queueRow.Name)
184+
}
185+
continue
186+
}
187+
return err
188+
}
189+
190+
// TODO(build-strategy): derive from queue last_green_uri + SourceControl.IsAncestor.
191+
request.BuildStrategy = entity.BuildStrategyFull
192+
request.BaseURI = ""
193+
194+
if err := c.markProcessing(ctx, &request); err != nil {
195+
return err
196+
}
197+
198+
// TODO(build-publish): publish BuildRequest to the build stage here.
199+
200+
c.logger.Infow("admitted request to build",
167201
"request_id", request.ID,
168202
"queue", request.Queue,
169203
"uri", request.URI,
204+
"build_strategy", string(request.BuildStrategy),
170205
)
171206
return nil
172207
}
173208

174-
// supersedeRequest CAS-marks request accepted→superseded, retrying on version conflicts.
209+
// claimBuildSlot CAS-increments queue.in_flight_count by one. On version mismatch it
210+
// reloads queueRow and returns ErrVersionMismatch so the caller can retry.
211+
func (c *Controller) claimBuildSlot(ctx context.Context, queueRow *entity.Queue) error {
212+
queueStore := c.store.GetQueueStore()
213+
214+
updated := *queueRow
215+
updated.InFlightCount = queueRow.InFlightCount + 1
216+
newVersion := queueRow.Version + 1
217+
if err := queueStore.Update(ctx, updated, queueRow.Version, newVersion); err != nil {
218+
if errors.Is(err, storage.ErrVersionMismatch) {
219+
got, getErr := queueStore.Get(ctx, queueRow.Name)
220+
if getErr != nil {
221+
return fmt.Errorf("ProcessController failed to reload queue %s after version mismatch: %w", queueRow.Name, getErr)
222+
}
223+
*queueRow = got
224+
return storage.ErrVersionMismatch
225+
}
226+
return fmt.Errorf("ProcessController failed to claim build slot for queue %s: %w", queueRow.Name, err)
227+
}
228+
updated.Version = newVersion
229+
*queueRow = updated
230+
return nil
231+
}
232+
233+
// markProcessing CAS-marks request accepted→processing, persisting BuildStrategy and BaseURI
234+
// already set on request by the admit workflow. Retries on version conflicts.
235+
func (c *Controller) markProcessing(ctx context.Context, request *entity.Request) error {
236+
reqStore := c.store.GetRequestStore()
237+
238+
for {
239+
if request.State != entity.RequestStateAccepted {
240+
return nil
241+
}
242+
243+
updated := *request
244+
updated.State = entity.RequestStateProcessing
245+
newVersion := request.Version + 1
246+
if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil {
247+
if errors.Is(err, storage.ErrVersionMismatch) {
248+
got, getErr := reqStore.Get(ctx, request.ID)
249+
if getErr != nil {
250+
return fmt.Errorf("ProcessController failed to reload request %s after version mismatch: %w", request.ID, getErr)
251+
}
252+
*request = got
253+
continue
254+
}
255+
return fmt.Errorf("ProcessController failed to mark request %s processing: %w", request.ID, err)
256+
}
257+
updated.Version = newVersion
258+
*request = updated
259+
return nil
260+
}
261+
}
262+
263+
// supersedeRequest transitions a request from accepted to superseded, retrying on version conflicts.
175264
func (c *Controller) supersedeRequest(ctx context.Context, request entity.Request) error {
176265
reqStore := c.store.GetRequestStore()
177266

stovepipe/controller/process/process_test.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
2626
"github.com/uber/submitqueue/platform/consumer"
2727
"github.com/uber/submitqueue/platform/errs"
28-
queuemock "github.com/uber/submitqueue/platform/extension/messagequeue/mock"
28+
mqmock "github.com/uber/submitqueue/platform/extension/messagequeue/mock"
2929
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
3030
"github.com/uber/submitqueue/stovepipe/entity"
3131
queueconfigdefault "github.com/uber/submitqueue/stovepipe/extension/queueconfig/default"
@@ -65,7 +65,7 @@ func newController(t *testing.T, ctrl *gomock.Controller) (*Controller, processM
6565

6666
func delivery(t *testing.T, ctrl *gomock.Controller, payload []byte) consumer.Delivery {
6767
t.Helper()
68-
d := queuemock.NewMockDelivery(ctrl)
68+
d := mqmock.NewMockDelivery(ctrl)
6969
d.EXPECT().Message().Return(entityqueue.NewMessage(testID, payload, testQueue, nil)).AnyTimes()
7070
d.EXPECT().Attempt().Return(1).AnyTimes()
7171
return d
@@ -88,6 +88,21 @@ func acceptedRequest(id string) entity.Request {
8888
}
8989
}
9090

91+
func expectAdmit(m processMocks, id string) {
92+
updatedQueue := entity.Queue{
93+
Name: testQueue,
94+
LatestRequestID: id,
95+
InFlightCount: 1,
96+
Version: 1,
97+
}
98+
m.queueStore.EXPECT().Update(gomock.Any(), updatedQueue, int32(1), int32(2)).Return(nil)
99+
100+
updatedReq := acceptedRequest(id)
101+
updatedReq.State = entity.RequestStateProcessing
102+
updatedReq.BuildStrategy = entity.BuildStrategyFull
103+
m.reqStore.EXPECT().Update(gomock.Any(), updatedReq, int32(1), int32(2)).Return(nil)
104+
}
105+
91106
func TestProcess(t *testing.T) {
92107
tests := []struct {
93108
name string
@@ -106,7 +121,7 @@ func TestProcess(t *testing.T) {
106121
},
107122
},
108123
{
109-
name: "processing is no-op until republish lands",
124+
name: "processing is no-op until build publish lands",
110125
setup: func(m processMocks) {
111126
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(entity.Request{
112127
ID: testID, Queue: testQueue, State: entity.RequestStateProcessing, Version: 2,
@@ -122,18 +137,19 @@ func TestProcess(t *testing.T) {
122137
},
123138
},
124139
{
125-
name: "latest accepted head awaits admit",
140+
name: "latest accepted head is admitted",
126141
setup: func(m processMocks) {
127142
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(acceptedRequest(testID), nil)
128143
m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(entity.Queue{
129144
Name: testQueue,
130145
LatestRequestID: testID,
131146
Version: 1,
132147
}, nil)
148+
expectAdmit(m, testID)
133149
},
134150
},
135151
{
136-
name: "accepted with empty latest pointer awaits admit",
152+
name: "accepted with empty latest pointer awaits ingest stamp",
137153
setup: func(m processMocks) {
138154
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(acceptedRequest(testID), nil)
139155
m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(entity.Queue{

0 commit comments

Comments
 (0)