Skip to content

Commit bac23cb

Browse files
committed
feat(orchestrator): support creating batches
Add a pre-processing batch state and make downstream readers safely tolerate batches whose reverse indexes are still being initialized. Jira Issues: CODEM-304
1 parent e9e8221 commit bac23cb

6 files changed

Lines changed: 65 additions & 15 deletions

File tree

submitqueue/entity/batch.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ type BatchState string
2222
const (
2323
// BatchStateUnknown is the unreachable state. It is set by default when the structure is initialized. It should never be seen in the system.
2424
BatchStateUnknown BatchState = ""
25-
// BatchStateCreated is the state of a batch that has been created for processing.
25+
// BatchStateCreating indicates that the batch has been persisted but its dependency reverse indexes may not yet be fully initialized.
26+
// A Creating batch is not eligible to be referenced as a dependency.
27+
BatchStateCreating BatchState = "creating"
28+
// BatchStateCreated indicates that the batch and its dependency reverse indexes are fully initialized and ready for processing.
2629
BatchStateCreated BatchState = "created"
2730
// BatchStateSpeculating is the state of a batch that is undergoing speculative execution.
2831
BatchStateSpeculating BatchState = "speculating"
@@ -72,13 +75,30 @@ func IsBatchStateHalted(s BatchState) bool {
7275
// batches that cancel redelivery must be able to resolve.
7376
func ActiveBatchStates() []BatchState {
7477
return []BatchState{
78+
BatchStateCreating,
7579
BatchStateCreated,
7680
BatchStateSpeculating,
7781
BatchStateMerging,
7882
BatchStateCancelling,
7983
}
8084
}
8185

86+
// AllBatchStates returns every persisted batch lifecycle state.
87+
// Use this only for bounded recovery paths that must find historical batches.
88+
// Normal processing should query the narrower active or dependency state sets.
89+
func AllBatchStates() []BatchState {
90+
return []BatchState{
91+
BatchStateCreating,
92+
BatchStateCreated,
93+
BatchStateSpeculating,
94+
BatchStateMerging,
95+
BatchStateSucceeded,
96+
BatchStateFailed,
97+
BatchStateCancelling,
98+
BatchStateCancelled,
99+
}
100+
}
101+
82102
// DependencyBatchStates returns the batch states that make an in-flight batch eligible
83103
// to be a dependency of a newly created batch. When a batch is created, the conflict
84104
// analyzer picks the existing batches it conflicts with as its dependencies; the new

submitqueue/entity/batch_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ func TestBatchState_IsTerminal(t *testing.T) {
2828
terminal bool
2929
}{
3030
{name: "unknown", state: BatchStateUnknown, terminal: false},
31+
{name: "creating", state: BatchStateCreating, terminal: false},
3132
{name: "created", state: BatchStateCreated, terminal: false},
3233
{name: "speculating", state: BatchStateSpeculating, terminal: false},
3334
{name: "merging", state: BatchStateMerging, terminal: false},
@@ -44,6 +45,21 @@ func TestBatchState_IsTerminal(t *testing.T) {
4445
}
4546
}
4647

48+
func TestBatchStateSets(t *testing.T) {
49+
assert.Contains(t, ActiveBatchStates(), BatchStateCreating)
50+
assert.NotContains(t, DependencyBatchStates(), BatchStateCreating)
51+
assert.ElementsMatch(t, []BatchState{
52+
BatchStateCreating,
53+
BatchStateCreated,
54+
BatchStateSpeculating,
55+
BatchStateMerging,
56+
BatchStateSucceeded,
57+
BatchStateFailed,
58+
BatchStateCancelling,
59+
BatchStateCancelled,
60+
}, AllBatchStates())
61+
}
62+
4763
func TestBatch_SerializationRoundTrip(t *testing.T) {
4864
tests := []struct {
4965
name string

submitqueue/extension/storage/batch_dependent_store.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,12 @@ import (
2525
// BatchDependentStore is an interface that defines methods for managing batch dependent information in the database.
2626
//
2727
// A BatchDependent is a reverse index ("batches that depend on me") paired one-to-one with a Batch.
28-
// The batch-creation flow always calls Create here before creating the Batch itself, so every active
29-
// Batch is guaranteed to have a corresponding BatchDependent row. Lookups via Get are only performed
30-
// for batch IDs returned from the active-batch set, meaning a missing row indicates data corruption or
31-
// out-of-band manipulation rather than a normal "not found" outcome. ErrNotFound is therefore part of
32-
// the contract for completeness but is not expected to be returned in steady-state operation.
28+
// The batch-creation flow creates this row while the Batch is Creating and before making the Batch eligible for pipeline processing.
29+
// A Creating Batch can briefly exist without its row; every Batch that reaches Created is guaranteed to have one.
3330
type BatchDependentStore interface {
3431
// Get retrieves the batch dependent by batch ID.
3532
// If the batch contains no dependents, the returned BatchDependent will have an empty Dependents list.
36-
// Returns ErrNotFound if the batch itself is not found, which should never happen in steady-state system and
37-
// therefore does not need a special handling.
33+
// Returns ErrNotFound if no reverse-index row exists for the batch.
3834
Get(ctx context.Context, batchID string) (entity.BatchDependent, error)
3935

4036
// Create creates a new batch dependent.

submitqueue/orchestrator/controller/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ Version arithmetic follows the [storage optimistic-locking contract](../../exten
8181

8282
| Batch state | Behavior |
8383
|---|---|
84+
| `Creating` | Acknowledge an early dependency wake; the batch controller publishes again after initialization. |
8485
| `Created` | Start speculation: publish to `build`, then record `Speculating`. |
8586
| `Speculating` | Once dependencies resolve, publish to `merge` and record `Merging`. |
8687
| `Merging` | Acknowledge without regressing the batch; the merge controller owns recovery. |

submitqueue/orchestrator/controller/speculate/speculate.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
// Per invocation, the controller advances the batch one step in the
3737
// state machine:
3838
//
39+
// - Creating → no-op; the batch controller publishes again after reverse-index initialization completes.
3940
// - Created → publish to build, transition to Speculating.
4041
// - Speculating → if all deps are Succeeded, publish to merge and
4142
// transition to Merging; otherwise no-op (or fail-fast if a dep is
@@ -133,6 +134,11 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
133134
}
134135

135136
switch batch.State {
137+
case entity.BatchStateCreating:
138+
// A dependency can finish after this batch subscribes in its reverse index but before initialization transitions it to Created.
139+
// Ack that early wake; the batch controller publishes again after initialization.
140+
metrics.NamedCounter(c.metricsScope, opName, "noop_creating", 1)
141+
return nil
136142
case entity.BatchStateCreated:
137143
return c.startSpeculation(ctx, batch)
138144
case entity.BatchStateSpeculating:
@@ -357,13 +363,10 @@ func (c *Controller) cancelBuild(ctx context.Context, batch entity.Batch) error
357363
return nil
358364
}
359365

360-
// respeculateDependents publishes a speculate event for every batch that
361-
// depends on the given batch. The batch controller creates a BatchDependent
362-
// row (with Dependents possibly empty) for every batch it persists, so a
363-
// missing row at this point is a storage invariant violation, not a normal
364-
// "no dependents" case — surface ErrNotFound as a regular storage error so
365-
// the message nacks and either an operator or the batch controller's own
366-
// crash-recovery can resolve the inconsistency.
366+
// respeculateDependents publishes a speculate event for every batch that depends on the given batch.
367+
// The batch controller creates a BatchDependent row before transitioning Creating → Created.
368+
// Only fully initialized batches are published and can reach a terminal state, so a missing row is a storage invariant violation.
369+
// Surface ErrNotFound as a regular storage error so the message nacks.
367370
//
368371
// Called both from the cancelBatch terminal flow and from the terminal
369372
// self-heal branch on redelivery of an already-Cancelled batch.

submitqueue/orchestrator/controller/speculate/speculate_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,20 @@ func TestController_Process_StartSpeculation(t *testing.T) {
129129
}
130130
}
131131

132+
func TestController_Process_CreatingNoOp(t *testing.T) {
133+
ctrl := gomock.NewController(t)
134+
batch := testBatch(entity.BatchStateCreating)
135+
136+
batchStore := storagemock.NewMockBatchStore(ctrl)
137+
batchStore.EXPECT().Get(gomock.Any(), batch.ID).Return(batch, nil)
138+
139+
store := storagemock.NewMockStorage(ctrl)
140+
store.EXPECT().GetBatchStore().Return(batchStore).AnyTimes()
141+
142+
controller := newTestController(t, ctrl, store, nil)
143+
require.NoError(t, runProcess(t, ctrl, controller, batch.ID))
144+
}
145+
132146
// tryFinalize: Speculating with no deps should publish to merge and CAS to Merging.
133147
func TestController_Process_FinalizeNoDeps(t *testing.T) {
134148
ctrl := gomock.NewController(t)

0 commit comments

Comments
 (0)