Skip to content

Commit 296d400

Browse files
committed
feat(orchestrator): Keep exact per-state request membership records so a queue's requests can be listed by state
1 parent 2bdc6d1 commit 296d400

25 files changed

Lines changed: 752 additions & 50 deletions

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ Vendor-agnostic, pluggable interfaces with implementations in subdirectories:
143143
3. **Service-scoped resolution** — when only one service of a multi-service domain resolves an extension, its `Factory`, its aggregate, its implementations, its mocks and its schema live at `{domain}/{service}/extension/{ext}/`. The behavioural contracts stay at `{domain}/extension/{ext}/`.
144144
4. Factory interface for dependency injection and lifecycle management (constructed in wiring, not inside `platform/extension` packages).
145145
146-
**Split on reachability, not on declaration.** What a service may resolve is decided by the accessors on its aggregate, so that is the part worth scoping to a service — a gateway holding a four-accessor `Storage` cannot obtain an orchestrator store whatever else it can name. The contracts stay shared because moving them adds no enforcement and costs every domain-level caller a dependency on a service package. `submitqueue/extension/storage` is the worked example: thirteen store contracts, their mocks and the error vocabulary shared, while each service owns its own aggregate, MySQL implementation and schema.
146+
**Split on reachability, not on declaration.** What a service may resolve is decided by the accessors on its aggregate, so that is the part worth scoping to a service — a gateway holding a four-accessor `Storage` cannot obtain an orchestrator store whatever else it can name. The contracts stay shared because moving them adds no enforcement and costs every domain-level caller a dependency on a service package. `submitqueue/extension/storage` is the worked example: fourteen store contracts, their mocks and the error vocabulary shared, while each service owns its own aggregate, MySQL implementation and schema.
147147
148148
**Extensions hold contracts and implementations only — not factories or routing.**
149149

‎doc/rfc/service-scoped-extensions.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ Separating the services onto their own databases already worked — the orchestr
2626
2. An extension shared between a domain's services stays wholly at `{domain}/extension/{ext}/` — including one used by a single service today but expected in both. `submitqueue/extension/queueconfig/` is gateway-only now and stays where it is on that basis. A single-service domain is unaffected: its domain root is its service root, so nothing moves.
2727
3. Storage is the extension this split applied to. `changeprovider`, `validator`, `conflict`, `buildrunner` and `speculation` are orchestrator-only and would qualify under Decision 1, but none has a schema and none was the reason for this change; they stay at `{domain}/extension/` until a later RFC takes them. The trigger for moving storage was a schema that a separate-database deployment must not over-provision.
2828
4. SubmitQueue's storage aggregate splits in two: `submitqueue/gateway/extension/storage/` declares a `Factory` and a four-accessor `Storage`, `submitqueue/orchestrator/extension/storage/` a `Factory` and a nine-accessor `Storage`. Neither can resolve the other's stores.
29-
5. `submitqueue/extension/storage/` keeps the thirteen store contracts, their mocks, the error vocabulary, `Config`, and the package README, which documents the contracts. Its import path does not change, so callers that name only a contract or an error are untouched.
29+
5. `submitqueue/extension/storage/` keeps the fourteen store contracts, their mocks, the error vocabulary, `Config`, and the package README, which documents the contracts. Its import path does not change, so callers that name only a contract or an error are untouched.
3030
6. The contract package is not promoted to `platform/`. It is not cross-domain — `stovepipe/extension/storage/storage.go` already declares its own verbatim copy of the same symbols, so a per-domain error vocabulary is the existing pattern rather than something this RFC introduces.
3131
7. Each half owns its own MySQL schema directory, so the tables a service creates are the tables it reads. A union target over both stays available for deployments that colocate the services — the end-to-end tests and the local compose stack take it and name no service directory. Split schemas are the opt-in, not the default.
3232

@@ -49,6 +49,7 @@ Every store, verified against actual usage rather than intent.
4949
| `BatchStore` | `batch` |
5050
| `BatchDependentStore` | `batch_dependent` |
5151
| `QueueBatchStateStore` | `queue_batch_state` |
52+
| `QueueRequestStateStore` | `queue_request_state` |
5253
| `BuildStore` | `build` |
5354
| `SpeculationPathSetStore` | `speculation_path_set` |
5455
| `PathBuildStore` | `path_build` |

