Skip to content

Commit 1d76e2f

Browse files
committed
feat(stovepipe): Show most broken projects first
1 parent 6ea7eb7 commit 1d76e2f

33 files changed

Lines changed: 998 additions & 54 deletions

‎doc/rfc/stovepipe/project-status-api.md‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,25 @@ The caller may supply the project IDs for which it wants recorded results. The A
6464

6565
When project IDs are supplied, the response includes only results for those IDs, in the supplied order, and paginates that list. An implementation may record results for every requested project, only requested projects with a non-green result, or another documented subset.
6666

67-
With no project IDs supplied, the response lists all recorded project results in project-ID order, excluding the repository fact. Results are a live view; restart pagination to discover facts recorded behind the cursor.
67+
With no project IDs supplied, the response lists all recorded project results by descending breakage degree, then ascending project ID, excluding the repository fact. A dedicated ordering mapping makes this a bounded primary-key range read, not a sort over all facts. The continuation token identifies the last project; its immutable fact supplies the degree needed to resume this ordering. A missing cursor fact returns an error. Results are a live view; restart pagination to discover facts recorded before the cursor.
6868

6969
Each project result is an immutable validation fact keyed by queue, commit URI, and project ID. A result must identify the selected request. `project_results_complete` is false until the implementation has durably finished producing results for the supplied project IDs; a completed empty set is valid.
7070

7171
Callers use `project_results_complete`, rather than the presence or absence of an individual project result, to determine whether the implementation has finished producing results for the requested projects. `next_page_token` is empty only on the final page. A terminal lifecycle notification is emitted only after the repository result and any applicable project completion record are durable. If notification delivery fails, consumers can recover the same result through either endpoint.
7272

7373
## Implementation
7474

75-
Both endpoints return durable lifecycle summaries, the whole-repository fact when present, and recorded facts for requested projects in paginated request order, or all recorded project results in project-ID order when none are requested. `project_results_complete` remains false until a durable completion projection is available. Missing facts are omitted; an explicitly recorded zero degree remains present. `updated_at_ms` is the maximum timestamp of the summary and the facts represented on the page.
75+
Both endpoints return durable lifecycle summaries, the whole-repository fact when present, and recorded facts for requested projects in paginated request order, or all recorded project results by descending breakage degree then ascending project ID when none are requested. `project_results_complete` remains false until a durable completion projection is available. Missing facts are omitted; an explicitly recorded zero degree remains present. `updated_at_ms` is the maximum timestamp of the summary and the facts represented on the page.
76+
77+
The ordering mapping is keyed by `(queue, URI, degree_order, project)`. `degree_order` reverses nonnegative IEEE-754 degree bit patterns into an ascending integer key without rounding; positive and negative zero share one key. Binary identifiers preserve bytewise tie order. A page reads at most `page_size + 1` mappings and hydrates only the returned facts, plus the cursor fact on continuations. A mapping that disagrees with its authoritative fact is a consistency error.
78+
79+
The record controller persists the winning immutable fact before ensuring its mapping. Redelivery repairs missing mappings even when the fact already exists, and derives their keys from the stored degree rather than a losing candidate. Project-recording logs and downstream terminal notifications follow successful mapping writes. No cross-store transaction is required.
80+
81+
### Schema and rollout
82+
83+
The ordering table is additive; the original fact identity and primary key are unchanged. Install the table before deploying code that writes or reads it. Existing facts require a one-time backfill before index-based listing is enabled. `stovepipe/core/projectindex.Backfill` rebuilds one queue-scoped URI using bounded project-key scans and idempotent mapping writes; it is restartable and does not load all facts into memory.
84+
85+
A write-first rollout or a recording pause during cutover must prevent unindexed concurrent writes while historical backfill is verified. Do not switch readers merely because the new table exists: incomplete mappings would omit recorded facts. Schema deployment and historical backfill are operator-controlled, not performed by an RPC or service startup. Reverting readers does not require dropping the additive table.
7686

7787
## Errors and Authorization
7888

‎stovepipe/controller/project_status.go‎

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,11 @@ func (c *projectStatusController) GetProjectStatus(ctx context.Context, req enti
161161
}
162162
}
163163

