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.

6 files changed, 137 insertions, 37 deletions

diff --git a/dashboard/app/access_test.go b/dashboard/app/access_test.go
index 45b673759..c02ea1d63 100644
--- a/dashboard/app/access_test.go
+++ b/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,
diff --git a/dashboard/app/api.go b/dashboard/app/api.go
index 80f1919e1..5c84ad75e 100644
--- a/dashboard/app/api.go
+++ b/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)
+	var toDelete []*datastore.Key
+	latestOnManager := make(map[string]bool)
+	deleted, reproCount, noreproCount := 0, 0, 0
 	for _, crash := range crashes {
-		if latestOnManager[crash.Manager] == nil {
-			latestOnManager[crash.Manager] = crash
+		if !crash.Reported.IsZero() {
+			log.Errorf(c, "purging reported crash?")
+			continue
 		}
-	}
-	// 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
+		// Preserve latest crash on each manager.
+		if !latestOnManager[crash.Manager] {
+			latestOnManager[crash.Manager] = true
+			continue
 		}
-		return crashes[i].Time.Before(crashes[j].Time)
-	})
-	crashes = crashes[:len(crashes)-maxCrashes]
-	var toDelete []*datastore.Key
-	for _, crash := range crashes {
-		if crash.ReproSyz != 0 || crash.ReproC != 0 || !crash.Reported.IsZero() {
-			log.Errorf(c, "purging reproducer?")
+		// 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) {
diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index af4bacf48..57202e7c4 100644
--- a/dashboard/app/app_test.go
+++ b/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)
+		}
+	}
+}
diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go
index 9154bc68b..152a12506 100644
--- a/dashboard/app/entities.go
+++ b/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)))
 }
diff --git a/dashboard/app/main.go b/dashboard/app/main.go
index aee82ed8a..044b4f767 100644
--- a/dashboard/app/main.go
+++ b/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
 	}
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 4982c8ade..4ae3ae29f 100644
--- a/dashboard/app/reporting.go
+++ b/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