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
This commit is contained in:
Dmitry Vyukov 2019-10-25 15:56:00 +02:00
parent c401a48c99
commit c2e837da36
4 changed files with 181 additions and 5 deletions

View File

@ -32,6 +32,12 @@ var testConfig = &GlobalConfig{
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 @@ var testConfig = &GlobalConfig{
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",

View File

@ -33,6 +33,8 @@ type GlobalConfig struct {
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 @@ type Config struct {
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 @@ type ConfigManager struct {
// 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 @@ func checkConfig(cfg *GlobalConfig) {
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 checkConfig(cfg *GlobalConfig) {
}
}
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 @@ func checkManager(ns, name string, mgr ConfigManager) {
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) {

View File

@ -102,6 +102,81 @@ func TestEmailNotifBadFix(t *testing.T) {
}
}
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 @@ func TestEmailNotifObsoleted(t *testing.T) {
c.expectNoEmail()
// Not yet.
c.advanceTime(119 * 24 * time.Hour)
c.advanceTime(59 * 24 * time.Hour)
c.expectNoEmail()
// Now!
@ -145,7 +220,7 @@ func TestEmailNotifObsoleted(t *testing.T) {
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 @@ func TestEmailNotifNotObsoleted(t *testing.T) {
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)

View File

@ -31,7 +31,7 @@ const (
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 @@ func handleReportNotif(c context.Context, typ string, bug *Bug) (*dashapi.BugNot
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 @@ func handleReportNotif(c context.Context, typ string, bug *Bug) (*dashapi.BugNot
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)