agent/cache: on error, return from Get immediately, don't block forever

agent/cache/cache.go

@@ -193,6 +193,15 @@ RETRY_GET:
 		}
 	}
 
+	// If this isn't our first time through and our last value has an error,
+	// then we return the error. This has the behavior that we don't sit in
+	// a retry loop getting the same error for the entire duration of the
+	// timeout. Instead, we make one effort to fetch a new value, and if
+	// there was an error, we return.
+	if !first && entry.Error != nil {
+		return entry.Value, entry.Error
+	}
+
 	if first {
 		// Record the miss if its our first time through
 		atomic.AddUint64(&c.misses, 1)
@@ -308,6 +317,12 @@ func (c *Cache) fetch(t, key string, r Request) (<-chan struct{}, error) {
 			newEntry.Valid = true
 		}
 
+		// If we have an error and the prior entry wasn't valid, then we
+		// set the error at least.
+		if err != nil && !newEntry.Valid {
+			newEntry.Error = err
+		}
+
 		// Create a new waiter that will be used for the next fetch.
 		newEntry.Waiter = make(chan struct{})
 

agent/cache/cache_test.go

@@ -42,6 +42,38 @@ func TestCacheGet_noIndex(t *testing.T) {
 	typ.AssertExpectations(t)
 }
 
+// Test a basic Get with no index and a failed fetch.
+func TestCacheGet_initError(t *testing.T) {
+	t.Parallel()
+
+	require := require.New(t)
+
+	typ := TestType(t)
+	defer typ.AssertExpectations(t)
+	c := TestCache(t)
+	c.RegisterType("t", typ, nil)
+
+	// Configure the type
+	fetcherr := fmt.Errorf("error")
+	typ.Static(FetchResult{}, fetcherr).Times(2)
+
+	// Get, should fetch
+	req := TestRequest(t, RequestInfo{Key: "hello"})
+	result, err := c.Get("t", req)
+	require.Error(err)
+	require.Nil(result)
+
+	// Get, should fetch again since our last fetch was an error
+	result, err = c.Get("t", req)
+	require.Error(err)
+	require.Nil(result)
+
+	// Sleep a tiny bit just to let maybe some background calls happen
+	// then verify that we still only got the one call
+	time.Sleep(20 * time.Millisecond)
+	typ.AssertExpectations(t)
+}
+
 // Test a Get with a request that returns a blank cache key. This should
 // force a backend request and skip the cache entirely.
 func TestCacheGet_blankCacheKey(t *testing.T) {