Skip to content

Commit 25bc437

Browse files
authored
feat(stovepipe): Recover failed issue tracking (#961)
## Summary **What**: - Add dead-letter recovery that stops failed issue-tracking passes while keeping completed issue changes. - Record incomplete issue tracking without changing the build's validation outcome. **Why**: - Unblock later builds when issue tracking exhausts normal retries. Today the unfinished pass remains saved, so later notifications try to resume the same failed work. Recovery needs to stop that pass without throwing away issue changes that already succeeded. - Keep delayed workers from changing issue state after recovery. Simply clearing progress would let a paused worker write an old opening or closing after a newer build has run. Recovery protects the current issue and retains its history before allowing newer work through. ## Test Plan - [x] Add unit tests. - [x] Verify recovery progress persists against a real MySQL database. ## Revert Plan - Revert this PR before enabling consumption. If already enabled, stop consumers and reconcile recovery progress before rolling back so an older consumer does not resume abandoned work. ## Issue Links - [CODEM-607](https://linear.app/uber/issue/CODEM-607/build-analysis) ## Stack 1. #942 1. #948 1. #949 1. #950 1. #951 1. #958 1. #961 1. #962
1 parent 69feef3 commit 25bc437

13 files changed

Lines changed: 744 additions & 10 deletions

File tree

‎doc/rfc/stovepipe/request-log.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ Stovepipe retains an append-only request log for each validation request. Its in
99
- `validation_fact_recorded`;
1010
- `project_facts_recorded`;
1111
- `record_abandoned`;
12-
- `build_analysis_abandoned`.
12+
- `build_analysis_abandoned`;
13+
- `issue_tracking_abandoned`.
1314

1415
The model deliberately follows SubmitQueue's distinction between statuses describing where a request is and events describing important activity that does not move it. It remains a bounded request-lifecycle log rather than a generic event bus or an audit of every correlated operation.
1516

@@ -128,6 +129,7 @@ Immutable Request context such as URI, build strategy, base URI, and terminal bu
128129
| `project_facts_recorded` | All project validation facts returned for a request became durable. | Project-fact count metadata and completion time |
129130
| `record_abandoned` | Record-stage work stopped after exhausting primary retries. | Event retention time |
130131
| `build_analysis_abandoned` | Build analysis could not complete; issue observations may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time |
132+
| `issue_tracking_abandoned` | Issue tracking stopped before completing a pass; existing issue changes remain, but reconciliation may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time |
131133

132134
Build running and unchanged polls are not retained. Trigger and terminal result explain the request outcome without turning polling into an unbounded log. Individual project facts remain outside the vocabulary; one batch-completion event summarizes their durable recording.
133135

@@ -219,6 +221,7 @@ The `project_facts_recorded` event also projects its retained timestamp into `Re
219221
| Record | Create or verify the whole-repository fact, then retain `validation_fact_recorded`; after recording project facts, retain one `project_facts_recorded` event. | An identical whole-repository fact and project-fact batch ensure their events before bookmark or promotion work. |
220222
| Record DLQ | Retain `record_abandoned`; for failed Requests, create or verify the repository fact and retain `validation_fact_recorded` before publishing its hook. | Existing facts remain authoritative; stable log and hook IDs make replay idempotent without resolving projects or replaying promotion. |
221223
| Build analysis DLQ | Retain `build_analysis_abandoned` without rerunning detection or changing validation state. | One stable event per build deduplicates recovery deliveries; only state reads and history retention retry. |
224+
| Issue tracking DLQ | Protect the current cursor from delayed writes, retain its issue history and `issue_tracking_abandoned`, then mark the pass abandoned. | A stable hash of the notification ID deduplicates the event. Recovery retains accepted issue changes and never processes the remaining range; consumer registration is separate. |
222225
| Reconciler | CAS an unrecoverable non-terminal Request to failed, then retain failed. | An existing terminal Request is repaired from its persisted outcome without relabeling it. |
223226

224227
Build running and unchanged polls create no entry. A failed runner trigger that creates no Build creates no event. Cancelled and superseded requests create no validation fact.

‎stovepipe/controller/issuetrack/BUILD.bazel‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
5-
srcs = ["issuetrack.go"],
5+
srcs = [
6+
"dlq.go",
7+
"issuetrack.go",
8+
],
69
importpath = "github.com/uber/submitqueue/stovepipe/controller/issuetrack",
710
visibility = ["//visibility:public"],
811
deps = [
@@ -13,6 +16,7 @@ go_library(
1316
"//stovepipe/core/fixrequest:go_default_library",
1417
"//stovepipe/core/loader:go_default_library",
1518
"//stovepipe/core/messagequeue:go_default_library",
19+
"//stovepipe/core/requestlog:go_default_library",
1620
"//stovepipe/entity:go_default_library",
1721
"//stovepipe/extension/storage:go_default_library",
1822
"@com_github_uber_go_tally//:go_default_library",
@@ -24,17 +28,21 @@ go_test(
2428
name = "go_default_test",
2529
srcs = [
2630
"delivery_test.go",
31+
"dlq_test.go",
2732
"fix_handoff_test.go",
2833
"fixture_test.go",
2934
"issuetrack_test.go",
3035
],
3136
embed = [":go_default_library"],
3237
deps = [
38+
"//platform/base/failure:go_default_library",
3339
"//platform/base/messagequeue:go_default_library",
3440
"//platform/consumer:go_default_library",
3541
"//platform/consumer/mock:go_default_library",
3642
"//platform/extension/messagequeue/mock:go_default_library",
3743
"//stovepipe/core/messagequeue:go_default_library",
44+
"//stovepipe/core/requestlog:go_default_library",
45+
"//stovepipe/core/requestlog/mock:go_default_library",
3846
"//stovepipe/entity:go_default_library",
3947
"//stovepipe/extension/storage:go_default_library",
4048
"//stovepipe/extension/storage/mock:go_default_library",

‎stovepipe/controller/issuetrack/README.md‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,27 @@ No closure occurs before all accumulated build issues have been processed. A tas
3838

3939
Every delivery processes at most 100 task/unit keys from one ordered range, including a revisited cursor and retained closed rows. Refreshing reads build issues; finalizing reads issue states. The cursor is always inclusive: revisit it to finish the issue and its history, then continue after it. A full batch retains its active phase and requests redelivery. Moving from refreshing to finalizing conditionally saves a reset cursor before any closing work begins.
4040

41-
An unfinished pass completes before another source is admitted. A notification that helps finish a different pass requests redelivery before handling its own source. Completed notifications and older sources are ignored, as are refreshes after the same Request has finalized. A terminal pass requires a terminal build chosen by its Request in both active phases.
41+
An unfinished pass completes or is safely abandoned before another source is admitted. A notification that helps finish a different pass requests redelivery before handling its own source. Completed notifications and older sources are ignored, as are refreshes after the same Request has finalized or its terminal pass was abandoned. A terminal pass requires a terminal build chosen by its Request in both active phases.
4242

4343
Build IDs are opaque; source order comes from Request IDs. All scans are bounded reads under leading primary-key prefixes. There is no transaction spanning progress, issue state, and history.
4444

4545
## Recovery and activation
4646

47-
An error returns to normal consumer retry. The next delivery resumes the cursor before advancing. Permanently failing cursor work blocks issue tracking for that queue; terminal recovery and consumer registration remain follow-up work. This PR does not enable consumption. The host must provide a fix publication route and a repair executor when registering the fix consumer.
47+
An error returns to normal consumer retry. The next delivery resumes the cursor before advancing. Once that retry policy dead-letters a notification, the recovery controller abandons the failed pass rather than rerunning its remaining batches:
48+
49+
1. Conditionally enter `abandoning`, retaining the source, notification, and cursor. Normal deliveries hold for 30 seconds while this phase is present; they cannot start another pass or resume issue application.
50+
2. If there is a cursor, read its issue and conditionally advance progress after that read. Advance the issue's version without changing its status or transition metadata. This invalidates a paused worker's earlier row guard. If the row has not been created yet, finish that one opening instead: the normal worker can only have saved such a cursor after finding a reported build issue. Creating it prevents the delayed create from succeeding. Closed issue rows are never deleted.
51+
3. Repair the cursor's opening or closing history from the retained row. Record an `issue_tracking_abandoned` Request-history event keyed by the failed notification, with its build ID.
52+
4. Conditionally enter `abandoned`. Keep the source-order boundary and notification identity, so the same delivery and older builds cannot restart the abandoned work.
53+
54+
Accepted opens and closes remain in place. Recovery does not scan either issue range, close the remaining issues, change Request or Build outcomes, or release build slots. An abandoned ordinary refresh still permits a new refresh or terminal notification for that Request; an abandoned terminal pass permits only newer Requests. A later successful build can reconcile issues left unchanged by the failed pass.
55+
56+
Recovery must finish fencing the current issue and retaining its history before admitting newer work. If a normal write wins the issue CAS first, recovery retries against that accepted row. If a replacement recovery worker advances first, its issue-version change rejects the old write. Merely marking the pass abandoned without touching the cursor row would not provide this protection.
57+
58+
A notification can fail while helping an older pass. Recovery abandons that saved pass first, then requests immediate redelivery to abandon the failed notification's own work if it has not already been superseded. A delayed DLQ delivery never abandons a newer build or a different notification from the same Request.
59+
60+
Storage and history failures return for retry; hosts must pair recovery with an always-retryable, unlimited-retry DLQ consumer. Only recovery work repeats. Malformed message identities, unresolvable queues, and missing source records are logged and acknowledged. No normal or DLQ consumer is registered yet. Completed opening passes publish repair notifications; recovery does not. The host must provide a fix publication route and a repair executor when registering the fix consumer.
4861

4962
## Analysis handoff
5063

51-
Build analysis publishes refresh or final notifications after persisting build issues. Final notification requires successful terminal detection and the Request's recorded terminal build. Terminal detection returns the complete cumulative issue set, including issues returned by earlier calls, so finalization does not depend on earlier workers finishing their writes. Nonterminal detection may omit earlier issues without removing them from the build's accumulated results. The reference host registers the publication route but does not enable issue-tracking consumption or recovery.
64+
Build analysis publishes refresh or final notifications after persisting build issues. Final notification requires successful terminal detection and the Request's recorded terminal build. Terminal detection returns the complete cumulative issue set, including issues returned by earlier calls, so finalization does not depend on earlier workers finishing their writes. Nonterminal detection may omit earlier issues without removing them from the build's accumulated results. The reference host registers the publication route but does not enable either issue-tracking consumer.

0 commit comments

Comments
 (0)