20187: Expire cache and start returning errors after 24h.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 9054dd633023309cc117c21bdb38d4740e5b0c7e..045e4a8c9c40b71092fd4e5e6d1976b65f8c1760 100644 (file)
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -226,11 +226,15 @@ type cacheEnt struct {
        mtx          sync.Mutex
        header       http.Header
        body         []byte
+       expireAfter  time.Time
        refreshAfter time.Time
        refreshLock  sync.Mutex
 }
 
-const cacheTTL = 5 * time.Minute
+const (
+       cacheTTL    = 5 * time.Minute
+       cacheExpire = 24 * time.Hour
+)
 
 func (ent *cacheEnt) refresh(path string, do func(*http.Request) (*http.Response, error)) (http.Header, []byte, error) {
        ent.refreshLock.Lock()
@@ -292,12 +296,16 @@ func (ent *cacheEnt) refresh(path string, do func(*http.Request) (*http.Response
        ent.header = header
        ent.body = body
        ent.refreshAfter = time.Now().Add(cacheTTL)
+       ent.expireAfter = time.Now().Add(cacheExpire)
        return ent.header, ent.body, nil
 }
 
 func (ent *cacheEnt) response() (http.Header, []byte, bool) {
        ent.mtx.Lock()
        defer ent.mtx.Unlock()
+       if ent.expireAfter.Before(time.Now()) {
+               ent.header, ent.body, ent.refreshAfter = nil, nil, time.Time{}
+       }
        return ent.header, ent.body, ent.refreshAfter.Before(time.Now())
 }
 

diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index 0cb0e1500988cf8ea8c9dfe96aff959535ba5e9d..fcd70d7cc535bbe1db0dbfab391049e15ac30e72 100644 (file)
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -147,12 +147,18 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
                        defer ent.mtx.Unlock()
                }
        }
-       expireCache := func() {
+       refreshNow := func() {
                waitPendingUpdates()
                for _, ent := range s.handler.cache {
                        ent.refreshAfter = time.Now()
                }
        }
+       expireNow := func() {
+               waitPendingUpdates()
+               for _, ent := range s.handler.cache {
+                       ent.expireAfter = time.Now()
+               }
+       }
 
        // Easy path: first req fetches, subsequent reqs use cache.
        c.Check(countRailsReqs(), check.Equals, 0)
@@ -182,7 +188,7 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
 
        // Race after expiry: concurrent reqs return the cached data
        // but initiate a new fetch in the background.
-       expireCache()
+       refreshNow()
        holdReqs = make(chan struct{})
        wg = getDDConcurrently(5, http.StatusOK, check.Commentf("race after expiry"))
        reqsBefore = countRailsReqs()
@@ -237,20 +243,20 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
        getDDConcurrently(5, http.StatusOK, check.Commentf("error with warm cache")).Wait()
        c.Check(countRailsReqs(), check.Equals, reqsBefore)
 
-       // Error with expired cache => caller gets OK with stale data
+       // Error with stale cache => caller gets OK with stale data
        // while the re-fetch is attempted in the background
-       expireCache()
+       refreshNow()
        wantError, wantBadContent = true, false
        reqsBefore = countRailsReqs()
        holdReqs = make(chan struct{})
-       getDDConcurrently(5, http.StatusOK, check.Commentf("error with expired cache")).Wait()
+       getDDConcurrently(5, http.StatusOK, check.Commentf("error with stale cache")).Wait()
        close(holdReqs)
        // Only one attempt to re-fetch (holdReqs ensured the first
        // update took long enough for the last incoming request to
        // arrive)
        c.Check(countRailsReqs(), check.Equals, reqsBefore+1)
 
-       expireCache()
+       refreshNow()
        wantError, wantBadContent = false, false
        reqsBefore = countRailsReqs()
        holdReqs = make(chan struct{})
@@ -258,6 +264,35 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) {
        close(holdReqs)
        waitPendingUpdates()
        c.Check(countRailsReqs(), check.Equals, reqsBefore+1)
+
+       // Make sure expireAfter is getting set
+       waitPendingUpdates()
+       exp := s.handler.cache["/discovery/v1/apis/arvados/v1/rest"].expireAfter.Sub(time.Now())
+       c.Check(exp > cacheTTL, check.Equals, true)
+       c.Check(exp < cacheExpire, check.Equals, true)
+
+       // After the cache *expires* it behaves as if uninitialized:
+       // each incoming request does a new upstream request until one
+       // succeeds.
+       //
+       // First check failure after expiry:
+       expireNow()
+       wantError, wantBadContent = true, false
+       reqsBefore = countRailsReqs()
+       holdReqs = make(chan struct{})
+       wg = getDDConcurrently(5, http.StatusBadGateway, check.Commentf("error after expiry"))
+       close(holdReqs)
+       wg.Wait()
+       c.Check(countRailsReqs(), check.Equals, reqsBefore+5)
+
+       // Success after expiry:
+       wantError, wantBadContent = false, false
+       reqsBefore = countRailsReqs()
+       holdReqs = make(chan struct{})
+       wg = getDDConcurrently(5, http.StatusOK, check.Commentf("success after expiry"))
+       close(holdReqs)
+       wg.Wait()
+       c.Check(countRailsReqs(), check.Equals, reqsBefore+1)
 }
 
 func (s *HandlerSuite) TestVocabularyExport(c *check.C) {