dashboard/app: flexible rules for bug obsoleting
Implement logic described in #1054:
- close bugs that happened a lot and then stopped faster
- close bugs in non-final reporting with different period
- allow closing bugs that happened only on 1 manager with different period
Fixes #1054
diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index 8b4e45b..89ef7ff 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -32,6 +32,12 @@
EmailBlacklist: []string{
"\"Bar\" <BlackListed@Domain.com>",
},
+ Obsoleting: ObsoletingConfig{
+ MinPeriod: 80 * 24 * time.Hour,
+ MaxPeriod: 100 * 24 * time.Hour,
+ NonFinalMinPeriod: 40 * 24 * time.Hour,
+ NonFinalMaxPeriod: 60 * 24 * time.Hour,
+ },
DefaultNamespace: "test1",
Namespaces: map[string]*Config{
"test1": {
@@ -54,6 +60,12 @@
CC: []string{"maintainers@repo10.org", "bugs@repo10.org"},
},
},
+ Managers: map[string]ConfigManager{
+ "special-obsoleting": {
+ ObsoletingMinPeriod: 10 * 24 * time.Hour,
+ ObsoletingMaxPeriod: 20 * 24 * time.Hour,
+ },
+ },
Reporting: []Reporting{
{
Name: "reporting1",
diff --git a/dashboard/app/config.go b/dashboard/app/config.go
index 34314d6..46a2b95 100644
--- a/dashboard/app/config.go
+++ b/dashboard/app/config.go
@@ -33,6 +33,8 @@
Clients map[string]string
// List of emails blacklisted from issuing test requests.
EmailBlacklist []string
+ // Bug obsoleting settings. See ObsoletingConfig for details.
+ Obsoleting ObsoletingConfig
// Namespace that is shown by default (no namespace selected yet).
DefaultNamespace string
// Per-namespace config.
@@ -78,6 +80,22 @@
Repos []KernelRepo
}
+// ObsoletingConfig describes how bugs without reproducer should be obsoleted.
+// First, for each bug we conservatively estimate period since the last crash
+// when we consider it stopped happenning. This estimation is based on the first/last time
+// and number and rate of crashes. Then this period is capped by MinPeriod/MaxPeriod.
+// Then if the period has elapsed since the last crash, we obsolete the bug.
+// NonFinalMinPeriod/NonFinalMaxPeriod (if specified) are used to cap bugs in non-final reportings.
+// Additionally ConfigManager.ObsoletingMin/MaxPeriod override the cap settings
+// for bugs that happen only on that manager.
+// If no periods are specified, no bugs are obsoleted.
+type ObsoletingConfig struct {
+ MinPeriod time.Duration
+ MaxPeriod time.Duration
+ NonFinalMinPeriod time.Duration
+ NonFinalMaxPeriod time.Duration
+}
+
// ConfigManager describes a single syz-manager instance.
// Dashboard does not generally need to know about all of them,
// but in some special cases it needs to know some additional information.
@@ -90,6 +108,10 @@
// and RestrictedTestingReason contains a human readable reason for the restriction.
RestrictedTestingRepo string
RestrictedTestingReason string
+ // If a bug happens only on this manager, this overrides global obsoleting settings.
+ // See ObsoletingConfig for details.
+ ObsoletingMinPeriod time.Duration
+ ObsoletingMaxPeriod time.Duration
}
// One reporting stage.
@@ -200,6 +222,7 @@
clientNames := make(map[string]bool)
checkClients(clientNames, cfg.Clients)
checkConfigAccessLevel(&cfg.AccessLevel, AccessPublic, "global")
+ checkObsoleting(cfg.Obsoleting)
if cfg.Namespaces[cfg.DefaultNamespace] == nil {
panic(fmt.Sprintf("default namespace %q is not found", cfg.DefaultNamespace))
}
@@ -208,6 +231,30 @@
}
}
+func checkObsoleting(o ObsoletingConfig) {
+ if (o.MinPeriod == 0) != (o.MaxPeriod == 0) {
+ panic(fmt.Sprintf("obsoleting: both or none of Min/MaxPeriod must be specified"))
+ }
+ if o.MinPeriod > o.MaxPeriod {
+ panic(fmt.Sprintf("obsoleting: Min > MaxPeriod"))
+ }
+ if o.MinPeriod != 0 && o.MinPeriod < 24*time.Hour {
+ panic(fmt.Sprintf("obsoleting: too low MinPeriod"))
+ }
+ if (o.NonFinalMinPeriod == 0) != (o.NonFinalMaxPeriod == 0) {
+ panic(fmt.Sprintf("obsoleting: both or none of NonFinalMin/MaxPeriod must be specified"))
+ }
+ if o.NonFinalMinPeriod > o.NonFinalMaxPeriod {
+ panic(fmt.Sprintf("obsoleting: NonFinalMin > MaxPeriod"))
+ }
+ if o.NonFinalMinPeriod != 0 && o.NonFinalMinPeriod < 24*time.Hour {
+ panic(fmt.Sprintf("obsoleting: too low MinPeriod"))
+ }
+ if o.MinPeriod == 0 && o.NonFinalMinPeriod != 0 {
+ panic(fmt.Sprintf("obsoleting: NonFinalMinPeriod without MinPeriod"))
+ }
+}
+
func checkNamespace(ns string, cfg *Config, namespaces, clientNames map[string]bool) {
if !namespaceNameRe.MatchString(ns) {
panic(fmt.Sprintf("bad namespace name: %q", ns))
@@ -324,6 +371,15 @@
if mgr.RestrictedTestingRepo == "" && mgr.RestrictedTestingReason != "" {
panic(fmt.Sprintf("unrestricted manager %v/%v has restriction reason", ns, name))
}
+ if (mgr.ObsoletingMinPeriod == 0) != (mgr.ObsoletingMaxPeriod == 0) {
+ panic(fmt.Sprintf("manager %v/%v obsoleting: both or none of Min/MaxPeriod must be specified", ns, name))
+ }
+ if mgr.ObsoletingMinPeriod > mgr.ObsoletingMaxPeriod {
+ panic(fmt.Sprintf("manager %v/%v obsoleting: Min > MaxPeriod", ns, name))
+ }
+ if mgr.ObsoletingMinPeriod != 0 && mgr.ObsoletingMinPeriod < 24*time.Hour {
+ panic(fmt.Sprintf("manager %v/%v obsoleting: too low MinPeriod", ns, name))
+ }
}
func checkConfigAccessLevel(current *AccessLevel, parent AccessLevel, what string) {
diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go
index bbd5d7c..3437576 100644
--- a/dashboard/app/notifications_test.go
+++ b/dashboard/app/notifications_test.go
@@ -102,6 +102,81 @@
}
}
+func TestBugObsoleting(t *testing.T) {
+ // To simplify test we specify all dates in days from a fixed point in time.
+ const day = 24 * time.Hour
+ days := func(n int) time.Time {
+ t := time.Date(2000, 0, 0, 0, 0, 0, 0, time.UTC)
+ t.Add(time.Duration(n+1) * day)
+ return t
+ }
+ tests := []struct {
+ bug *Bug
+ period time.Duration
+ }{
+ // Final bug with just 1 crash: max final period.
+ {
+ bug: &Bug{
+ FirstTime: days(0),
+ LastTime: days(0),
+ NumCrashes: 1,
+ Reporting: []BugReporting{{Reported: days(0)}},
+ },
+ period: 100 * day,
+ },
+ // Non-final bug with just 1 crash: max non-final period.
+ {
+ bug: &Bug{
+ FirstTime: days(0),
+ LastTime: days(0),
+ NumCrashes: 1,
+ Reporting: []BugReporting{{Reported: days(0)}, {}},
+ },
+ period: 60 * day,
+ },
+ // Special manger: max period that that manager.
+ {
+ bug: &Bug{
+ FirstTime: days(0),
+ LastTime: days(0),
+ NumCrashes: 1,
+ HappenedOn: []string{"special-obsoleting"},
+ Reporting: []BugReporting{{Reported: days(0)}, {}},
+ },
+ period: 20 * day,
+ },
+ // Special manger and a non-special: normal rules.
+ {
+ bug: &Bug{
+ FirstTime: days(0),
+ LastTime: days(0),
+ NumCrashes: 1,
+ HappenedOn: []string{"special-obsoleting", "non-special-manager"},
+ Reporting: []BugReporting{{Reported: days(0)}},
+ },
+ period: 100 * day,
+ },
+ // Happened a lot: min period.
+ {
+ bug: &Bug{
+ FirstTime: days(0),
+ LastTime: days(1),
+ NumCrashes: 1000,
+ Reporting: []BugReporting{{Reported: days(0)}},
+ },
+ period: 80 * day,
+ },
+ }
+ for i, test := range tests {
+ test.bug.Namespace = "test1"
+ got := test.bug.obsoletePeriod()
+ if got != test.period {
+ t.Errorf("test #%v: got: %.2f, want %.2f",
+ i, float64(got/time.Hour)/24, float64(test.period/time.Hour)/24)
+ }
+ }
+}
+
func TestEmailNotifObsoleted(t *testing.T) {
c := NewCtx(t)
defer c.Close()
@@ -124,7 +199,7 @@
c.expectNoEmail()
// Not yet.
- c.advanceTime(119 * 24 * time.Hour)
+ c.advanceTime(59 * 24 * time.Hour)
c.expectNoEmail()
// Now!
@@ -145,7 +220,7 @@
c.incomingEmail(report.Sender, "#syz upstream")
report = c.pollEmailBug()
- c.advanceTime(121 * 24 * time.Hour)
+ c.advanceTime(101 * 24 * time.Hour)
notif = c.pollEmailBug()
if !strings.Contains(notif.Body, "Auto-closing this bug as obsolete") {
t.Fatalf("bad notification text: %q", notif.Body)
@@ -190,7 +265,7 @@
c.incomingEmail(report4.Sender, "#syz upstream")
report4 = c.pollEmailBug()
- c.advanceTime(119 * 24 * time.Hour)
+ c.advanceTime(59 * 24 * time.Hour)
c.expectNoEmail()
c.client2.ReportCrash(crash2)
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 99adb19..cb3f52c 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -31,7 +31,7 @@
maxInlineError = 16 << 10
notifyResendPeriod = 14 * 24 * time.Hour
notifyAboutBadCommitPeriod = 90 * 24 * time.Hour
- autoObsoletePeriod = 120 * 24 * time.Hour
+ never = 100 * 365 * 24 * time.Hour
internalError = "internal error"
// This is embedded as first line of syzkaller reproducer files.
syzReproPrefix = "# See https://goo.gl/kgGztJ for information about syzkaller reproducers.\n"
@@ -207,7 +207,7 @@
if len(bug.Commits) == 0 &&
bug.ReproLevel == ReproLevelNone &&
timeSince(c, bug.LastActivity) > notifyResendPeriod &&
- timeSince(c, bug.LastTime) > autoObsoletePeriod {
+ timeSince(c, bug.LastTime) > bug.obsoletePeriod() {
log.Infof(c, "%v: obsoleting: %v", bug.Namespace, bug.Title)
return createNotification(c, dashapi.BugNotifObsoleted, false, "", bug, reporting, bugReporting)
}
@@ -222,6 +222,39 @@
return nil, nil
}
+func (bug *Bug) obsoletePeriod() time.Duration {
+ period := never
+ if config.Obsoleting.MinPeriod == 0 {
+ return period
+ }
+ // Before we have at least 10 crashes, any estimation of frequency is too imprecise.
+ // In such case we conservatively assume it still happens.
+ if bug.NumCrashes >= 10 {
+ // This is linear extrapolation for when the next crash should happen.
+ period = bug.LastTime.Sub(bug.FirstTime) / time.Duration(bug.NumCrashes-1)
+ // Let's be conservative with obsoleting too early.
+ period *= 100
+ }
+ min, max := config.Obsoleting.MinPeriod, config.Obsoleting.MaxPeriod
+ if config.Obsoleting.NonFinalMinPeriod != 0 &&
+ bug.Reporting[len(bug.Reporting)-1].Reported.IsZero() {
+ min, max = config.Obsoleting.NonFinalMinPeriod, config.Obsoleting.NonFinalMaxPeriod
+ }
+ if len(bug.HappenedOn) == 1 {
+ mgr := config.Namespaces[bug.Namespace].Managers[bug.HappenedOn[0]]
+ if mgr.ObsoletingMinPeriod != 0 {
+ min, max = mgr.ObsoletingMinPeriod, mgr.ObsoletingMaxPeriod
+ }
+ }
+ if period < min {
+ period = min
+ }
+ if period > max {
+ period = max
+ }
+ return period
+}
+
func createNotification(c context.Context, typ dashapi.BugNotif, public bool, text string, bug *Bug,
reporting *Reporting, bugReporting *BugReporting) (*dashapi.BugNotification, error) {
reportingConfig, err := json.Marshal(reporting.Config)
