app/appengine: do maintner & coordinator RPCs concurrently
Fixes a TODO.
The coordinator RPC should be temporary, but a) who ever knows how
long temporary is, and b) it's easy enough to do, and c) faster is
nicer.
Updates golang/go#34744
Change-Id: Id7216e1c67dc04152565bb8c5d6181cde674a12e
Reviewed-on: https://go-review.googlesource.com/c/build/+/210997
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/app/appengine/ui.go b/app/appengine/ui.go
index 6316d40..e731736 100644
--- a/app/appengine/ui.go
+++ b/app/appengine/ui.go
@@ -25,6 +25,7 @@
"golang.org/x/build/maintner/maintnerd/apipb"
"golang.org/x/build/repos"
"golang.org/x/build/types"
+ "golang.org/x/sync/errgroup"
"grpc.go4.org"
"grpc.go4.org/codes"
)
@@ -43,18 +44,34 @@
return
}
- // TODO: if we end up fetching the building state from the
- // coordinator too, do that concurrently in an
- // x/sync/errgroup.Group here. But for now we're only doing
- // one RPC call.
ctx := r.Context()
- dashRes, err := maintnerClient.GetDashboard(ctx, dashReq)
- if err != nil {
+ var (
+ dashRes *apipb.DashboardResponse
+ activeBuilds []types.ActivePostSubmitBuild
+ )
+ var rpcs errgroup.Group
+ rpcs.Go(func() error {
+ var err error
+ dashRes, err = maintnerClient.GetDashboard(ctx, dashReq)
+ return err
+ })
+ if view.ShowsActiveBuilds() {
+ rpcs.Go(func() error {
+ activeBuilds = getActiveBuilds(ctx)
+ return nil
+ })
+ }
+ if err := rpcs.Wait(); err != nil {
http.Error(w, "maintner.GetDashboard: "+err.Error(), httpStatusOfErr(err))
return
}
- tb := newUITemplateDataBuilder(view, dashReq, dashRes)
+ tb := &uiTemplateDataBuilder{
+ view: view,
+ req: dashReq,
+ res: dashRes,
+ activeBuilds: activeBuilds,
+ }
data, err := tb.buildTemplateData(ctx)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -67,6 +84,9 @@
// See viewForRequest.
type dashboardView interface {
ServeDashboard(w http.ResponseWriter, r *http.Request, data *uiTemplateData)
+ // ShowsActiveBuilds reports whether this view uses
+ // information about the currently active builds.
+ ShowsActiveBuilds() bool
}
// viewForRequest selects the dashboardView based on the HTTP
@@ -96,9 +116,10 @@
// dashboardViews. That is, it maps the maintner protobuf response to
// the data structure needed by the dashboardView/template.
type uiTemplateDataBuilder struct {
- view dashboardView
- req *apipb.DashboardRequest
- res *apipb.DashboardResponse
+ view dashboardView
+ req *apipb.DashboardRequest
+ res *apipb.DashboardResponse
+ activeBuilds []types.ActivePostSubmitBuild // optional; for blue gopher links
// testCommitData, if non-nil, provides an alternate data
// source to use for testing instead of making real datastore
@@ -106,17 +127,6 @@
testCommitData map[string]*Commit
}
-// newUITemplateDataBuilder returns a new uiTemplateDataBuilder for a
-// given view, dashboard request, and dashboard response from
-// maintner.
-func newUITemplateDataBuilder(view dashboardView, req *apipb.DashboardRequest, res *apipb.DashboardResponse) *uiTemplateDataBuilder {
- return &uiTemplateDataBuilder{
- view: view,
- req: req,
- res: res,
- }
-}
-
// getCommitsToLoad returns a set (all values are true) of which commits to load from
// the datastore.
func (tb *uiTemplateDataBuilder) getCommitsToLoad() map[commitInPackage]bool {
@@ -324,9 +334,10 @@
if tb.req.Repo == "" && tb.req.Branch == "master" {
data.Builders = onlyGoMasterBuilders(data.Builders)
}
-
+ }
+ if tb.view.ShowsActiveBuilds() {
// Populate building URLs for the HTML UI only.
- data.populateBuildingURLs(ctx)
+ data.populateBuildingURLs(ctx, tb.activeBuilds)
}
return data, nil
@@ -335,6 +346,7 @@
// htmlView renders the HTML (default) form of https://build.golang.org/ with no mode parameter.
type htmlView struct{}
+func (htmlView) ShowsActiveBuilds() bool { return true }
func (htmlView) ServeDashboard(w http.ResponseWriter, r *http.Request, data *uiTemplateData) {
var buf bytes.Buffer
if err := uiTemplate.Execute(&buf, data); err != nil {
@@ -398,6 +410,7 @@
// hash builder failure-url
type failuresView struct{}
+func (failuresView) ShowsActiveBuilds() bool { return false }
func (failuresView) ServeDashboard(w http.ResponseWriter, r *http.Request, data *uiTemplateData) {
w.Header().Set("Content-Type", "text/plain")
for _, c := range data.Commits {
@@ -416,6 +429,7 @@
// The output is a types.BuildStatus JSON object.
type jsonView struct{}
+func (jsonView) ShowsActiveBuilds() bool { return false }
func (jsonView) ServeDashboard(w http.ResponseWriter, r *http.Request, data *uiTemplateData) {
// cell returns one of "" (no data), "ok", or a failure URL.
cell := func(res *Result) string {
@@ -708,44 +722,38 @@
Branch string
}
-var altActiveBuildingForTest func() []types.ActivePostSubmitBuild
-
-// getActiveBuilding returns the builds that coordinator is currently doing.
+// getActiveBuilds returns the builds that coordinator is currently doing.
// This isn't critical functionality so errors are logged but otherwise ignored for now.
// Once this is merged into the coordinator we won't need to make an RPC to get
// this info. See https://github.com/golang/go/issues/34744#issuecomment-563398753.
-func getActiveBuilding(ctx context.Context) (builds []types.ActivePostSubmitBuild) {
- if f := altActiveBuildingForTest; f != nil {
- return f()
- }
+func getActiveBuilds(ctx context.Context) (builds []types.ActivePostSubmitBuild) {
ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
defer cancel()
req, _ := http.NewRequest("GET", "https://farmer.golang.org/status/post-submit-active.json", nil)
req = req.WithContext(ctx)
res, err := http.DefaultClient.Do(req)
if err != nil {
- log.Printf("getActiveBuilding: Do: %v", err)
+ log.Printf("getActiveBuilds: Do: %v", err)
return
}
defer res.Body.Close()
if res.StatusCode != 200 {
- log.Printf("getActiveBuilding: %v", res.Status)
+ log.Printf("getActiveBuilds: %v", res.Status)
return
}
if err := json.NewDecoder(res.Body).Decode(&builds); err != nil {
- log.Printf("getActiveBuilding: JSON decode: %v", err)
+ log.Printf("getActiveBuilds: JSON decode: %v", err)
}
return builds
}
// populateBuildingURLs populates each commit in Commits' buildingURLs map with the
// URLs of builds which are currently in progress.
-func (td *uiTemplateData) populateBuildingURLs(ctx context.Context) {
- activeBuilding := getActiveBuilding(ctx)
+func (td *uiTemplateData) populateBuildingURLs(ctx context.Context, activeBuilds []types.ActivePostSubmitBuild) {
// active maps from a build record with its status URL zeroed
// out to to the actual value of that status URL.
active := map[types.ActivePostSubmitBuild]string{}
- for _, rec := range activeBuilding {
+ for _, rec := range activeBuilds {
statusURL := rec.StatusURL
rec.StatusURL = ""
active[rec] = statusURL
diff --git a/app/appengine/ui_test.go b/app/appengine/ui_test.go
index f5658e1..5ff7072 100644
--- a/app/appengine/ui_test.go
+++ b/app/appengine/ui_test.go
@@ -20,10 +20,7 @@
// Thin the list of builders to make this test's data lighter
// and require less maintenance keeping it in sync.
origBuilders := dashboard.Builders
- defer func() {
- dashboard.Builders = origBuilders
- altActiveBuildingForTest = nil
- }()
+ defer func() { dashboard.Builders = origBuilders }()
dashboard.Builders = map[string]*dashboard.BuildConfig{
"linux-amd64": origBuilders["linux-amd64"],
"linux-386": origBuilders["linux-386"],
@@ -315,12 +312,12 @@
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- altActiveBuildingForTest = func() (builds []types.ActivePostSubmitBuild) {
- return tt.activeBuilds
- }
- tb := newUITemplateDataBuilder(tt.view, tt.req, tt.res)
- if tt.testCommitData != nil {
- tb.testCommitData = tt.testCommitData
+ tb := &uiTemplateDataBuilder{
+ view: tt.view,
+ req: tt.req,
+ res: tt.res,
+ activeBuilds: tt.activeBuilds,
+ testCommitData: tt.testCommitData,
}
data, err := tb.buildTemplateData(context.Background())
if err != nil {
