From fc5508f8a32d067af42e9575704e9117781b337b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 9 May 2018 11:10:17 -0700 Subject: [PATCH] agent/cache: string through attempt rather than storing on the entry --- agent/cache/cache.go | 16 ++++++++-------- agent/cache/entry.go | 7 +++---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/agent/cache/cache.go b/agent/cache/cache.go index 754d635df7..d0d5665b1a 100644 --- a/agent/cache/cache.go +++ b/agent/cache/cache.go @@ -257,7 +257,7 @@ RETRY_GET: // At this point, we know we either don't have a value at all or the // value we have is too old. We need to wait for new data. - waiterCh, err := c.fetch(t, key, r, true) + waiterCh, err := c.fetch(t, key, r, true, 0) if err != nil { return nil, err } @@ -287,7 +287,7 @@ func (c *Cache) entryKey(r *RequestInfo) string { // If allowNew is true then the fetch should create the cache entry // if it doesn't exist. If this is false, then fetch will do nothing // if the entry doesn't exist. This latter case is to support refreshing. -func (c *Cache) fetch(t, key string, r Request, allowNew bool) (<-chan struct{}, error) { +func (c *Cache) fetch(t, key string, r Request, allowNew bool, attempt uint8) (<-chan struct{}, error) { // Get the type that we're fetching c.typesLock.RLock() tEntry, ok := c.types[t] @@ -355,14 +355,14 @@ func (c *Cache) fetch(t, key string, r Request, allowNew bool) (<-chan struct{}, metrics.IncrCounter([]string{"consul", "cache", "fetch_success"}, 1) metrics.IncrCounter([]string{"consul", "cache", t, "fetch_success"}, 1) - // Reset the attepts counter so we don't have any backoff - newEntry.ErrAttempts = 0 + // Reset the attempts counter so we don't have any backoff + attempt = 0 } else { metrics.IncrCounter([]string{"consul", "cache", "fetch_error"}, 1) metrics.IncrCounter([]string{"consul", "cache", t, "fetch_error"}, 1) - // Always increment the attempts to control backoff - newEntry.ErrAttempts++ + // Increment attempt counter + attempt++ // If the entry wasn't valid, we set an error. If it was valid, // we don't set an error so that the prior value can continue @@ -399,7 +399,7 @@ func (c *Cache) fetch(t, key string, r Request, allowNew bool) (<-chan struct{}, // If refresh is enabled, run the refresh in due time. The refresh // below might block, but saves us from spawning another goroutine. if tEntry.Opts.Refresh { - c.refresh(tEntry.Opts, newEntry.ErrAttempts, t, key, r) + c.refresh(tEntry.Opts, attempt, t, key, r) } }() @@ -456,7 +456,7 @@ func (c *Cache) refresh(opts *RegisterOptions, attempt uint8, t string, key stri // Trigger. The "allowNew" field is false because in the time we were // waiting to refresh we may have expired and got evicted. If that // happened, we don't want to create a new entry. - c.fetch(t, key, r, false) + c.fetch(t, key, r, false, attempt) } // runExpiryLoop is a blocking function that watches the expiration diff --git a/agent/cache/entry.go b/agent/cache/entry.go index 6bab621a30..50c575ff73 100644 --- a/agent/cache/entry.go +++ b/agent/cache/entry.go @@ -16,10 +16,9 @@ type cacheEntry struct { Index uint64 // Metadata that is used for internal accounting - Valid bool // True if the Value is set - Fetching bool // True if a fetch is already active - Waiter chan struct{} // Closed when this entry is invalidated - ErrAttempts uint8 // Number of fetch errors since last success (Error may be nil) + Valid bool // True if the Value is set + Fetching bool // True if a fetch is already active + Waiter chan struct{} // Closed when this entry is invalidated // Expiry contains information about the expiration of this // entry. This is a pointer as its shared as a value in the