Skip to content

Commit 80153ea

Browse files
dutifulbobdutifulbob
authored
perf: instrument read query paths
Add request-level DB metrics and batch annotation hydration for hot read paths.
1 parent cbf54f7 commit 80153ea

9 files changed

Lines changed: 591 additions & 51 deletions

File tree

‎internal/core/hydration_test.go‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
package core
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
"github.com/dutifuldev/prtags/internal/database"
9+
"github.com/stretchr/testify/require"
10+
"gorm.io/datatypes"
11+
)
12+
13+
func TestBatchAnnotationHydrationHelpers(t *testing.T) {
14+
ctx := context.Background()
15+
service, _, server := newTestService(t)
16+
defer server.Close()
17+
18+
actor := permissionsActor()
19+
_, err := service.CreateFieldDefinition(ctx, actor, "acme", "widgets", FieldDefinitionInput{
20+
Name: "intent",
21+
ObjectScope: "pull_request",
22+
FieldType: "text",
23+
}, "")
24+
require.NoError(t, err)
25+
_, err = service.SetAnnotations(ctx, actor, "acme", "widgets", "pull_request", 22, nil, map[string]any{
26+
"intent": "batch annotations",
27+
}, "")
28+
require.NoError(t, err)
29+
30+
require.Empty(t, annotationTargetsForSearchRows([]scoredSearchTarget{{TargetType: "pull_request"}}))
31+
require.Empty(t, annotationTargetsForFieldValues([]database.FieldValue{{TargetType: "pull_request"}}))
32+
33+
targetKey := objectTargetKey(101, "pull_request", 22)
34+
empty, err := service.getAnnotationsForTargetKeys(ctx, 101, []annotationTarget{{}})
35+
require.NoError(t, err)
36+
require.Empty(t, empty)
37+
38+
annotations, err := service.getAnnotationsForTargetKeys(ctx, 101, []annotationTarget{
39+
{targetType: "pull_request", targetKey: targetKey},
40+
{targetType: "pull_request", targetKey: targetKey},
41+
{targetType: "issue", targetKey: objectTargetKey(101, "issue", 11)},
42+
{},
43+
})
44+
require.NoError(t, err)
45+
require.Equal(t, "batch annotations", annotations[annotationMapKey("pull_request", targetKey)]["intent"])
46+
require.Empty(t, annotations[annotationMapKey("issue", objectTargetKey(101, "issue", 11))])
47+
}
48+
49+
func TestSearchHydrationHelpersCoverInvalidAndGroupTargets(t *testing.T) {
50+
ctx := context.Background()
51+
service, _, server := newTestService(t)
52+
defer server.Close()
53+
54+
group, err := service.CreateGroup(ctx, permissionsActor(), "acme", "widgets", GroupInput{Kind: "mixed", Title: "Auth"}, "")
55+
require.NoError(t, err)
56+
57+
targetKey := objectTargetKey(101, "pull_request", 22)
58+
annotations := map[string]map[string]any{
59+
annotationMapKey("pull_request", targetKey): {"intent": "retry"},
60+
annotationMapKey("group", groupTargetKey(group.PublicID)): {"theme": "auth"},
61+
}
62+
summaries := map[string]GroupMemberObjectSummary{
63+
targetKey: {Title: "Retry auth", State: "open", UpdatedAt: time.Now().UTC()},
64+
}
65+
66+
objectResult, err := service.populateObjectSearchResultWithHydration("pull_request", targetKey, TextSearchResult{TargetType: "pull_request", TargetKey: targetKey}, summaries, annotations)
67+
require.NoError(t, err)
68+
require.Equal(t, "Retry auth", objectResult.ObjectSummary.Title)
69+
require.Equal(t, "retry", objectResult.Annotations["intent"])
70+
71+
invalidObject, err := service.populateObjectSearchResultWithHydration("pull_request", "bad-key", TextSearchResult{TargetKey: "bad-key"}, summaries, annotations)
72+
require.NoError(t, err)
73+
require.Nil(t, invalidObject.Annotations)
74+
75+
groupResult, err := service.populateGroupSearchResultWithAnnotations(ctx, groupTargetKey(group.PublicID), TextSearchResult{TargetType: "group", TargetKey: groupTargetKey(group.PublicID)}, annotations)
76+
require.NoError(t, err)
77+
require.Equal(t, group.PublicID, groupResult.ID)
78+
require.Equal(t, "auth", groupResult.Annotations["theme"])
79+
80+
invalidGroup, err := service.populateGroupSearchResultWithAnnotations(ctx, "bad-key", TextSearchResult{TargetKey: "bad-key"}, annotations)
81+
require.NoError(t, err)
82+
require.Empty(t, invalidGroup.ID)
83+
84+
_, err = service.populateGroupSearchResultWithAnnotations(ctx, groupTargetKey("missing"), TextSearchResult{TargetType: "group", TargetKey: groupTargetKey("missing")}, annotations)
85+
require.Error(t, err)
86+
}
87+
88+
func TestFilterHydrationHelpersCoverBatchBranches(t *testing.T) {
89+
ctx := context.Background()
90+
service, _, server := newTestService(t)
91+
defer server.Close()
92+
93+
group, err := service.CreateGroup(ctx, permissionsActor(), "acme", "widgets", GroupInput{Kind: "mixed", Title: "Auth"}, "")
94+
require.NoError(t, err)
95+
96+
values := []database.FieldValue{
97+
{
98+
GitHubRepositoryID: 101,
99+
TargetType: "pull_request",
100+
ObjectNumber: intPtr(22),
101+
TargetKey: objectTargetKey(101, "pull_request", 22),
102+
MultiEnumJSON: datatypes.JSON(`["bug","auth"]`),
103+
},
104+
{
105+
GitHubRepositoryID: 101,
106+
TargetType: "issue",
107+
ObjectNumber: intPtr(11),
108+
TargetKey: objectTargetKey(101, "issue", 11),
109+
MultiEnumJSON: datatypes.JSON(`["docs"]`),
110+
},
111+
}
112+
113+
nonMulti, err := matchingFieldValues("text", "auth", values)
114+
require.NoError(t, err)
115+
require.Len(t, nonMulti, 2)
116+
matched, err := matchingFieldValues("multi_enum", "auth", values)
117+
require.NoError(t, err)
118+
require.Len(t, matched, 1)
119+
_, err = matchingFieldValues("multi_enum", "auth", []database.FieldValue{{MultiEnumJSON: datatypes.JSON(`{`)}})
120+
require.Error(t, err)
121+
122+
emptySummaries, err := service.filteredTargetSummaries(ctx, 101, []database.FieldValue{{TargetType: "group"}})
123+
require.NoError(t, err)
124+
require.Empty(t, emptySummaries)
125+
summaries, err := service.filteredTargetSummaries(ctx, 101, values)
126+
require.NoError(t, err)
127+
require.Equal(t, "Retry ACP turns safely", summaries[objectTargetKey(101, "pull_request", 22)].Title)
128+
129+
require.Empty(t, groupIDsForFieldValues(values))
130+
groupValue := database.FieldValue{
131+
GitHubRepositoryID: 101,
132+
TargetType: "group",
133+
TargetKey: groupTargetKey(group.PublicID),
134+
GroupID: &group.ID,
135+
}
136+
require.Equal(t, []uint{group.ID}, groupIDsForFieldValues([]database.FieldValue{groupValue, groupValue}))
137+
138+
emptyGroups, err := service.groupsByID(ctx, nil)
139+
require.NoError(t, err)
140+
require.Empty(t, emptyGroups)
141+
groups, err := service.groupsByID(ctx, []uint{group.ID})
142+
require.NoError(t, err)
143+
require.Equal(t, group.PublicID, groups[group.ID].PublicID)
144+
145+
annotations := map[string]map[string]any{
146+
annotationMapKey("pull_request", objectTargetKey(101, "pull_request", 22)): {"intent": "retry"},
147+
annotationMapKey("group", groupTargetKey(group.PublicID)): {"theme": "auth"},
148+
}
149+
objectResult, err := filteredTargetResultFromHydration(values[0], summaries, annotations, groups)
150+
require.NoError(t, err)
151+
require.Equal(t, 22, objectResult.ObjectNumber)
152+
require.Equal(t, "retry", objectResult.Annotations["intent"])
153+
require.Equal(t, "Retry ACP turns safely", objectResult.ObjectSummary.Title)
154+
155+
groupResult, err := filteredTargetResultFromHydration(groupValue, summaries, annotations, groups)
156+
require.NoError(t, err)
157+
require.Equal(t, group.PublicID, groupResult.ID)
158+
require.Equal(t, "auth", groupResult.Annotations["theme"])
159+
160+
groupMissing := groupValue
161+
missingID := group.ID + 999
162+
groupMissing.GroupID = &missingID
163+
_, err = filteredTargetResultFromHydration(groupMissing, summaries, nil, groups)
164+
require.ErrorIs(t, err, ErrNotFound)
165+
166+
noAnnotations, err := filteredTargetResultFromHydration(values[1], summaries, nil, groups)
167+
require.NoError(t, err)
168+
require.Empty(t, noAnnotations.Annotations)
169+
}

