Merge pull request #33 from tstromberg/first-run

Refactor cache interfaces to accept stale data during startup

cmd/server/main.go

@@ -20,8 +20,10 @@ import (
 	"fmt"
 	"net/http"
 	"os"
+	"os/signal"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/google/go-github/v31/github"
@@ -48,10 +50,13 @@ var (
 	port          = flag.Int("port", 8080, "port to run server at")
 	siteName      = flag.String("site_name", "", "override site name from config file")
 
-	maxListAge    = flag.Duration("max_list_age", 12*time.Hour, "maximum time to cache GitHub searches")
+	itemExpiry = flag.Duration("item_expiry", 12*time.Hour, "maximum time to cache GitHub search results")
+	orgExpiry  = flag.Duration("org_expiry", 30*12*time.Hour, "maximum time to cache GitHub organizational membership")
+
 	maxRefreshAge = flag.Duration("max_refresh_age", 15*time.Minute, "Maximum time between collection runs")
-	minRefreshAge = flag.Duration("min_refresh_age", 15*time.Second, "Minimum time between collection runs")
-	warnAge       = flag.Duration("warn_age", 30*time.Minute, "Maximum time before warning about stale results. Recommended: 2*max_refresh_age")
+	minRefreshAge = flag.Duration("min_refresh_age", 60*time.Second, "Minimum time between collection runs")
+
+	warnAge = flag.Duration("warn_age", 30*time.Minute, "Maximum time before warning about stale results. Recommended: 2*max_refresh_age")
 )
 
 func main() {
@@ -89,16 +94,16 @@ func main() {
 	}
 	klog.Infof("cache path: %s", cachePath)
 
-	c, err := initcache.Load(cachePath)
-	if err != nil {
+	c := initcache.New(initcache.Config{Type: "disk", Path: cachePath})
+	if err := c.Initialize(); err != nil {
 		klog.Exitf("initcache load to %s: %v", cachePath, err)
 	}
 
 	cfg := triage.Config{
-		Client:      client,
-		Cache:       c,
-		MaxListAge:  *maxListAge,
-		MaxEventAge: 90 * 24 * time.Hour,
+		Client:          client,
+		Cache:           c,
+		ItemExpiry:      *itemExpiry,
+		OrgMemberExpiry: *orgExpiry,
 	}
 
 	if *reposOverride != "" {
@@ -122,7 +127,7 @@ func main() {
 	}
 
 	// Make sure save works
-	if err := initcache.Save(c, cachePath); err != nil {
+	if err := c.Save(); err != nil {
 		klog.Exitf("initcache save to %s: %v", cachePath, err)
 	}
 
@@ -131,7 +136,7 @@ func main() {
 		MinRefreshAge: *minRefreshAge,
 		MaxRefreshAge: *maxRefreshAge,
 		PersistFunc: func() error {
-			return initcache.Save(c, cachePath)
+			return c.Save()
 		},
 	})
 
@@ -144,9 +149,21 @@ func main() {
 	}
 
 	klog.Infof("Starting update loop: %+v", u)
+	sigc := make(chan os.Signal, 1)
+	signal.Notify(sigc, os.Interrupt, syscall.SIGTERM)
+	go func() {
+		for sig := range sigc {
+			klog.Infof("signal caught: %v", sig)
+			if err := c.Save(); err != nil {
+				klog.Errorf("save errro: %v", err)
+			}
+			os.Exit(0)
+		}
+	}()
+
 	go func() {
-		if err := u.Loop(ctx); err != nil {
-			panic(fmt.Errorf("update loop failed: %w", err))
+		if err := u.Loop(ctx); err == nil {
+			klog.Exitf("loop failed: %v", err)
 		}
 	}()
 

cmd/tester/main.go

@@ -74,17 +74,17 @@ func main() {
 
 	}
 
-	c, err := initcache.Load(cachePath)
-	if err != nil {
+	c := initcache.New(initcache.Config{Type: "disk", Path: cachePath})
+	if err := c.Initialize(); err != nil {
 		klog.Exitf("initcache load to %s: %v", cachePath, err)
 	}
 
 	cfg := triage.Config{
-		Client:      client,
-		Cache:       c,
-		MaxListAge:  24 * time.Hour,
-		MaxEventAge: 90 * 24 * time.Hour,
-		DebugNumber: *number,
+		Client:          client,
+		Cache:           c,
+		ItemExpiry:      7 * 24 * time.Hour,
+		OrgMemberExpiry: 90 * 24 * time.Hour,
+		DebugNumber:     *number,
 	}
 
 	if *reposOverride != "" {
@@ -102,7 +102,7 @@ func main() {
 		executeRule(ctx, tp)
 	}
 
-	if err := initcache.Save(c, cachePath); err != nil {
+	if err := c.Save(); err != nil {
 		klog.Exitf("initcache save to %s: %v", cachePath, err)
 	}
 }
@@ -113,7 +113,7 @@ func executeCollection(ctx context.Context, tp *triage.Party) {
 		klog.Exitf("collection: %v", err)
 	}
 
-	r, err := tp.ExecuteCollection(ctx, s)
+	r, err := tp.ExecuteCollection(ctx, s, time.Time{})
 	if err != nil {
 		klog.Exitf("execute: %v", err)
 	}
@@ -143,7 +143,7 @@ func executeRule(ctx context.Context, tp *triage.Party) {
 		klog.Exitf("rule: %v", err)
 	}
 
-	rr, err := tp.ExecuteRule(ctx, r, nil)
+	rr, err := tp.ExecuteRule(ctx, r, nil, time.Time{})
 	if err != nil {
 		klog.Exitf("execute: %v", err)
 	}

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/imjasonmiller/godice v0.1.2
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
-	golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
 	gopkg.in/yaml.v2 v2.2.8
 	k8s.io/klog v1.0.0
 )

go.sum

@@ -9,6 +9,8 @@ github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
+github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY=
+github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ=
 github.com/google/go-github/v31 v31.0.0 h1:JJUxlP9lFK+ziXKimTCprajMApV1ecWD4NB6CCb0plo=
 github.com/google/go-github/v31 v31.0.0/go.mod h1:NQPZol8/1sMoWYGN2yaALIBytu17gAWfhbweiEed3pM=
 github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
@@ -37,10 +39,9 @@ golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4Iltr
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 h1:YyJpGZS1sBuBCzLAR1VEpK193GlqGZbnPFnPV/5Rsb4=
 golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
-golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=

pkg/hubbub/cache.go

@@ -4,89 +4,54 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/google/go-github/v31/github"
 	"k8s.io/klog"
 )
 
-// PRCommentCache are cached comments
-type PRCommentCache struct {
-	Time    time.Time
-	Content []*github.PullRequestComment
-}
-
-// PRSearchCache are cached PR's
-type PRSearchCache struct {
-	Time    time.Time
-	Content []*github.PullRequest
-}
-
-// IssueCommentCache are cached issue comments
-type IssueCommentCache struct {
-	Time    time.Time
-	Content []*github.IssueComment
-}
-
-// IssueSearchCache are cached issues
-type IssueSearchCache struct {
-	Time    time.Time
-	Content []*github.Issue
+// Toggle acceptability of stale results, useful for bootstrapping
+func (e *Engine) AcceptStaleResults(b bool) {
+	klog.V(1).Infof("Setting stale results=%v", b)
+	e.acceptStaleResults = b
 }
 
 // FlushSearchCache invalidates the in-memory search cache
-func (h *Engine) FlushSearchCache(org string, project string, minAge time.Duration) error {
-	if err := h.flushIssueSearchCache(org, project, minAge); err != nil {
-		return fmt.Errorf("issues: %v", err)
-	}
-	if err := h.flushPRSearchCache(org, project, minAge); err != nil {
-		return fmt.Errorf("prs: %v", err)
+func (h *Engine) FlushSearchCache(org string, project string, olderThan time.Time) error {
+	if h.acceptStaleResults {
+		return fmt.Errorf("stale results enabled, refusing to flush")
 	}
+
+	h.flushIssueSearchCache(org, project, olderThan)
+	h.flushPRSearchCache(org, project, olderThan)
 	return nil
 }
 
-func (h *Engine) flushIssueSearchCache(org string, project string, minAge time.Duration) error {
-	klog.Infof("flushIssues older than %s: %s/%s", minAge, org, project)
+func (h *Engine) flushIssueSearchCache(org string, project string, olderThan time.Time) {
+	klog.Infof("flushIssues older than %s: %s/%s", olderThan, org, project)
 
 	keys := []string{
 		issueSearchKey(org, project, "open", 0),
 		issueSearchKey(org, project, "closed", closedIssueDays),
 	}
 
 	for _, key := range keys {
-		x, ok := h.cache.Get(key)
-		if !ok {
-			return fmt.Errorf("no such key: %v", key)
-		}
-		is := x.(IssueSearchCache)
-		if time.Since(is.Time) < minAge {
-			return fmt.Errorf("%s not old enough: %v", key, is.Time)
+		if err := h.cache.DeleteOlderThan(key, olderThan); err != nil {
+			klog.Warningf("delete %q: %v", key, err)
 		}
-		klog.Infof("Flushing %s", key)
-		h.cache.Delete(key)
 	}
-	return nil
 }
 
-func (h *Engine) flushPRSearchCache(org string, project string, minAge time.Duration) error {
-	klog.Infof("flushPRs older than %s: %s/%s", minAge, org, project)
+func (h *Engine) flushPRSearchCache(org string, project string, olderThan time.Time) {
+	klog.Infof("flushPRs older than %s: %s/%s", olderThan, org, project)
 
 	keys := []string{
 		issueSearchKey(org, project, "open", 0),
 		issueSearchKey(org, project, "closed", closedPRDays),
 	}
 
 	for _, key := range keys {
-		x, ok := h.cache.Get(key)
-		if !ok {
-			return fmt.Errorf("no such key: %v", key)
-		}
-		is := x.(PRSearchCache)
-		if time.Since(is.Time) < minAge {
-			return fmt.Errorf("%s not old enough: %v", key, is.Time)
+		if err := h.cache.DeleteOlderThan(key, olderThan); err != nil {
+			klog.Warningf("delete %q: %v", key, err)
 		}
-		klog.Infof("Flushing %s", key)
-		h.cache.Delete(key)
 	}
-	return nil
 }
 
 // issueSearchKey is the cache key used for issues

pkg/hubbub/hubbub.go

@@ -19,15 +19,18 @@ import (
 	"time"
 
 	"github.com/google/go-github/v31/github"
+	"github.com/google/triage-party/pkg/initcache"
 )
 
 // Config is how to configure a new hubbub engine
 type Config struct {
-	Client      *github.Client // Client is a GitHub client
-	Cache       Cacher         // Cacher is a cache interface
-	Repos       []string       // Repos is the repositories to search
-	MaxListAge  time.Duration  // MaxListAge is when GitHub searches go bad
-	MaxEventAge time.Duration  // MaxEventAge is when GitHub events go bad
+	Client *github.Client   // Client is a GitHub client
+	Cache  initcache.Cacher // Cacher is a cache interface
+	Repos  []string         // Repos is the repositories to search
+
+	// Cache expiration times
+	ItemExpiry      time.Duration
+	OrgMemberExpiry time.Duration
 
 	// MinSimilarity is how close two items need to be to each other to be called similar
 	MinSimilarity float64
@@ -38,10 +41,11 @@ type Config struct {
 
 // Engine is the search engine interface for hubbub
 type Engine struct {
-	cache       Cacher
-	client      *github.Client
-	maxListAge  time.Duration
-	maxEventAge time.Duration
+	cache  initcache.Cacher
+	client *github.Client
+
+	itemExpiry      time.Duration
+	orgMemberExpiry time.Duration
 
 	// Must be settable from config
 	MinSimilarity float64
@@ -56,14 +60,18 @@ type Engine struct {
 
 	// when did we last see a new item?
 	lastItemUpdate time.Time
+
+	// are stale results acceptable?
+	acceptStaleResults bool
 }
 
 func New(cfg Config) *Engine {
 	e := &Engine{
-		cache:       cfg.Cache,
-		client:      cfg.Client,
-		maxListAge:  cfg.MaxListAge,
-		maxEventAge: cfg.MaxEventAge,
+		cache:  cfg.Cache,
+		client: cfg.Client,
+
+		itemExpiry:      cfg.ItemExpiry,
+		orgMemberExpiry: cfg.OrgMemberExpiry,
 
 		seen:          map[string]*Conversation{},
 		similarCache:  map[string][]string{},
@@ -72,10 +80,3 @@ func New(cfg Config) *Engine {
 	}
 	return e
 }
-
-// Cacher is the supported cache interface for GitHub data
-type Cacher interface {
-	Set(string, interface{}, time.Duration)
-	Delete(string)
-	Get(string) (interface{}, bool)
-}