Skip to content

Commit 75ef1f2

Browse files
albertywuJamyDev
authored andcommitted
docs(orchestrator): document controller correctness (#398)
## Summary Document controller correctness guidelines ## Test Plan ✅ `make fmt && make build && make test && make e2e-test` ## Issues
1 parent fe576ef commit 75ef1f2

1 file changed

Lines changed: 112 additions & 0 deletions

File tree

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Controller Correctness
2+
3+
SubmitQueue controllers are built around eventual consistency. Controllers advance a workflow through durable state checkpoints, and every component must tolerate retries before the next checkpoint is recorded.
4+
5+
The core model is:
6+
7+
> Load durable state, reconcile it toward this controller's checkpoint, then replay the checkpoint's fanout until it is accepted.
8+
9+
Optimistic locking protects checkpoints from concurrent writers. Failures and races are expected to be uncommon, so the system may leave harmless partial or orphaned data from attempts that never reached a checkpoint. That data can be cleaned up separately if it becomes a problem.
10+
11+
## Checkpoint pattern
12+
13+
Each controller owns a small set of state transitions. It must classify the latest state before writing:
14+
15+
```text
16+
Process(message):
17+
entity = load latest durable state
18+
19+
if state is before my checkpoint:
20+
perform retry-safe preparation
21+
record checkpoint with optimistic locking
22+
if the version changed:
23+
return ErrVersionMismatch
24+
25+
if state is at my checkpoint:
26+
replay complete fanout using stable message identities
27+
return success
28+
29+
if state is beyond or supersedes my checkpoint:
30+
return success
31+
32+
return invalid-state error
33+
```
34+
35+
The important states are:
36+
37+
| State relative to this controller | Behavior |
38+
|---|---|
39+
| Before checkpoint | Perform retry-safe work and record the checkpoint. |
40+
| At checkpoint | Skip the state transition and replay the complete fanout. |
41+
| Beyond checkpoint | A downstream controller already consumed the handoff. Acknowledge without regressing state. |
42+
| Superseded | Cancellation, failure, or another outcome made this work unnecessary. |
43+
| Invalid | Return an error rather than inventing a transition. |
44+
45+
## Retry and redelivery
46+
47+
Prefer one reconciliation pass per delivery. The consumer framework is the retry loop:
48+
49+
```text
50+
controller returns error
51+
-> error processor classifies it
52+
-> consumer nacks retryable errors
53+
-> redelivery re-enters Process and reloads durable state
54+
```
55+
56+
Controllers should not classify ordinary backend failures merely because replay would be convenient. Return the raw wrapped error and let the configured classifiers decide whether it is transient. A permanent publish or storage failure must eventually reach the DLQ rather than retry forever.
57+
58+
## Persist before publishing
59+
60+
For a state transition followed by queue fanout:
61+
62+
```text
63+
persist checkpoint
64+
publish complete fanout
65+
ack delivery
66+
```
67+
68+
The checkpoint proves that the state transition happened. It does not prove that every output was published.
69+
70+
If a process fails after recording the checkpoint, redelivery observes the checkpoint, skips the transition, and republishes the complete fanout. Every replayed output must use the same topic, partition key, logical message ID, and payload.
71+
72+
## Optimistic locking
73+
74+
Optimistic locking answers whether an entity changed since it was read. It does not decide whether a lifecycle transition is valid.
75+
76+
A controller must write only from states it owns. For example, score may transition `Created` to `Scored`; it must not load `Speculating` and write it back to `Scored`.
77+
78+
Version arithmetic follows the [storage optimistic-locking contract](../../extension/storage/README.md): compute the new version in the controller and update the in-memory entity only after the write succeeds.
79+
80+
## Example: score
81+
82+
| Batch state | Behavior |
83+
|---|---|
84+
| `Created` | Compute the score and conditionally record `Scored`. |
85+
| `Scored` | Preserve the durable score and replay logs plus `speculate`. |
86+
| `Speculating`, `Merging` | Acknowledge without regressing the batch. |
87+
| `Cancelling`, terminal | The transition was superseded or another controller owns recovery. |
88+
89+
If publishing fails after the batch reaches `Scored`, the controller returns an error. Redelivery reloads `Scored`, skips rescoring, and republishes the fanout with stable message identities.
90+
91+
## External effects
92+
93+
An external effect whose outcome was not recorded cannot be made safe by queue deduplication alone:
94+
95+
```text
96+
provider accepts operation
97+
controller fails before recording the result
98+
```
99+
100+
Such effects require a provider-supported idempotency key, a stable operation identity that can be queried, or an explicit acceptance that duplicate or orphaned work is harmless.
101+
102+
## Review checklist
103+
104+
For each controller, make these answers clear:
105+
106+
1. What durable checkpoint does it own?
107+
2. Is all work before that checkpoint safe to retry?
108+
3. How does each possible durable state classify relative to the checkpoint?
109+
4. Can the complete fanout be reconstructed and replayed with stable message identities?
110+
5. Which controller or DLQ path owns superseded and terminal recovery?
111+
112+
See the [consumer error contract](../../../platform/consumer/README.md), the [orchestrator workflow](../../../doc/rfc/submitqueue/workflow.md), and the [SQL queue RFC](../../../doc/rfc/sql-queue-rfc.md).

0 commit comments

Comments
 (0)