‎internal/core/indexing.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -511,16 +511,20 @@ func (s *Service) resolveSearchResults(ctx context.Context, repositoryID int64,
511511
if err != nil {
512512
return nil, err
513513
}
514+
annotations, err := s.getAnnotationsForTargetKeys(ctx, repositoryID, annotationTargetsForSearchRows(rows))
515+
if err != nil {
516+
return nil, err
517+
}
514518
for _, row := range rows {
515519
result := TextSearchResult{
516520
TargetType: row.TargetType,
517521
TargetKey: row.TargetKey,
518522
Score: row.Score,
519523
}
520524
if row.TargetType == "group" {
521-
result, err = s.populateGroupSearchResult(ctx, repositoryID, row.TargetKey, result)
525+
result, err = s.populateGroupSearchResultWithAnnotations(ctx, row.TargetKey, result, annotations)
522526
} else {
523-
result, err = s.populateObjectSearchResultWithSummaries(ctx, repositoryID, row.TargetType, row.TargetKey, result, summaries)
527+
result, err = s.populateObjectSearchResultWithHydration(row.TargetType, row.TargetKey, result, summaries, annotations)
524528
}
525529
if err != nil {
526530
return nil, err
@@ -568,19 +572,14 @@ func (s *Service) populateObjectSearchResult(ctx context.Context, repositoryID i
568572
return result, nil
569573
}
570574

571-
func (s *Service) populateObjectSearchResultWithSummaries(ctx context.Context, repositoryID int64, targetType, targetKey string, result TextSearchResult, summaries map[string]GroupMemberObjectSummary) (TextSearchResult, error) {
572-
number, ok := objectNumberFromTargetKey(targetKey)
573-
if !ok {
575+
func (s *Service) populateObjectSearchResultWithHydration(targetType, targetKey string, result TextSearchResult, summaries map[string]GroupMemberObjectSummary, annotations map[string]map[string]any) (TextSearchResult, error) {
576+
if _, ok := objectNumberFromTargetKey(targetKey); !ok {
574577
return result, nil
575578
}
576579
if summary, ok := summaries[targetKey]; ok {
577580
result.ObjectSummary = &summary
578581
}
579-
annotations, err := s.getAnnotationsForTarget(ctx, targetType, repositoryID, number, nil)
580-
if err != nil {
581-
return TextSearchResult{}, err
582-
}
583-
result.Annotations = annotations
582+
result.Annotations = annotations[annotationMapKey(targetType, targetKey)]
584583
return result, nil
585584
}
586585

@@ -602,6 +601,20 @@ func (s *Service) populateGroupSearchResult(ctx context.Context, repositoryID in
602601
return result, nil
603602
}
604603

604+
func (s *Service) populateGroupSearchResultWithAnnotations(ctx context.Context, targetKey string, result TextSearchResult, annotations map[string]map[string]any) (TextSearchResult, error) {
605+
groupPublicID, ok := groupPublicIDFromTargetKey(targetKey)
606+
if !ok {
607+
return result, nil
608+
}
609+
group, err := s.lookupGroupByPublicID(ctx, groupPublicID)
610+
if err != nil {
611+
return TextSearchResult{}, translateDBError(err)
612+
}
613+
result.ID = group.PublicID
614+
result.Annotations = annotations[annotationMapKey("group", groupTargetKey(group.PublicID))]
615+
return result, nil
616+
}
617+
605618
func (i *Indexer) baseSearchParts(ctx context.Context, repositoryID int64, targetType, targetKey string) ([]string, time.Time, error) {
606619
if targetType == "group" {
607620
return i.groupSearchParts(ctx, targetKey)

‎internal/core/indexing_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,27 @@ func TestResolveSearchResultsBatchesMirrorSummaries(t *testing.T) {
9595
})
9696
defer server.Close()
9797

98+
actor := permissions.Actor{Type: "user", ID: "tester"}
99+
_, err := service.CreateFieldDefinition(ctx, actor, "acme", "widgets", FieldDefinitionInput{
100+
Name: "intent",
101+
ObjectScope: "pull_request",
102+
FieldType: "text",
103+
IsSearchable: true,
104+
}, "")
105+
require.NoError(t, err)
106+
_, err = service.CreateFieldDefinition(ctx, actor, "acme", "widgets", FieldDefinitionInput{
107+
Name: "theme",
108+
ObjectScope: "issue",
109+
FieldType: "text",
110+
IsSearchable: true,
111+
}, "")
112+
require.NoError(t, err)
113+
_, err = service.SetAnnotations(ctx, actor, "acme", "widgets", "pull_request", 22, nil, map[string]any{"intent": "retry auth safely"}, "")
114+
require.NoError(t, err)
115+
_, err = service.SetAnnotations(ctx, actor, "acme", "widgets", "issue", 11, nil, map[string]any{"theme": "auth flakes"}, "")
116+
require.NoError(t, err)
117+
batchCalls.Store(0)
118+
98119
results, err := service.resolveSearchResults(ctx, 101, []scoredSearchTarget{
99120
{TargetType: "pull_request", TargetKey: objectTargetKey(101, "pull_request", 22), Score: 0.75},
100121
{TargetType: "issue", TargetKey: objectTargetKey(101, "issue", 11), Score: 0.5},
@@ -104,6 +125,8 @@ func TestResolveSearchResultsBatchesMirrorSummaries(t *testing.T) {
104125
require.Equal(t, int32(1), batchCalls.Load())
105126
require.Equal(t, "Retry ACP turns safely", results[0].ObjectSummary.Title)
106127
require.Equal(t, "Auth retries are flaky", results[1].ObjectSummary.Title)
128+
require.Equal(t, "retry auth safely", results[0].Annotations["intent"])
129+
require.Equal(t, "auth flakes", results[1].Annotations["theme"])
107130
}
108131

109132
func TestBuildSearchResultLoadsGroupAnnotations(t *testing.T) {

0 commit comments

Comments
 (0)