164-
page, err := loadProjectStatusPage(ctx, req, requestSummary, factStore)
164+
var indexStore storage.ProjectValidationIndexStore
165+
if len(req.Projects) == 0 {
166+
indexStore = store.GetProjectValidationIndexStore()
167+
}
168+
page, err := loadProjectStatusPage(ctx, req, requestSummary, factStore, indexStore)
165169
if err != nil {
166170
return entity.GetProjectStatusResult{}, err
167171
}
@@ -221,6 +225,7 @@ func loadProjectStatusPage(
221225
req entity.GetProjectStatusRequest,
222226
summary entity.RequestSummary,
223227
store storage.ValidationFactStore,
228+
indexStore storage.ProjectValidationIndexStore,
224229
) (projectStatusPage, error) {
225230
token, err := parseProjectStatusPageToken(req, summary)
226231
if err != nil {
@@ -233,7 +238,7 @@ func loadProjectStatusPage(
233238
}
234239

235240
if len(req.Projects) == 0 {
236-
return listProjectStatusPage(ctx, summary, store, token, pageSize)
241+
return listProjectStatusPage(ctx, summary, store, indexStore, token, pageSize)
237242
}
238243

239244
return getRequestedProjectStatusPage(ctx, summary, store, token, pageSize)
@@ -282,32 +287,61 @@ func listProjectStatusPage(
282287
ctx context.Context,
283288
summary entity.RequestSummary,
284289
store storage.ValidationFactStore,
290+
indexStore storage.ProjectValidationIndexStore,
285291
token projectStatusPageToken,
286292
pageSize int,
287293
) (projectStatusPage, error) {
288-
facts, err := store.ListProjects(ctx, summary.URI, token.AfterProject, pageSize+1)
289-
if err != nil {
290-
return projectStatusPage{}, fmt.Errorf("GetProjectStatus failed to list project facts for request %q: %w", summary.RequestID, err)
291-
}
294+
var after entity.ProjectValidationIndex
295+
if token.AfterProject != "" {
296+
fact, err := store.Get(ctx, summary.URI, token.AfterProject)
297+
if err != nil {
298+
return projectStatusPage{}, fmt.Errorf("GetProjectStatus failed to load cursor project %q: %w", token.AfterProject, err)
299+
}
292300

293-
for _, fact := range facts {
294-
if err := validateProjectFact(fact, summary, fact.Project); err != nil {
301+
if err := validateProjectFact(fact, summary, token.AfterProject); err != nil {
295302
return projectStatusPage{}, err
296303
}
297304

298-
if fact.Project == "" {
299-
return projectStatusPage{}, &ProjectStatusConsistencyError{message: "project listing contains a repository fact"}
305+
after, err = entity.NewProjectValidationIndex(fact)
306+
if err != nil {
307+
return projectStatusPage{}, err
300308
}
301309
}
302310

303-
page := projectStatusPage{facts: facts}
304-
if len(facts) > pageSize {
305-
page.facts = facts[:pageSize]
306-
token.AfterProject = page.facts[pageSize-1].Project
311+
entries, err := indexStore.List(ctx, summary.URI, after, pageSize+1)
312+
if err != nil {
313+
return projectStatusPage{}, fmt.Errorf("GetProjectStatus failed to list project index for request %q: %w", summary.RequestID, err)
314+
}
315+
316+
var page projectStatusPage
317+
if len(entries) > pageSize {
318+
entries = entries[:pageSize]
319+
token.AfterProject = entries[pageSize-1].Project
307320
page.nextToken, err = encodeProjectStatusPageToken(token)
321+
if err != nil {
322+
return projectStatusPage{}, err
323+
}
308324
}
309325

310-
return page, err
326+
for _, entry := range entries {
327+
fact, err := store.Get(ctx, summary.URI, entry.Project)
328+
if err != nil {
329+
return projectStatusPage{}, fmt.Errorf("GetProjectStatus failed to load indexed project %q: %w", entry.Project, err)
330+
}
331+
332+
if err := validateProjectFact(fact, summary, entry.Project); err != nil {
333+
return projectStatusPage{}, err
334+
}
335+
336+
expected, err := entity.NewProjectValidationIndex(fact)
337+
if err != nil || entry != expected {
338+
return projectStatusPage{}, &ProjectStatusConsistencyError{message: "project ordering index disagrees with stored fact"}
339+
}
340+
341+
page.facts = append(page.facts, fact)
342+
}
343+
344+
return page, nil
311345
}
312346

313347
func encodeProjectStatusPageToken(token projectStatusPageToken) (string, error) {

‎stovepipe/controller/project_status_test.go‎

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ func TestGetProjectStatus(t *testing.T) {
111111
uriStore := storagemock.NewMockRequestURIStore(mockCtrl)
112112
summaryStore := storagemock.NewMockRequestSummaryStore(mockCtrl)
113113
factStore := storagemock.NewMockValidationFactStore(mockCtrl)
114+
indexStore := storagemock.NewMockProjectValidationIndexStore(mockCtrl)
114115
if !(tt.wantInvalid && tt.request.PageToken == "") {
115116
factory.EXPECT().For(storage.Config{QueueName: projectStatusQueue}).Return(store, nil)
116117
store.EXPECT().GetRequestURIStore().Return(uriStore)
@@ -123,7 +124,14 @@ func TestGetProjectStatus(t *testing.T) {
123124
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "").Return(tt.fact, tt.factErr)
124125
if !tt.noProjectLookup && !(tt.factErr == nil && tt.wantConsistency) {
125126
if len(tt.request.Projects) == 0 {
126-
factStore.EXPECT().ListProjects(gomock.Any(), projectStatusURI, "", defaultProjectStatusPageSize+1).Return(tt.projectFacts, tt.wantErr)
127+
store.EXPECT().GetProjectValidationIndexStore().Return(indexStore)
128+
indexStore.EXPECT().List(gomock.Any(), projectStatusURI, entity.ProjectValidationIndex{}, defaultProjectStatusPageSize+1).
129+
Return(projectStatusIndexEntries(t, tt.projectFacts), tt.wantErr)
130+
if tt.wantErr == nil {
131+
for _, fact := range tt.projectFacts {
132+
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, fact.Project).Return(fact, nil)
133+
}
134+
}
127135
}
128136
for _, project := range tt.request.Projects {
129137
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, project).Return(entity.ValidationFact{}, storage.ErrNotFound)
@@ -300,9 +308,9 @@ func TestGetProjectStatusReturnsProjectFactsInRequestedOrder(t *testing.T) {
300308
summaryStore.EXPECT().Get(gomock.Any(), projectStatusID).Return(requestSummary, nil)
301309
store.EXPECT().GetValidationFactStore().Return(factStore)
302310
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "").Return(entity.ValidationFact{}, storage.ErrNotFound)
303-
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-b").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-b", Degree: entity.DegreeBroken, CreatedAt: 20}, nil)
311+
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-b").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-b", Degree: entity.DegreeGreen, CreatedAt: 20}, nil)
304312
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "missing").Return(entity.ValidationFact{}, storage.ErrNotFound)
305-
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-a").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-a", Degree: entity.DegreeGreen, CreatedAt: 30}, nil)
313+
factStore.EXPECT().Get(gomock.Any(), projectStatusURI, "project-a").Return(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-a", Degree: entity.DegreeBroken, CreatedAt: 30}, nil)
306314

307315
controller := NewProjectStatusController(zap.NewNop().Sugar(), tally.NoopScope, factory)
308316
got, err := controller.GetProjectStatus(context.Background(), entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI, Projects: []string{"project-b", "missing", "project-a"}})
@@ -452,32 +460,38 @@ func TestParseProjectStatusPageTokenRejectsMismatches(t *testing.T) {
452460
func TestGetProjectStatusPaginatesStoredProjects(t *testing.T) {
453461
summary := entity.RequestSummary{RequestID: projectStatusID, Queue: projectStatusQueue, URI: projectStatusURI, StateTimestampMs: 10}
454462
stored := []entity.ValidationFact{
463+
{URI: projectStatusURI, RequestID: projectStatusID, Project: "c", Degree: 1, CreatedAt: 60},
455464
{URI: projectStatusURI, RequestID: projectStatusID, Project: "a", Degree: 0.5, CreatedAt: 20},
456465
{URI: projectStatusURI, RequestID: projectStatusID, Project: "b", Degree: 0, CreatedAt: 50},
457-
{URI: projectStatusURI, RequestID: projectStatusID, Project: "c", Degree: 1, CreatedAt: 60},
458466
}
459467

460468
ctrl := gomock.NewController(t)
461469
factory := storagemock.NewMockFactory(ctrl)
462470
store := storagemock.NewMockStorage(ctrl)
463471
summaries := storagemock.NewMockRequestSummaryStore(ctrl)
464472
facts := storagemock.NewMockValidationFactStore(ctrl)
473+
indexStore := storagemock.NewMockProjectValidationIndexStore(ctrl)
474+
entries := projectStatusIndexEntries(t, stored)
465475

466476
factory.EXPECT().For(storage.Config{QueueName: projectStatusQueue}).Return(store, nil).Times(2)
467477
store.EXPECT().GetRequestSummaryStore().Return(summaries).Times(2)
468478
summaries.EXPECT().Get(gomock.Any(), projectStatusID).Return(summary, nil).Times(2)
469479
store.EXPECT().GetValidationFactStore().Return(facts).Times(2)
480+
store.EXPECT().GetProjectValidationIndexStore().Return(indexStore).Times(2)
470481
facts.EXPECT().Get(gomock.Any(), projectStatusURI, "").Return(entity.ValidationFact{}, storage.ErrNotFound).Times(2)
471-
facts.EXPECT().ListProjects(gomock.Any(), projectStatusURI, "", 3).Return(stored, nil)
472-
facts.EXPECT().ListProjects(gomock.Any(), projectStatusURI, "b", defaultProjectStatusPageSize+1).Return(stored[2:], nil)
482+
indexStore.EXPECT().List(gomock.Any(), projectStatusURI, entity.ProjectValidationIndex{}, 3).Return(entries, nil)
483+
indexStore.EXPECT().List(gomock.Any(), projectStatusURI, entries[1], defaultProjectStatusPageSize+1).Return(entries[2:], nil)
484+
facts.EXPECT().Get(gomock.Any(), projectStatusURI, "c").Return(stored[0], nil)
485+
facts.EXPECT().Get(gomock.Any(), projectStatusURI, "a").Return(stored[1], nil).Times(2)
486+
facts.EXPECT().Get(gomock.Any(), projectStatusURI, "b").Return(stored[2], nil)
473487

474488
controller := NewProjectStatusController(zap.NewNop().Sugar(), tally.NoopScope, factory)
475489
req := entity.GetProjectStatusRequest{Queue: projectStatusQueue, RequestID: projectStatusID, PageSize: 2}
476490

477491
first, err := controller.GetProjectStatus(context.Background(), req)
478492
require.NoError(t, err)
479493
assert.Equal(t, stored[:2], first.ProjectValidationFacts)
480-
assert.Equal(t, int64(50), first.UpdatedAtMs)
494+
assert.Equal(t, int64(60), first.UpdatedAtMs)
481495
require.NotEmpty(t, first.NextPageToken)
482496

483497
req.PageToken = first.NextPageToken
@@ -487,10 +501,69 @@ func TestGetProjectStatusPaginatesStoredProjects(t *testing.T) {
487501
second, err := controller.GetProjectStatus(context.Background(), req)
488502
require.NoError(t, err)
489503
assert.Equal(t, stored[2:], second.ProjectValidationFacts)
490-
assert.Equal(t, int64(60), second.UpdatedAtMs)
504+
assert.Equal(t, int64(50), second.UpdatedAtMs)
491505
assert.Empty(t, second.NextPageToken)
492506
}
493507

508+
func projectStatusIndexEntries(t *testing.T, facts []entity.ValidationFact) []entity.ProjectValidationIndex {
509+
t.Helper()
510+
var entries []entity.ProjectValidationIndex
511+
for _, fact := range facts {
512+
entry, err := entity.NewProjectValidationIndex(fact)
513+
require.NoError(t, err)
514+
entries = append(entries, entry)
515+
}
516+
return entries
517+
}
518+
519+
func TestListProjectStatusPageRejectsMissingCursor(t *testing.T) {
520+
for _, readErr := range []error{storage.ErrNotFound, errors.New("database unavailable")} {
521+
t.Run(readErr.Error(), func(t *testing.T) {
522+
ctrl := gomock.NewController(t)
523+
facts := storagemock.NewMockValidationFactStore(ctrl)
524+
index := storagemock.NewMockProjectValidationIndexStore(ctrl)
525+
facts.EXPECT().Get(gomock.Any(), projectStatusURI, "cursor").Return(entity.ValidationFact{}, readErr)
526+
_, err := listProjectStatusPage(context.Background(), entity.RequestSummary{URI: projectStatusURI}, facts, index, projectStatusPageToken{AfterProject: "cursor"}, 2)
527+
require.ErrorIs(t, err, readErr)
528+
})
529+
}
530+
}
531+
532+
func TestListProjectStatusPageRejectsInconsistentIndex(t *testing.T) {
533+
fact := entity.ValidationFact{URI: projectStatusURI, Project: "p", Degree: 0.5, RequestID: projectStatusID}
534+
entry, err := entity.NewProjectValidationIndex(fact)
535+
require.NoError(t, err)
536+
for _, tt := range []struct {
537+
name string
538+
modify func(*entity.ProjectValidationIndex, *entity.ValidationFact)
539+
factErr error
540+
}{
541+
{name: "missing fact", factErr: storage.ErrNotFound, modify: func(*entity.ProjectValidationIndex, *entity.ValidationFact) {}},
542+
{name: "read failure", factErr: errors.New("database unavailable"), modify: func(*entity.ProjectValidationIndex, *entity.ValidationFact) {}},
543+
{name: "wrong order key", modify: func(e *entity.ProjectValidationIndex, _ *entity.ValidationFact) { e.DegreeOrder++ }},
544+
{name: "wrong URI", modify: func(e *entity.ProjectValidationIndex, _ *entity.ValidationFact) { e.URI = "other" }},
545+
{name: "wrong request", modify: func(_ *entity.ProjectValidationIndex, f *entity.ValidationFact) { f.RequestID = "other" }},
546+
{name: "repository fact", modify: func(e *entity.ProjectValidationIndex, f *entity.ValidationFact) { e.Project = ""; f.Project = "" }},
547+
} {
548+
t.Run(tt.name, func(t *testing.T) {
549+
ctrl := gomock.NewController(t)
550+
facts := storagemock.NewMockValidationFactStore(ctrl)
551+
index := storagemock.NewMockProjectValidationIndexStore(ctrl)
552+
e, f := entry, fact
553+
tt.modify(&e, &f)
554+
index.EXPECT().List(gomock.Any(), projectStatusURI, entity.ProjectValidationIndex{}, 3).Return([]entity.ProjectValidationIndex{e}, nil)
555+
facts.EXPECT().Get(gomock.Any(), projectStatusURI, e.Project).Return(f, tt.factErr)
556+
_, err := listProjectStatusPage(context.Background(), entity.RequestSummary{URI: projectStatusURI, RequestID: projectStatusID}, facts, index, projectStatusPageToken{}, 2)
557+
require.Error(t, err)
558+
if tt.factErr != nil {
559+
assert.ErrorIs(t, err, tt.factErr)
560+
} else {
561+
assert.True(t, IsProjectStatusConsistency(err))
562+
}
563+
})
564+
}
565+
}
566+
494567
func TestValidateProjectFact(t *testing.T) {
495568
requestSummary := entity.RequestSummary{RequestID: projectStatusID, URI: projectStatusURI}
496569
assert.NoError(t, validateProjectFact(entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Project: "project-a", Degree: entity.DegreeGreen}, requestSummary, "project-a"))

‎stovepipe/controller/record/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
"//stovepipe/core/hookevent:go_default_library",
1818
"//stovepipe/core/loader:go_default_library",
1919
"//stovepipe/core/messagequeue:go_default_library",
20+
"//stovepipe/core/projectindex:go_default_library",
2021
"//stovepipe/core/requestlog:go_default_library",
2122
"//stovepipe/entity:go_default_library",
2223
"//stovepipe/extension/projectstatus:go_default_library",

‎stovepipe/controller/record/record.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"github.com/uber/submitqueue/stovepipe/core/hookevent"
4747
"github.com/uber/submitqueue/stovepipe/core/loader"
4848
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
49+
"github.com/uber/submitqueue/stovepipe/core/projectindex"
4950
"github.com/uber/submitqueue/stovepipe/core/requestlog"
5051
"github.com/uber/submitqueue/stovepipe/entity"
5152
"github.com/uber/submitqueue/stovepipe/extension/projectstatus"
@@ -197,7 +198,7 @@ func (c *Controller) recordProjectFacts(ctx context.Context, store storage.Stora
197198
return err
198199
}
199200
for _, result := range results {
200-
_, _, err := c.recordValidationFact(ctx, store.GetValidationFactStore(), entity.ValidationFact{
201+
fact, _, err := c.recordValidationFact(ctx, store.GetValidationFactStore(), entity.ValidationFact{
201202
URI: request.URI,
202203
Project: result.Project,
203204
Degree: result.Degree,
@@ -207,6 +208,10 @@ func (c *Controller) recordProjectFacts(ctx context.Context, store storage.Stora
207208
if err != nil {
208209
return err
209210
}
211+
212+
if err := projectindex.Ensure(ctx, store.GetProjectValidationIndexStore(), fact); err != nil {
213+
return fmt.Errorf("failed to index project fact for request %q and project %q: %w", request.ID, fact.Project, err)
214+
}
210215
}
211216
if len(results) == 0 {
212217
return nil

0 commit comments

Comments
 (0)