‎service/submitqueue/orchestrator/server/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func run() error {
143143

144144
cnt := counterFactory{db: appDB, scope: scope.SubScope("counter")}
145145

146-
store, err := mysqlstorage.NewStorage(appDB, scope.SubScope("storage"))
146+
store, err := mysqlstorage.NewStorage(appDB, scope.SubScope("storage"), mysqlstorage.WithQueueRequestStateRecords())
147147
if err != nil {
148148
return fmt.Errorf("failed to create storage: %w", err)
149149
}

‎submitqueue/entity/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
"queue_batch_state.go",
1818
"queue_config.go",
1919
"queue_pause.go",
20+
"queue_request_state.go",
2021
"request.go",
2122
"request_batch.go",
2223
"request_history.go",
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright (c) 2026 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package entity
16+
17+
// QueueRequestState is a membership record filing one in-queue request under its current
18+
// state. Exactly one record exists for a request from its creation until it reaches a
19+
// terminal state, and none afterwards.
20+
//
21+
// The (Queue, State, RequestID) triple is the record's identity. The record mirrors the
22+
// State of the Request identified by RequestID and changes together with it.
23+
type QueueRequestState struct {
24+
// Queue is the name of the queue the request belongs to. Queue name is defined in the
25+
// configuration and should be unique within the system.
26+
Queue string
27+
// State is the request's current state. Always one of InQueueRequestStates.
28+
State RequestState
29+
// RequestID is the request's identifier, unique within Queue.
30+
RequestID string
31+
// EnteredAtMs is when the request entered State, in Unix milliseconds. A version-only
32+
// update that keeps the state does not change it.
33+
EnteredAtMs int64
34+
}

‎submitqueue/entity/request.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ func IsRequestStateTerminal(s RequestState) bool {
5959
return s == RequestStateLanded || s == RequestStateError || s == RequestStateCancelled
6060
}
6161

62+
// InQueueRequestStates lists every non-terminal request state: the states a request occupies while it is in its
63+
// queue. Every valid RequestState is either listed here or terminal.
64+
var InQueueRequestStates = []RequestState{
65+
RequestStateStarted,
66+
RequestStateValidated,
67+
RequestStateBatched,
68+
RequestStateProcessing,
69+
RequestStateCancelling,
70+
}
71+
6272
// IsRequestStateHalted returns true if the request is either terminal or in the process of being cancelled.
6373
// Forward-progress controllers (validate, batch, ...) use this to short-circuit work for requests that the
6474
// user has asked to cancel — even though Cancelling is non-terminal, no further pipeline work should start.

‎submitqueue/entity/request_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package entity
1616

1717
import (
18+
"slices"
1819
"testing"
1920

2021
"github.com/stretchr/testify/assert"
@@ -41,6 +42,28 @@ func TestIsRequestStateTerminal(t *testing.T) {
4142
}
4243
}
4344

45+
func TestInQueueRequestStates(t *testing.T) {
46+
tests := []struct {
47+
state RequestState
48+
inQueue bool
49+
}{
50+
{RequestStateStarted, true},
51+
{RequestStateValidated, true},
52+
{RequestStateBatched, true},
53+
{RequestStateProcessing, true},
54+
{RequestStateCancelling, true},
55+
{RequestStateLanded, false},
56+
{RequestStateError, false},
57+
{RequestStateCancelled, false},
58+
}
59+
for _, tt := range tests {
60+
t.Run(string(tt.state), func(t *testing.T) {
61+
assert.Equal(t, tt.inQueue, slices.Contains(InQueueRequestStates, tt.state))
62+
assert.NotEqual(t, tt.inQueue, IsRequestStateTerminal(tt.state), "a valid state is either in-queue or terminal")
63+
})
64+
}
65+
}
66+
4467
func TestIsRequestStateHalted(t *testing.T) {
4568
tests := []struct {
4669
state RequestState

‎submitqueue/extension/storage/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
"change_store.go",
1010
"path_build_store.go",
1111
"queue_batch_state_store.go",
12+
"queue_request_state_store.go",
1213
"request_batch_store.go",
1314
"request_log_store.go",
1415
"request_receipt_store.go",

‎submitqueue/extension/storage/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Store interfaces are designed for the storage technology *space*, not for SQL (s
6161

6262
**Domain state is often already the index.** Before adding any lookup, check whether an entity the caller already loads enumerates the children — an aggregate that references its parts by ID (e.g. a tree whose paths record their build identities) is the batch→children index, persisted and versioned as domain state. Duplicating that relationship as a database index adds a second source of truth for something the domain already owns.
6363

64-
**When neither applies, the reverse lookup is real — give it its own mapping store.** In the KV space there is no third mechanism: the only way to look up by an attribute is to make that attribute a primary key somewhere. So promote the relationship to a first-class mapping entity — keyed by the lookup attribute, written by the same flow that creates the source entity with idempotent puts, and rebuildable as a projection if it drifts. `ChangeRecord` is the in-repo example: it exists so "which requests claimed this change URI" is a by-key read on (queue, URI). `QueueBatchState` is the same pattern for a mutable attribute: "which batches of this queue are in this state" is a by-key read on (queue, state), maintained as advisory records that move buckets alongside the batch's own state CAS (the shared primitives in `submitqueue/orchestrator/core/batch` own that protocol) — it replaced `BatchStore.GetByQueueAndStates`, which was the contract's one query-by-attribute. Unlike a `KEY idx_*`, the relationship is visible in the contract and portable to any backend.
64+
**When neither applies, the reverse lookup is real — give it its own mapping store.** In the KV space there is no third mechanism: the only way to look up by an attribute is to make that attribute a primary key somewhere. So promote the relationship to a first-class mapping entity — keyed by the lookup attribute, written by the same flow that creates the source entity with idempotent puts, and rebuildable as a projection if it drifts. `ChangeRecord` is the in-repo example: it exists so "which requests claimed this change URI" is a by-key read on (queue, URI). `QueueBatchState` is the same pattern for a mutable attribute: "which batches of this queue are in this state" is a by-key read on (queue, state), maintained as advisory records that move buckets alongside the batch's own state CAS (the shared primitives in `submitqueue/orchestrator/core/batch` own that protocol) — it replaced `BatchStore.GetByQueueAndStates`, which was the contract's one query-by-attribute. `QueueRequestState` applies the pattern to requests and is exact rather than advisory: a backend writes each record as part of the request write it mirrors, so `QueueRequestStateStore` is read-only and needs no transition protocol. Unlike a `KEY idx_*`, the relationship is visible in the contract and portable to any backend.
6565

6666
### Decision path
6767

‎submitqueue/extension/storage/mock/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
"change_store_mock.go",
1010
"path_build_store_mock.go",
1111
"queue_batch_state_store_mock.go",
12+
"queue_request_state_store_mock.go",
1213
"request_batch_store_mock.go",
1314
"request_log_store_mock.go",
1415
"request_receipt_store_mock.go",

0 commit comments

Comments
 (0)