Skip to content

Commit f23a3c2

Browse files
prathshenoyprathshenoy
committed
feat(stovepipe): Show most broken projects first
1 parent 4ddb37d commit f23a3c2

8 files changed

Lines changed: 190 additions & 35 deletions

File tree

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ The request and response fields have the following API-level meaning. The publis
1717
| `queue` | Yes | Identifies the queue in which to find the validation request. |
1818
| `request_id` | ID lookup | Canonical positive decimal request ID returned by `Ingest`. |
1919
| `change_uri` | URI lookup | Identifies the exact commit URI under validation. |
20-
| `projects` | No | Consumer-defined project IDs whose results are requested. Omitted or empty returns all recorded project results. |
20+
| `projects` | No | Consumer-defined project IDs whose results are requested. When omitted or empty, returns recorded project results, most broken first. |
2121
| `page_size` | No | Limits one page of results for the requested projects. |
2222
| `page_token` | No | Continues a previous page of results for the requested projects. |
2323

@@ -29,7 +29,7 @@ The request and response fields have the following API-level meaning. The publis
2929
| `updated_at_ms` | Records, in Unix milliseconds, the newest durable lifecycle or result record represented by the response. |
3030
| `repository_breakage_degree` | Returns the whole-repository result when it is durably recorded. |
3131
| `project_results_complete` | Indicates whether the implementation has durably finished producing results for the supplied project IDs. |
32-
| `projects` | Returns recorded results for requested project IDs, or all recorded project results when none are supplied. |
32+
| `projects` | Returns recorded results for requested project IDs in the supplied order, or recorded project results ordered by descending breakage degree, then ascending project ID when none are supplied. |
3333
| `next_page_token` | Continues requested-project result pagination when another page exists. |
3434

3535
`request_state` is a stable public lifecycle vocabulary: `accepted`, `processing`, `succeeded`, `failed`, `cancelled`, or `superseded`. Clients must tolerate a future value. A terminal request state does not by itself mean that project results are complete.
@@ -64,15 +64,17 @@ 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 recorded project results by descending breakage degree, then ascending project ID, excluding the repository fact. If the result for the cursor project is missing, the API 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 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+
A dedicated ordering mapping makes listing a bounded primary-key range read, not a sort over all facts. Listing reads at most `page_size + 1` ordering mappings and hydrates only the returned facts, plus the cursor fact on continuations. The continuation token identifies the last project; its immutable fact supplies the degree needed to resume this ordering.
7678

7779
## Errors and Authorization
7880

‎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.ProjectFactByDegreeStore
165+
if len(req.Projects) == 0 {
166+
indexStore = store.GetProjectFactByDegreeStore()
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.ProjectFactByDegreeStore,
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.ProjectFactByDegreeStore,
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.ProjectFactByDegree
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.NewProjectFactByDegree(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 validation facts by degree 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.NewProjectFactByDegree(fact)
337+
if err != nil || entry != expected {
338+
return projectStatusPage{}, &ProjectStatusConsistencyError{message: "degree-ordered fact lookup 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.NewMockProjectFactByDegreeStore(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().GetProjectFactByDegreeStore().Return(indexStore)
128+
indexStore.EXPECT().List(gomock.Any(), projectStatusURI, entity.ProjectFactByDegree{}, 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.NewMockProjectFactByDegreeStore(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().GetProjectFactByDegreeStore().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.ProjectFactByDegree{}, 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.ProjectFactByDegree {
509+
t.Helper()
510+
var entries []entity.ProjectFactByDegree
511+
for _, fact := range facts {
512+
entry, err := entity.NewProjectFactByDegree(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.NewMockProjectFactByDegreeStore(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.NewProjectFactByDegree(fact)
535+
require.NoError(t, err)
536+
for _, tt := range []struct {
537+
name string
538+
modify func(*entity.ProjectFactByDegree, *entity.ValidationFact)
539+
factErr error
540+
}{
541+
{name: "missing fact", factErr: storage.ErrNotFound, modify: func(*entity.ProjectFactByDegree, *entity.ValidationFact) {}},
542+
{name: "read failure", factErr: errors.New("database unavailable"), modify: func(*entity.ProjectFactByDegree, *entity.ValidationFact) {}},
543+
{name: "wrong degree", modify: func(e *entity.ProjectFactByDegree, _ *entity.ValidationFact) { e.Degree = 0.75 }},
544+
{name: "wrong URI", modify: func(e *entity.ProjectFactByDegree, _ *entity.ValidationFact) { e.URI = "other" }},
545+
{name: "wrong request", modify: func(_ *entity.ProjectFactByDegree, f *entity.ValidationFact) { f.RequestID = "other" }},
546+
{name: "repository fact", modify: func(e *entity.ProjectFactByDegree, 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.NewMockProjectFactByDegreeStore(ctrl)
552+
e, f := entry, fact
553+
tt.modify(&e, &f)
554+
index.EXPECT().List(gomock.Any(), projectStatusURI, entity.ProjectFactByDegree{}, 3).Return([]entity.ProjectFactByDegree{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/entity/project_status.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ type GetProjectStatusResult struct {
4040
HasRepositoryValidationFact bool
4141
// ProjectResultsComplete reports whether the implementation has finished its project-result set.
4242
ProjectResultsComplete bool
43-
// ProjectValidationFacts contains recorded results in request order, or project-ID order when Projects is empty.
43+
// ProjectValidationFacts contains recorded results in request order, or descending degree then ascending project ID when Projects is empty.
4444
ProjectValidationFacts []ValidationFact
4545
// NextPageToken continues requested-project result pagination when another page exists.
4646
NextPageToken string

‎test/integration/stovepipe/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,14 @@ go_test(
1616
],
1717
deps = [
1818
"//api/stovepipe/protopb:go_default_library",
19+
"//stovepipe/entity:go_default_library",
20+
"//stovepipe/extension/storage/mysql:go_default_library",
1921
"//test/testutil:go_default_library",
2022
"@com_github_go_sql_driver_mysql//:go_default_library",
2123
"@com_github_stretchr_testify//assert:go_default_library",
2224
"@com_github_stretchr_testify//require:go_default_library",
2325
"@com_github_stretchr_testify//suite:go_default_library",
26+
"@com_github_uber_go_tally//:go_default_library",
2427
"@org_golang_google_grpc//:go_default_library",
2528
"@org_golang_google_protobuf//proto:go_default_library",
2629
],

0 commit comments

Comments
 (0)