fillcache pkg: trigger cache update immediately

internal/pkg/groups/fillcache.go

@@ -18,9 +18,9 @@ const (
 type MemberSetCache interface {
 	// Get returns a MemberSet from the cache
 	Get(string) (MemberSet, bool)
-	// Update updates the MemberSet of a given key, return a boolean updated value, and and error
-	Update(string) (bool, error)
-	// RefreshLoop starts an update refresh loop for a given key and returns a boolean value of it was started
+	// Update updates the MemberSet of a given key and returns a boolean value indicating whether the value was updated or not.
+	Update(string) bool
+	// RefreshLoop starts an update refresh loop for a given key and returns a boolean value indicating whether a refresh loop has been started or not
 	RefreshLoop(string) bool
 	// Stop is a function to stop all goroutines that may have been spun up for the cache.
 	Stop()
@@ -71,11 +71,14 @@ func (c *FillCache) Get(group string) (MemberSet, bool) {
 // Update recomputes the value for the given key, unless another goroutine is
 // already computing the value, and returns a bool indicating whether the value
 // was updated
-func (c *FillCache) Update(group string) (bool, error) {
+func (c *FillCache) Update(group string) bool {
+	logger := log.NewLogEntry()
+	logger.WithUserGroup(group).Info("updating fill cache")
+
 	c.mu.Lock()
 	if _, waiting := c.inflight[group]; waiting {
 		c.mu.Unlock()
-		return false, nil
+		return false
 	}
 
 	c.inflight[group] = struct{}{}
@@ -88,9 +91,16 @@ func (c *FillCache) Update(group string) (bool, error) {
 
 	if err == nil {
 		c.cache[group] = val
-		return true, nil
+		return true
 	}
-	return false, err
+
+	c.StatsdClient.Incr("groups_cache.error",
+		[]string{
+			fmt.Sprintf("group:%s", group),
+			fmt.Sprintf("error:%s", err),
+		}, 1.0)
+	logger.WithUserGroup(group).Error(err, "error updating fill cache")
+	return false
 }
 
 // RefreshLoop runs in a separate goroutine for the key in the cache and
@@ -117,31 +127,27 @@ func (c *FillCache) RefreshLoop(group string) bool {
 
 	ticker := time.NewTicker(c.refreshTTL)
 	go func() {
+		logger := log.NewLogEntry()
 		// cleanup if this goroutine exits
 		defer func() {
 			c.mu.Lock()
 			delete(c.refreshLoopGroups, group)
 			c.mu.Unlock()
 		}()
 
+		// we update the cache once before looping to ensure the cache is filled immediately,
+		// instead of only after c.refreshTTL
+		updated := c.Update(group)
+		if !updated {
+			logger.WithUserGroup(group).Info("cache was not updated")
+		}
+
 		for {
 			select {
 			case <-c.stopCh:
 				return
 			case <-ticker.C:
-				logger := log.NewLogEntry()
-
-				logger.WithUserGroup(group).Info("updating fill cache")
-				updated, err := c.Update(group)
-				if err != nil {
-					c.StatsdClient.Incr("groups_cache.error",
-						[]string{
-							fmt.Sprintf("group:%s", group),
-							fmt.Sprintf("error:%s", err),
-						}, 1.0)
-					logger.WithUserGroup(group).Error(
-						err, "error updating fill cache")
-				}
+				updated = c.Update(group)
 				if !updated {
 					logger.WithUserGroup(group).Info("cache was not updated")
 				}

internal/pkg/groups/fillcache_test.go

@@ -14,34 +14,27 @@ func testFillFunc(members MemberSet, fillError error) func(string) (MemberSet, e
 
 func TestFillCacheUpdate(t *testing.T) {
 	testCases := []struct {
-		name          string
-		members       MemberSet
-		fillError     error
-		updated       bool
-		expectedError bool
+		name      string
+		members   MemberSet
+		fillError error
+		updated   bool
 	}{
 		{
-			name:    "update to empty cache, no fill errors",
+			name:    "successful update to empty cache",
 			members: MemberSet{"a": {}, "b": {}, "c": {}},
 			updated: true,
 		},
 		{
-			name:          "update with fill function error",
-			fillError:     fmt.Errorf("fill error"),
-			expectedError: true,
+			name:      "unsuccessful update to cache",
+			fillError: fmt.Errorf("fill error"),
+			updated:   false,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			fillCache := NewFillCache(testFillFunc(tc.members, tc.fillError), time.Hour)
 			defer fillCache.Stop()
-			ok, err := fillCache.Update("groupKey")
-			if err == nil && tc.expectedError {
-				t.Errorf("expected error but err was nil")
-			}
-			if err != nil && !tc.expectedError {
-				t.Errorf("unexpected error %s", err)
-			}
+			ok := fillCache.Update("groupKey")
 			if tc.updated != ok {
 				t.Errorf("expected updated to be %v but was %v", tc.updated, ok)
 			}
@@ -71,11 +64,22 @@ func TestRefreshLoop(t *testing.T) {
 			if tc.refreshLoopGroups != nil {
 				fillCache.refreshLoopGroups = tc.refreshLoopGroups
 			}
+
 			started := fillCache.RefreshLoop("group1")
+
 			if tc.expectedStarted != started {
 				t.Errorf("expected started to be %v but was %v", tc.expectedStarted, started)
 			}
 
+			if tc.expectedStarted {
+				// wait briefly to allow the cache to be updated
+				time.Sleep(50 * time.Millisecond)
+				_, ok := fillCache.Get("group1")
+				if !ok {
+					t.Errorf("expected the group cache to be updated immediately")
+				}
+			}
+
 		})
 	}
 

internal/pkg/groups/mock_cache.go

@@ -5,7 +5,6 @@ type MockCache struct {
 	ListMembershipsFunc func(string) (MemberSet, bool)
 	Exists              bool
 	Updated             bool
-	UpdateError         error
 	Refreshed           bool
 }
 
@@ -15,8 +14,8 @@ func (mc *MockCache) Get(group string) (MemberSet, bool) {
 }
 
 // Update updates the cache
-func (mc *MockCache) Update(string) (bool, error) {
-	return mc.Updated, mc.UpdateError
+func (mc *MockCache) Update(string) bool {
+	return mc.Updated
 }
 
 // RefreshLoop returns a boolean of if the refresh loop is refreshed