[gold] Add firestore implementation of ignorestore.
Note to reviewer: PS1 adds just Create and List.
Change-Id: Ifd5f0a82bc4299d29ed3f3483624667d7cc7b9e8
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/264318
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/go/ignore/fs_ignorestore/FIRESTORE.md b/golden/go/ignore/fs_ignorestore/FIRESTORE.md
new file mode 100644
index 0000000..264074b
--- /dev/null
+++ b/golden/go/ignore/fs_ignorestore/FIRESTORE.md
@@ -0,0 +1,32 @@
+Storing Ignore Rules in Firestore
+=================================
+
+We need to store the user-created rules for ignoring certain sets of traces.
+
+Schema
+------
+
+We should have one Firestore Collection (i.e. table) for these rules.
+
+ ruleEntry
+ # ID will be autogenerated
+ CreatedBy string # email address
+ UpdatedBy string # email address
+ Expires time.Time
+ Query string
+ Note string
+
+Indexing
+--------
+
+No indexes required.
+
+Usage
+-----
+
+We just Add, Update, Delete, and List (all).
+
+Growth Opportunities
+-------------------
+
+None projected.
\ No newline at end of file
diff --git a/golden/go/ignore/fs_ignorestore/fs_ignorestore.go b/golden/go/ignore/fs_ignorestore/fs_ignorestore.go
new file mode 100644
index 0000000..71922bc
--- /dev/null
+++ b/golden/go/ignore/fs_ignorestore/fs_ignorestore.go
@@ -0,0 +1,195 @@
+// Package fs_ignorestore hosts a Firestore-based implementation of ignore.Store.
+package fs_ignorestore
+
+import (
+ "context"
+ "math/rand"
+ "sort"
+ "sync"
+ "time"
+
+ "cloud.google.com/go/firestore"
+
+ ifirestore "go.skia.org/infra/go/firestore"
+ "go.skia.org/infra/go/skerr"
+ "go.skia.org/infra/go/sklog"
+ "go.skia.org/infra/golden/go/ignore"
+)
+
+const (
+ // These are the collections in Firestore.
+ rulesCollection = "ignorestore_rules"
+
+ maxReadAttempts = 5
+ maxWriteAttempts = 5
+ maxOperationTime = time.Minute
+
+ // recoverTime is the minimum amount of time to wait before recreating any QuerySnapshotIterator
+ // if it fails. A random amount of time should be added to this, proportional to recoverTime.
+ recoverTime = 30 * time.Second
+)
+
+// StoreImpl is the Firestore-based implementation of ignore.Store. It uses query snapshots to
+// synchronize a local cache with the data in Firestore, thus it does not need to poll. We don't
+// do any searches on our data, so keeping all the ignore rules in RAM (backed by Firestore) is
+// a fine solution for the sake of performance (and avoiding unnecessary Firestore traffic). Ignore
+// Rules generally top out in the 100s, plenty to fit in RAM.
+type StoreImpl struct {
+ client *ifirestore.Client
+
+ cacheMutex sync.RWMutex
+ cache map[string]ignore.Rule
+}
+
+// ruleEntry represents how an ignore.Rule is stored in Firestore.
+type ruleEntry struct {
+ CreatedBy string `firestore:"name"`
+ UpdatedBy string `firestore:"updatedby"`
+ Expires time.Time `firestore:"expires"`
+ Query string `firestore:"query"`
+ Note string `firestore:"note"`
+}
+
+func toRule(id string, r ruleEntry) ignore.Rule {
+ return ignore.Rule{
+ ID: id,
+ CreatedBy: r.CreatedBy,
+ UpdatedBy: r.UpdatedBy,
+ Expires: r.Expires,
+ Query: r.Query,
+ Note: r.Note,
+ }
+}
+
+func toEntry(r ignore.Rule) ruleEntry {
+ return ruleEntry{
+ CreatedBy: r.CreatedBy,
+ UpdatedBy: r.UpdatedBy,
+ Expires: r.Expires,
+ Query: r.Query,
+ Note: r.Note,
+ }
+}
+
+// New returns a new StoreImpl.
+func New(ctx context.Context, client *ifirestore.Client) *StoreImpl {
+ s := &StoreImpl{
+ client: client,
+ cache: map[string]ignore.Rule{},
+ }
+ s.startQueryIterator(ctx)
+ return s
+}
+
+// startQueryIterator sets up the listener to the Query Snapshots which keep the local cache of
+// ignore rules in sync with those in Firestore.
+func (s *StoreImpl) startQueryIterator(ctx context.Context) {
+ go func() {
+ // TODO(kjlubick) deduplicate this logic with fs_expstore maybe? We'd like to be able to
+ // recover, so maybe we need a variant of QuerySnapshotChannel
+ snap := s.client.Collection(rulesCollection).Snapshots(ctx)
+ for {
+ if err := ctx.Err(); err != nil {
+ sklog.Debugf("Stopping query of ignores due to context error: %s", err)
+ snap.Stop()
+ return
+ }
+ qs, err := snap.Next()
+ if err != nil {
+ sklog.Errorf("reading query snapshot: %s", err)
+ snap.Stop()
+ // sleep and rebuild the snapshot query. Once a SnapshotQueryIterator returns
+ // an error, it seems to always return that error.
+ t := recoverTime + time.Duration(float32(recoverTime)*rand.Float32())
+ time.Sleep(t)
+ sklog.Infof("Trying to recreate query snapshot after having slept %s", t)
+ snap = s.client.Collection(rulesCollection).Snapshots(ctx)
+ continue
+ }
+ s.updateCacheWithEntriesFrom(qs)
+ }
+ }()
+}
+
+// updateCacheWithEntriesFrom loops through all the changes in the given snapshot and updates
+// the cache with those new values (or deletes the old ones).
+func (s *StoreImpl) updateCacheWithEntriesFrom(qs *firestore.QuerySnapshot) {
+ s.cacheMutex.Lock()
+ defer s.cacheMutex.Unlock()
+ for _, dc := range qs.Changes {
+ id := dc.Doc.Ref.ID
+ if dc.Kind == firestore.DocumentRemoved {
+ delete(s.cache, id)
+ continue
+ }
+ entry := ruleEntry{}
+ if err := dc.Doc.DataTo(&entry); err != nil {
+ sklog.Errorf("corrupt data in firestore, could not unmarshal ruleEntry with id %s", id)
+ continue
+ }
+ s.cache[id] = toRule(id, entry)
+ }
+}
+
+// Create implements the ignore.Store interface.
+func (s *StoreImpl) Create(ctx context.Context, r ignore.Rule) error {
+ doc := s.client.Collection(rulesCollection).NewDoc()
+ if _, err := s.client.Create(ctx, doc, toEntry(r), maxWriteAttempts, maxOperationTime); err != nil {
+ return skerr.Wrapf(err, "storing new ignore rule to Firestore (%#v)", r)
+ }
+ return nil
+}
+
+// List implements the ignore.Store interface. It returns the local cache of ignore rules, never
+// re-fetching from Firestore because the query snapshots should keep the local cache up to date,
+// give or take a few seconds.
+func (s *StoreImpl) List(_ context.Context) ([]ignore.Rule, error) {
+ s.cacheMutex.RLock()
+ defer s.cacheMutex.RUnlock()
+ rv := make([]ignore.Rule, 0, len(s.cache))
+ for _, r := range s.cache {
+ rv = append(rv, r)
+ }
+ sort.Slice(rv, func(i, j int) bool {
+ return rv[i].Expires.Before(rv[j].Expires)
+ })
+ return rv, nil
+}
+
+// Update implements the ignore.Store interface.
+func (s *StoreImpl) Update(ctx context.Context, rule ignore.Rule) error {
+ if rule.ID == "" {
+ return skerr.Fmt("ID for ignore rule cannot be empty")
+ }
+ doc := s.client.Collection(rulesCollection).Doc(rule.ID)
+ dSnap, err := s.client.Get(ctx, doc, maxReadAttempts, maxOperationTime)
+ if err != nil {
+ return skerr.Wrapf(err, "getting ignore rule %s before updating", rule.ID)
+ }
+ var oldRule ruleEntry
+ if err := dSnap.DataTo(&oldRule); err != nil {
+ return skerr.Wrapf(err, "corrupt data in firestore for id %s", rule.ID)
+ }
+ updatedRule := toEntry(rule)
+ updatedRule.CreatedBy = oldRule.CreatedBy
+ if _, err := s.client.Set(ctx, doc, updatedRule, maxWriteAttempts, maxOperationTime); err != nil {
+ return skerr.Wrapf(err, "updating ignore rule %s", rule.ID)
+ }
+ return nil
+}
+
+// Delete implements the ignore.Store interface.
+func (s *StoreImpl) Delete(ctx context.Context, id string) (bool, error) {
+ if id == "" {
+ return false, skerr.Fmt("ID for ignore rule cannot be empty")
+ }
+ s.client.Collection(rulesCollection).Doc(id)
+ if wr, err := s.client.Collection(rulesCollection).Doc(id).Delete(ctx); err != nil {
+ return false, skerr.Wrapf(err, "deleting ignore rule with id %s", id)
+ } else if wr.UpdateTime.Unix() <= 0 {
+ // UpdateTime is only set if something is modified. If the document didn't exist, UpdateTime
+ // will be set to the epoch.
+ return false, nil
+ }
+ return true, nil
+}
diff --git a/golden/go/ignore/fs_ignorestore/fs_ignorestore_test.go b/golden/go/ignore/fs_ignorestore/fs_ignorestore_test.go
new file mode 100644
index 0000000..3865366
--- /dev/null
+++ b/golden/go/ignore/fs_ignorestore/fs_ignorestore_test.go
@@ -0,0 +1,211 @@
+package fs_ignorestore
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "go.skia.org/infra/go/deepequal"
+ "go.skia.org/infra/go/firestore"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/golden/go/ignore"
+)
+
+func TestCreateListIgnoreRule(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := newEmptyStore(ctx, t, c)
+
+ xir := makeIgnoreRules()
+ // Add them in a not-sorted order to make sure List sorts them.
+ require.NoError(t, f.Create(ctx, xir[2]))
+ require.NoError(t, f.Create(ctx, xir[0]))
+ require.NoError(t, f.Create(ctx, xir[3]))
+ require.NoError(t, f.Create(ctx, xir[1]))
+
+ requireCurrentListMatchesExpected(t, ctx, f)
+}
+
+func TestCreateDelete(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := newEmptyStore(ctx, t, c)
+
+ xir := makeIgnoreRules()
+ // Add 0, 1, 2, 2, 2, 2, 3 (there are 3 extra of index 2)
+ require.NoError(t, f.Create(ctx, xir[0]))
+ require.NoError(t, f.Create(ctx, xir[1]))
+ require.NoError(t, f.Create(ctx, xir[2]))
+ require.NoError(t, f.Create(ctx, xir[2]))
+ require.NoError(t, f.Create(ctx, xir[2]))
+ require.NoError(t, f.Create(ctx, xir[2]))
+ require.NoError(t, f.Create(ctx, xir[3]))
+ // Wait until those 7 rules are in the list
+ require.Eventually(t, func() bool {
+ actualRules, _ := f.List(ctx)
+ return len(actualRules) == 7
+ }, 5*time.Second, 200*time.Millisecond)
+ // Re-query the rules to make sure none got dropped or added unexpectedly.
+ actualRules, err := f.List(ctx)
+ require.NoError(t, err)
+ require.Len(t, actualRules, 7) // should still have 7 elements in the list
+
+ ok, err := f.Delete(ctx, actualRules[3].ID)
+ require.NoError(t, err)
+ assert.True(t, ok)
+ ok, err = f.Delete(ctx, actualRules[4].ID)
+ require.NoError(t, err)
+ assert.True(t, ok)
+ ok, err = f.Delete(ctx, actualRules[5].ID)
+ require.NoError(t, err)
+ assert.True(t, ok)
+
+ requireCurrentListMatchesExpected(t, ctx, f)
+}
+
+func TestDeleteNonExistentRule(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := New(ctx, c)
+ ok, err := f.Delete(ctx, "Not in there")
+ require.NoError(t, err)
+ assert.False(t, ok)
+}
+
+func TestDeleteEmptyRule(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := New(ctx, c)
+ _, err := f.Delete(ctx, "")
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "empty")
+}
+
+func TestCreateUpdate(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := newEmptyStore(ctx, t, c)
+
+ xir := makeIgnoreRules()
+ require.NoError(t, f.Create(ctx, xir[1]))
+ // Wait until that rule is in the list
+ require.Eventually(t, func() bool {
+ actualRules, _ := f.List(ctx)
+ return len(actualRules) == 1
+ }, 5*time.Second, 200*time.Millisecond)
+
+ actualRules, err := f.List(ctx)
+ require.NoError(t, err)
+ toUpdateID := actualRules[0].ID
+ newContent := xir[0]
+ newContent.ID = toUpdateID
+
+ require.NoError(t, f.Update(ctx, newContent))
+ require.Eventually(t, func() bool {
+ rules, err := f.List(ctx)
+ assert.NoError(t, err)
+ return compareIgnoreRulesIgnoringIDs(rules, []ignore.Rule{
+ {
+ // Notice the Name is the same as the original rule, but everything else is changed.
+ CreatedBy: "beta@example.com",
+ UpdatedBy: "alpha@example.com",
+ Expires: now.Add(-time.Hour),
+ Query: "config=gpu",
+ Note: "expired",
+ },
+ })
+ }, 5*time.Second, 200*time.Millisecond)
+}
+
+func TestUpdateNonExistentRule(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := New(ctx, c)
+ ir := makeIgnoreRules()[0]
+ ir.ID = "whoops"
+ err := f.Update(ctx, ir)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "before updating")
+}
+
+func TestUpdateEmptyRule(t *testing.T) {
+ unittest.LargeTest(t)
+ c, cleanup := firestore.NewClientForTesting(t)
+ defer cleanup()
+
+ ctx := context.Background()
+ f := New(ctx, c)
+
+ err := f.Update(ctx, ignore.Rule{})
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "empty")
+}
+
+var now = time.Date(2020, time.January, 13, 14, 15, 16, 0, time.UTC)
+
+func newEmptyStore(ctx context.Context, t *testing.T, c *firestore.Client) *StoreImpl {
+ f := New(ctx, c)
+ empty, err := f.List(ctx)
+ require.NoError(t, err)
+ require.Empty(t, empty)
+ return f
+}
+
+func makeIgnoreRules() []ignore.Rule {
+ return []ignore.Rule{
+ ignore.NewRule("alpha@example.com", now.Add(-time.Hour), "config=gpu", "expired"),
+ ignore.NewRule("beta@example.com", now.Add(time.Minute*10), "config=8888", "No good reason."),
+ ignore.NewRule("beta@example.com", now.Add(time.Minute*50), "extra=123&extra=abc", "Ignore multiple."),
+ ignore.NewRule("alpha@example.com", now.Add(time.Minute*100), "extra=123&extra=abc&config=8888", "Ignore multiple 2."),
+ }
+}
+
+// compareIgnoreRulesIgnoringIDs returns true if the two lists of rules match (disregarding the ID).
+// ID is ignored because it is nondeterministic.
+func compareIgnoreRulesIgnoringIDs(first []ignore.Rule, second []ignore.Rule) bool {
+ if len(first) != len(second) {
+ return false
+ }
+ for i := range first {
+ r1, r2 := first[i], second[i]
+ r1.ID = ""
+ r2.ID = ""
+ if !deepequal.DeepEqual(r1, r2) {
+ return false
+ }
+ }
+ return true
+}
+
+// requireCurrentListMatchesExpected either returns because the content in the given store
+// matches makeIgnoreRules() or it panics because it does not match.
+func requireCurrentListMatchesExpected(t *testing.T, ctx context.Context, f *StoreImpl) {
+ // List uses a query snapshot, which is not synchronous, so we might have to query a few times
+ // before everything syncs up.
+ require.Eventually(t, func() bool {
+ actualRules, err := f.List(ctx)
+ assert.NoError(t, err)
+ return compareIgnoreRulesIgnoringIDs(actualRules, makeIgnoreRules())
+ }, 5*time.Second, 200*time.Millisecond)
+}
diff --git a/golden/go/ignore/ignore.go b/golden/go/ignore/ignore.go
index 6add555..47e254a 100644
--- a/golden/go/ignore/ignore.go
+++ b/golden/go/ignore/ignore.go
@@ -21,7 +21,8 @@
// List returns all ignore rules in the ignore store.
List(context.Context) ([]Rule, error)
- // Update sets a Rule.
+ // Update sets a Rule. The "Name" field should be ignored, but all other fields should be
+ // applied to the existing data.
Update(ctx context.Context, rule Rule) error
// Delete removes a Rule from the store. The boolean indicates if the deletion was successful
