Skip to content

Commit 3de55ee

Browse files
julieqiujulieqiu
committed
internal: delete ReadSymbolHistory experiment
The ReadSymbolHistory is deleted, since the symbol_history table has been fully populated. When either ExperimentSymbolHistoryMainPage or ExperimentSymbolHistoryVersionsPage is active, data is now always read from symbol_history instead of a JOIN on package_symbols and documentation_symbols. For golang/go#37102 Change-Id: I8ca0aece399217b804a9a6c55bbfec5efe5b3a18 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/338450 Trust: Julie Qiu <[email protected]> Reviewed-by: Jamal Carvalho <[email protected]>
1 parent 8309cc4 commit 3de55ee

File tree

8 files changed

+135
-230
lines changed

8 files changed

+135
-230
lines changed

‎internal/experiment.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const (
1010
ExperimentEnableStdFrontendFetch = "enable-std-frontend-fetch"
1111
ExperimentInsertSymbolSearchDocuments = "insert-symbol-search-documents"
1212
ExperimentNewUnitLayout = "new-unit-layout"
13-
ExperimentReadSymbolHistory = "read-symbol-history"
1413
ExperimentSearchGrouping = "search-grouping"
1514
ExperimentSearchIncrementally = "search-incrementally"
1615
ExperimentSkipInsertSymbols = "skip-insert-symbols"
@@ -27,7 +26,6 @@ var Experiments = map[string]string{
2726
ExperimentEnableStdFrontendFetch: "Enable frontend fetching for module std.",
2827
ExperimentInsertSymbolSearchDocuments: "Insert data into symbol_search_documents and documentation_symbols.",
2928
ExperimentNewUnitLayout: "Enable the new layout on the unit page.",
30-
ExperimentReadSymbolHistory: "Read data from the symbol_history table.",
3129
ExperimentSearchGrouping: "Group search results.",
3230
ExperimentSearchIncrementally: "Use incremental query for search results.",
3331
ExperimentSkipInsertSymbols: "Don't insert data into symbols tables.",

‎internal/frontend/server_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1205,7 +1205,6 @@ func TestServer(t *testing.T) {
12051205
return append(serverTestCases(), linksTestCases...)
12061206
},
12071207
experiments: []string{
1208-
internal.ExperimentReadSymbolHistory,
12091208
internal.ExperimentSymbolHistoryMainPage,
12101209
internal.ExperimentSymbolHistoryVersionsPage,
12111210
},

‎internal/postgres/symbol_history.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"golang.org/x/pkgsite/internal"
1414
"golang.org/x/pkgsite/internal/database"
1515
"golang.org/x/pkgsite/internal/derrors"
16-
"golang.org/x/pkgsite/internal/experiment"
1716
"golang.org/x/pkgsite/internal/middleware"
1817
"golang.org/x/pkgsite/internal/stdlib"
1918
"golang.org/x/pkgsite/internal/symbol"
@@ -29,10 +28,7 @@ func (db *DB) GetSymbolHistory(ctx context.Context, packagePath, modulePath stri
2928
if modulePath == stdlib.ModulePath {
3029
return GetSymbolHistoryFromTable(ctx, db.db, packagePath, modulePath)
3130
}
32-
if experiment.IsActive(ctx, internal.ExperimentReadSymbolHistory) {
33-
return GetSymbolHistoryFromTable(ctx, db.db, packagePath, modulePath)
34-
}
35-
return GetSymbolHistoryWithPackageSymbols(ctx, db.db, packagePath, modulePath)
31+
return GetSymbolHistoryFromTable(ctx, db.db, packagePath, modulePath)
3632
}
3733

3834
// GetSymbolHistoryFromTable returns a SymbolHistory, which is a representation of the

‎internal/postgres/symbol_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ func TestInsertSymbolNamesAndHistory(t *testing.T) {
2626
defer release()
2727
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
2828
ctx = experiment.NewContext(ctx,
29-
internal.ExperimentReadSymbolHistory,
3029
internal.ExperimentInsertSymbolSearchDocuments,
3130
)
3231
defer cancel()
@@ -93,7 +92,6 @@ func TestInsertSymbolHistory_Basic(t *testing.T) {
9392
defer release()
9493
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
9594
ctx = experiment.NewContext(ctx,
96-
internal.ExperimentReadSymbolHistory,
9795
internal.ExperimentInsertSymbolSearchDocuments,
9896
)
9997
defer cancel()
@@ -125,7 +123,6 @@ func TestInsertSymbolHistory_MultiVersions(t *testing.T) {
125123
defer release()
126124
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
127125
ctx = experiment.NewContext(ctx,
128-
internal.ExperimentReadSymbolHistory,
129126
internal.ExperimentInsertSymbolSearchDocuments,
130127
)
131128
defer cancel()
@@ -229,7 +226,6 @@ func TestInsertSymbolHistory_MultiGOOS(t *testing.T) {
229226
defer release()
230227
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
231228
ctx = experiment.NewContext(ctx,
232-
internal.ExperimentReadSymbolHistory,
233229
internal.ExperimentInsertSymbolSearchDocuments,
234230
)
235231
defer cancel()

‎internal/postgres/unit.go

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -545,30 +545,12 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b
545545
return nil, err
546546
}
547547
}
548-
549-
if !experiment.IsActive(ctx, internal.ExperimentSymbolHistoryMainPage) {
550-
return &u, nil
551-
}
552-
if experiment.IsActive(ctx, internal.ExperimentReadSymbolHistory) {
548+
if experiment.IsActive(ctx, internal.ExperimentSymbolHistoryMainPage) {
553549
u.SymbolHistory, err = GetSymbolHistoryForBuildContext(ctx, db.db, pathID, um.ModulePath, bcMatched)
554550
if err != nil {
555551
return nil, err
556552
}
557-
return &u, nil
558-
}
559-
sh, err := GetSymbolHistoryWithPackageSymbols(ctx, db.db, um.Path,
560-
um.ModulePath)
561-
if err != nil {
562-
return nil, err
563-
}
564-
nameToVersion := map[string]string{}
565-
for _, v := range sh.Versions() {
566-
nts := sh.SymbolsAtVersion(v)
567-
for n := range nts {
568-
nameToVersion[n] = v
569-
}
570553
}
571-
u.SymbolHistory = nameToVersion
572554
}
573555
return &u, nil
574556
}

‎internal/testing/integration/frontend_main_test.go

Lines changed: 70 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -25,104 +25,82 @@ var (
2525

2626
func TestFrontendMainPage(t *testing.T) {
2727
defer postgres.ResetTestDB(testDB, t)
28-
defer postgres.ResetTestDB(testDB, t)
29-
for _, exp := range []struct {
30-
name string
31-
exps []string
28+
29+
processVersions(
30+
experiment.NewContext(context.Background(), internal.ExperimentSymbolHistoryMainPage),
31+
t, testModules)
32+
33+
const modulePath = "example.com/symbols"
34+
for _, test := range []struct {
35+
name, pkgPath string
36+
want htmlcheck.Checker
3237
}{
3338
{
34-
"no experiment",
35-
[]string{
36-
internal.ExperimentSymbolHistoryMainPage,
37-
},
39+
"main page symbols - one version all symbols",
40+
modulePath,
41+
in("",
42+
in("#F",
43+
in(".Documentation-sinceVersion", hasText(""))),
44+
in("#I1",
45+
in(".Documentation-sinceVersion", hasText(""))),
46+
in("#I2",
47+
in(".Documentation-sinceVersion", hasText(""))),
48+
in("#Int",
49+
in(".Documentation-sinceVersion", hasText(""))),
50+
in("#Num",
51+
in(".Documentation-sinceVersion", hasText(""))),
52+
in("#S1",
53+
in(".Documentation-sinceVersion", hasText(""))),
54+
in("#S2",
55+
in(".Documentation-sinceVersion", hasText(""))),
56+
in("#String",
57+
in(".Documentation-sinceVersion > .Documentation-sinceVersionVersion", hasText("v1.1.0"))),
58+
in("#T",
59+
in(".Documentation-sinceVersion", hasText(""))),
60+
in("#TF",
61+
in(".Documentation-sinceVersion", hasText(""))),
62+
),
3863
},
3964
{
40-
"experiment insert and read symbol_history",
41-
[]string{
42-
internal.ExperimentReadSymbolHistory,
43-
internal.ExperimentSymbolHistoryMainPage,
44-
},
65+
"main page hello - multi GOOS default page",
66+
modulePath + "/hello",
67+
// Hello is the only symbol when GOOS is not set, so return the
68+
// empty string instead of v1.2.0.
69+
// TODO(https://golang.org/issue/37102): decide whether it makes
70+
// sense to show the version in this case.
71+
in("", in("#Hello",
72+
in(".Documentation-sinceVersion", hasText("")))),
4573
},
74+
/*
75+
TODO: fix flaky test and uncomment
76+
{
77+
"main page hello - multi GOOS JS page",
78+
modulePath + "/hello?GOOS=js",
79+
in("",
80+
// HelloJS was introduced in v1.1.0, the earliest version of
81+
// this package. Omit the version, even though it is not the
82+
// earliest version of the module.
83+
in("#HelloJS",
84+
in(".Documentation-sinceVersion", hasText(""))),
85+
// Hello is not the only symbol when GOOS=js, so show that
86+
// it was added in v1.2.0.
87+
in("#Hello",
88+
in(".Documentation-sinceVersion", hasText("v1.2.0")))),
89+
},
90+
*/
4691
} {
47-
t.Run(exp.name, func(t *testing.T) {
48-
exps := append(exp.exps, internal.ExperimentInsertSymbolSearchDocuments)
49-
processVersions(
50-
experiment.NewContext(context.Background(), exps...),
51-
t, testModules)
52-
53-
const modulePath = "example.com/symbols"
54-
for _, test := range []struct {
55-
name, pkgPath string
56-
want htmlcheck.Checker
57-
}{
58-
{
59-
"main page symbols - one version all symbols",
60-
modulePath,
61-
in("",
62-
in("#F",
63-
in(".Documentation-sinceVersion", hasText(""))),
64-
in("#I1",
65-
in(".Documentation-sinceVersion", hasText(""))),
66-
in("#I2",
67-
in(".Documentation-sinceVersion", hasText(""))),
68-
in("#Int",
69-
in(".Documentation-sinceVersion", hasText(""))),
70-
in("#Num",
71-
in(".Documentation-sinceVersion", hasText(""))),
72-
in("#S1",
73-
in(".Documentation-sinceVersion", hasText(""))),
74-
in("#S2",
75-
in(".Documentation-sinceVersion", hasText(""))),
76-
in("#String",
77-
in(".Documentation-sinceVersion > .Documentation-sinceVersionVersion", hasText("v1.1.0"))),
78-
in("#T",
79-
in(".Documentation-sinceVersion", hasText(""))),
80-
in("#TF",
81-
in(".Documentation-sinceVersion", hasText(""))),
82-
),
83-
},
84-
{
85-
"main page hello - multi GOOS default page",
86-
modulePath + "/hello",
87-
// Hello is the only symbol when GOOS is not set, so return the
88-
// empty string instead of v1.2.0.
89-
// TODO(https://golang.org/issue/37102): decide whether it makes
90-
// sense to show the version in this case.
91-
in("", in("#Hello",
92-
in(".Documentation-sinceVersion", hasText("")))),
93-
},
94-
/*
95-
TODO: fix flaky test and uncomment
96-
{
97-
"main page hello - multi GOOS JS page",
98-
modulePath + "/hello?GOOS=js",
99-
in("",
100-
// HelloJS was introduced in v1.1.0, the earliest version of
101-
// this package. Omit the version, even though it is not the
102-
// earliest version of the module.
103-
in("#HelloJS",
104-
in(".Documentation-sinceVersion", hasText(""))),
105-
// Hello is not the only symbol when GOOS=js, so show that
106-
// it was added in v1.2.0.
107-
in("#Hello",
108-
in(".Documentation-sinceVersion", hasText("v1.2.0")))),
109-
},
110-
*/
111-
} {
112-
t.Run(test.name, func(t *testing.T) {
113-
urlPath := fmt.Sprintf("/%s", test.pkgPath)
114-
body := getFrontendPage(t, urlPath, exps...)
115-
doc, err := html.Parse(strings.NewReader(body))
116-
if err != nil {
117-
t.Fatal(err)
118-
}
119-
if err := test.want(doc); err != nil {
120-
if testing.Verbose() {
121-
html.Render(os.Stdout, doc)
122-
}
123-
t.Error(err)
124-
}
125-
})
92+
t.Run(test.name, func(t *testing.T) {
93+
urlPath := fmt.Sprintf("/%s", test.pkgPath)
94+
body := getFrontendPage(t, urlPath, internal.ExperimentSymbolHistoryMainPage)
95+
doc, err := html.Parse(strings.NewReader(body))
96+
if err != nil {
97+
t.Fatal(err)
98+
}
99+
if err := test.want(doc); err != nil {
100+
if testing.Verbose() {
101+
html.Render(os.Stdout, doc)
102+
}
103+
t.Error(err)
126104
}
127105
})
128106
}

0 commit comments

Comments
 (0)