dashboard/app: purge old repros

We have some bugs with insane amount of repros.
So many that new crashes don't show up on dashboard at all.
Purge old repros too. There is no need to keep more than 40.

dashboard/app/access_test.go

@@ -80,7 +80,7 @@ func TestAccess(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		bugID := bugKeyHash(bug.Namespace, bug.Title, bug.Seq)
+		bugID := bug.keyHash()
 		entities = append(entities, []entity{
 			{
 				level: level,

dashboard/app/api.go

@@ -399,7 +399,7 @@ func addCommitsToBug(c context.Context, bug *Bug, manager string, managers []str
 		return nil
 	}
 	now := timeNow(c)
-	bugKey := datastore.NewKey(c, "Bug", bugKeyHash(bug.Namespace, bug.Title, bug.Seq), 0, nil)
+	bugKey := bug.key(c)
 	tx := func(c context.Context) error {
 		bug := new(Bug)
 		if err := datastore.Get(c, bugKey, bug); err != nil {
@@ -508,7 +508,7 @@ func apiReportBuildError(c context.Context, ns string, r *http.Request, payload
 		return nil, err
 	}
 	if err := updateManager(c, ns, req.Build.Manager, func(mgr *Manager, stats *ManagerStats) {
-		mgr.FailedBuildBug = bugKeyHash(bug.Namespace, bug.Title, bug.Seq)
+		mgr.FailedBuildBug = bug.keyHash()
 	}); err != nil {
 		return nil, err
 	}
@@ -659,23 +659,19 @@ func saveCrash(c context.Context, ns string, req *dashapi.Crash, bugKey *datasto
 }
 
 func purgeOldCrashes(c context.Context, bug *Bug, bugKey *datastore.Key) {
-	if bug.NumCrashes <= maxCrashes || (bug.NumCrashes-1)%10 != 0 {
+	const purgeEvery = 10
+	if bug.NumCrashes <= 2*maxCrashes || (bug.NumCrashes-1)%purgeEvery != 0 {
 		return
 	}
 	var crashes []*Crash
 	keys, err := datastore.NewQuery("Crash").
 		Ancestor(bugKey).
-		Filter("ReproC=", 0).
-		Filter("ReproSyz=", 0).
 		Filter("Reported=", time.Time{}).
 		GetAll(c, &crashes)
 	if err != nil {
 		log.Errorf(c, "failed to fetch purge crashes: %v", err)
 		return
 	}
-	if len(keys) <= maxCrashes {
-		return
-	}
 	keyMap := make(map[*Crash]*datastore.Key)
 	for i, crash := range crashes {
 		keyMap[crash] = keys[i]
@@ -684,27 +680,26 @@ func purgeOldCrashes(c context.Context, bug *Bug, bugKey *datastore.Key) {
 	sort.Slice(crashes, func(i, j int) bool {
 		return crashes[i].Time.After(crashes[j].Time)
 	})
-	// Find latest crash on each manager.
-	latestOnManager := make(map[string]*Crash)
-	for _, crash := range crashes {
-		if latestOnManager[crash.Manager] == nil {
-			latestOnManager[crash.Manager] = crash
-		}
-	}
-	// Oldest first but move latest crash on each manager to the end (preserve them).
-	sort.Slice(crashes, func(i, j int) bool {
-		latesti := latestOnManager[crashes[i].Manager] == crashes[i]
-		latestj := latestOnManager[crashes[j].Manager] == crashes[j]
-		if latesti != latestj {
-			return latestj
-		}
-		return crashes[i].Time.Before(crashes[j].Time)
-	})
-	crashes = crashes[:len(crashes)-maxCrashes]
 	var toDelete []*datastore.Key
+	latestOnManager := make(map[string]bool)
+	deleted, reproCount, noreproCount := 0, 0, 0
 	for _, crash := range crashes {
-		if crash.ReproSyz != 0 || crash.ReproC != 0 || !crash.Reported.IsZero() {
-			log.Errorf(c, "purging reproducer?")
+		if !crash.Reported.IsZero() {
+			log.Errorf(c, "purging reported crash?")
+			continue
+		}
+		// Preserve latest crash on each manager.
+		if !latestOnManager[crash.Manager] {
+			latestOnManager[crash.Manager] = true
+			continue
+		}
+		// Preserve maxCrashes latest crashes with repro and without repro.
+		count := &noreproCount
+		if crash.ReproSyz != 0 || crash.ReproC != 0 {
+			count = &reproCount
+		}
+		if *count < maxCrashes {
+			*count++
 			continue
 		}
 		toDelete = append(toDelete, keyMap[crash])
@@ -714,12 +709,25 @@ func purgeOldCrashes(c context.Context, bug *Bug, bugKey *datastore.Key) {
 		if crash.Report != 0 {
 			toDelete = append(toDelete, datastore.NewKey(c, textCrashReport, "", crash.Report, nil))
 		}
+		if crash.ReproSyz != 0 {
+			toDelete = append(toDelete, datastore.NewKey(c, textReproSyz, "", crash.ReproSyz, nil))
+		}
+		if crash.ReproC != 0 {
+			toDelete = append(toDelete, datastore.NewKey(c, textReproC, "", crash.ReproC, nil))
+		}
+		deleted++
+		if deleted == 2*purgeEvery {
+			break
+		}
+	}
+	if len(toDelete) == 0 {
+		return
 	}
 	if err := datastore.DeleteMulti(c, toDelete); err != nil {
 		log.Errorf(c, "failed to delete old crashes: %v", err)
 		return
 	}
-	log.Infof(c, "deleted %v crashes for bug %q", len(crashes), bug.Title)
+	log.Infof(c, "deleted %v crashes for bug %q", deleted, bug.Title)
 }
 
 func apiReportFailedRepro(c context.Context, ns string, r *http.Request, payload []byte) (interface{}, error) {

dashboard/app/app_test.go

@@ -7,6 +7,7 @@ package dash
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -285,3 +286,87 @@ func TestApp(t *testing.T) {
 		ReproLevel: dashapi.ReproLevelC,
 	})
 }
+
+// Test purging of old crashes for bugs with lots of crashes.
+func TestPurgeOldCrashes(t *testing.T) {
+	if testing.Short() {
+		t.Skip()
+	}
+	c := NewCtx(t)
+	defer c.Close()
+
+	build := testBuild(1)
+	c.client.UploadBuild(build)
+
+	// First, send 3 crashes that are reported. These need to be preserved regardless.
+	crash := testCrash(build, 1)
+	crash.ReproOpts = []byte("no repro")
+	c.client.ReportCrash(crash)
+	rep := c.client.pollBug()
+
+	crash.ReproSyz = []byte("getpid()")
+	crash.ReproOpts = []byte("syz repro")
+	c.client.ReportCrash(crash)
+	c.client.pollBug()
+
+	crash.ReproC = []byte("int main() {}")
+	crash.ReproOpts = []byte("C repro")
+	c.client.ReportCrash(crash)
+	c.client.pollBug()
+
+	// Now report lots of bugs with/without repros. Some of the older ones should be purged.
+	const totalReported = 3 * maxCrashes
+	for i := 0; i < totalReported; i++ {
+		c.advanceTime(2 * time.Hour) // This ensures that crashes are saved.
+		crash.ReproSyz = nil
+		crash.ReproC = nil
+		crash.ReproOpts = []byte(fmt.Sprintf("%v", i))
+		c.client.ReportCrash(crash)
+
+		crash.ReproSyz = []byte("syz repro")
+		crash.ReproC = []byte("C repro")
+		crash.ReproOpts = []byte(fmt.Sprintf("%v", i))
+		c.client.ReportCrash(crash)
+	}
+	bug, _, _ := c.loadBug(rep.ID)
+	crashes, _, err := queryCrashesForBug(c.ctx, bug.key(c.ctx), 10*totalReported)
+	if err != nil {
+		c.t.Fatal(err)
+	}
+	// First, count how many crashes of different types we have.
+	// We should get all 3 reported crashes + some with repros and some without repros.
+	reported, norepro, repro := 0, 0, 0
+	for _, crash := range crashes {
+		if !crash.Reported.IsZero() {
+			reported++
+		} else if crash.ReproSyz == 0 {
+			norepro++
+		} else {
+			repro++
+		}
+	}
+	c.t.Logf("got reported=%v, norepro=%v, repro=%v, maxCrashes=%v",
+		reported, norepro, repro, maxCrashes)
+	if reported != 3 ||
+		norepro < maxCrashes || norepro > maxCrashes+10 ||
+		repro < maxCrashes || repro > maxCrashes+10 {
+		c.t.Fatalf("bad purged crashes")
+	}
+	// Then, check that latest crashes were preserved.
+	for _, crash := range crashes {
+		if !crash.Reported.IsZero() {
+			continue
+		}
+		idx, err := strconv.Atoi(string(crash.ReproOpts))
+		if err != nil {
+			c.t.Fatal(err)
+		}
+		count := norepro
+		if crash.ReproSyz != 0 {
+			count = repro
+		}
+		if idx < totalReported-count {
+			c.t.Errorf("preserved bad crash repro=%v: %v", crash.ReproC != 0, idx)
+		}
+	}
+}

dashboard/app/entities.go

@@ -336,12 +336,20 @@ func canonicalBug(c context.Context, bug *Bug) (*Bug, error) {
 		bugKey := datastore.NewKey(c, "Bug", bug.DupOf, 0, nil)
 		if err := datastore.Get(c, bugKey, canon); err != nil {
 			return nil, fmt.Errorf("failed to get dup bug %q for %q: %v",
-				bug.DupOf, bugKeyHash(bug.Namespace, bug.Title, bug.Seq), err)
+				bug.DupOf, bug.keyHash(), err)
 		}
 		bug = canon
 	}
 }
 
+func (bug *Bug) key(c context.Context) *datastore.Key {
+	return datastore.NewKey(c, "Bug", bug.keyHash(), 0, nil)
+}
+
+func (bug *Bug) keyHash() string {
+	return bugKeyHash(bug.Namespace, bug.Title, bug.Seq)
+}
+
 func bugKeyHash(ns, title string, seq int64) string {
 	return hash.String([]byte(fmt.Sprintf("%v-%v-%v-%v", config.Namespaces[ns].Key, ns, title, seq)))
 }

dashboard/app/main.go

@@ -418,7 +418,7 @@ func fetchNamespaceBugs(c context.Context, accessLevel AccessLevel, ns string,
 			continue
 		}
 		uiBug := createUIBug(c, bug, state, managers)
-		bugMap[bugKeyHash(bug.Namespace, bug.Title, bug.Seq)] = uiBug
+		bugMap[bug.keyHash()] = uiBug
 		id := uiBug.ReportingIndex
 		if bug.Status == BugStatusFixed {
 			id = -1
@@ -491,7 +491,7 @@ func fetchNamespaceBugs(c context.Context, accessLevel AccessLevel, ns string,
 
 func loadDupsForBug(c context.Context, r *http.Request, bug *Bug, state *ReportingState, managers []string) (
 	*uiBugGroup, error) {
-	bugHash := bugKeyHash(bug.Namespace, bug.Title, bug.Seq)
+	bugHash := bug.keyHash()
 	var dups []*Bug
 	_, err := datastore.NewQuery("Bug").
 		Filter("Status=", BugStatusDup).
@@ -603,7 +603,7 @@ func createUIBug(c context.Context, bug *Bug, state *ReportingState, managers []
 	if err != nil {
 		log.Errorf(c, "failed to generate credit email: %v", err)
 	}
-	id := bugKeyHash(bug.Namespace, bug.Title, bug.Seq)
+	id := bug.keyHash()
 	uiBug := &uiBug{
 		Namespace:      bug.Namespace,
 		Title:          bug.displayTitle(),
@@ -662,10 +662,9 @@ func updateBugBadness(c context.Context, bug *uiBug) {
 }
 
 func loadCrashesForBug(c context.Context, bug *Bug) ([]*uiCrash, []byte, error) {
-	bugHash := bugKeyHash(bug.Namespace, bug.Title, bug.Seq)
-	bugKey := datastore.NewKey(c, "Bug", bugHash, 0, nil)
+	bugKey := bug.key(c)
 	// We can have more than maxCrashes crashes, if we have lots of reproducers.
-	crashes, _, err := queryCrashesForBug(c, bugKey, maxCrashes+200)
+	crashes, _, err := queryCrashesForBug(c, bugKey, 2*maxCrashes+200)
 	if err != nil || len(crashes) == 0 {
 		return nil, nil, err
 	}

dashboard/app/reporting.go

@@ -447,7 +447,7 @@ func findDupBug(c context.Context, cmd *dashapi.BugUpdate, bug *Bug, bugKey *dat
 	if !dupReporting.Closed.IsZero() && dupCanon.Status == BugStatusOpen {
 		return "", false, "Dup bug is already upstreamed.", nil
 	}
-	dupHash := bugKeyHash(dup.Namespace, dup.Title, dup.Seq)
+	dupHash := dup.keyHash()
 	return dupHash, true, "", nil
 }
 
@@ -723,7 +723,7 @@ func queryCrashesForBug(c context.Context, bugKey *datastore.Key, limit int) (
 }
 
 func findCrashForBug(c context.Context, bug *Bug) (*Crash, *datastore.Key, error) {
-	bugKey := datastore.NewKey(c, "Bug", bugKeyHash(bug.Namespace, bug.Title, bug.Seq), 0, nil)
+	bugKey := bug.key(c)
 	crashes, keys, err := queryCrashesForBug(c, bugKey, 1)
 	if err != nil {
 		return nil, nil, err