dashboard/app: ignore, try not to create partial Commits

It looks like the partial Commits are coming from the build breakages mails.
If you have commit A newer than commit B, then there are two code
paths depending on which reports its build result first.
For slow development, B finishes before A is committed, so when
A notices a failure it checks to see if B was okay.
That code path seems to be okay.
For submit of back-to-back changes, typically A finishes before B,
so when B notices an okay it checks to see if A failed.
That code path seems to zero the Commit for A while
trying to set its FailNotificationSent to true.
It does (did) succeed in setting FailNotificationSent to true,
it just zeroed everything else.

This CL adds code to refuse to do the datastore.Put to
update FailNotificationSent if the Commit info is incomplete.

It also tries to ignore Num=0 records, but that may not be
as important anymore.

LGTM=cmang
R=cmang
CC=golang-codereviews
https://golang.org/cl/154080043
This commit is contained in:
Russ Cox 2014-10-07 15:37:31 -04:00
parent f463296c27
commit 46e9d8e060
2 changed files with 15 additions and 1 deletions

View File

@ -46,7 +46,13 @@ func commitHandler(r *http.Request) (interface{}, error) {
if r.Method == "GET" {
com.PackagePath = r.FormValue("packagePath")
com.Hash = r.FormValue("hash")
if err := datastore.Get(c, com.Key(c), com); err != nil {
err := datastore.Get(c, com.Key(c), com)
if com.Num == 0 && com.Desc == "" {
// Perf builder might have written an incomplete Commit.
// Pretend it doesn't exist, so that we can get complete details.
err = datastore.ErrNoSuchEntity
}
if err != nil {
if err == datastore.ErrNoSuchEntity {
// This error string is special.
// The commit watcher expects it.

View File

@ -15,6 +15,7 @@ import (
"net/http"
"net/url"
"regexp"
"runtime"
"sort"
"text/template"
@ -216,6 +217,13 @@ func sendPerfFailMail(c appengine.Context, builder string, res *PerfResult) erro
}
func commonNotify(c appengine.Context, com *Commit, builder, logHash string) error {
if com.Num == 0 || com.Desc == "" {
stk := make([]byte, 10000)
n := runtime.Stack(stk, false)
stk = stk[:n]
c.Errorf("refusing to notify with com=%+v\n%s", *com, string(stk))
return fmt.Errorf("misuse of commonNotify")
}
if com.FailNotificationSent {
return nil
}