[ARVADOS] created: a8aec576a4047a27f6ec4227258393d2f7fef59d
Git user
git at public.curoverse.com
Fri Jul 7 16:24:09 EDT 2017
at a8aec576a4047a27f6ec4227258393d2f7fef59d (commit)
commit a8aec576a4047a27f6ec4227258393d2f7fef59d
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Jul 7 16:01:13 2017 -0400
11945: Key collection cache on pdh+token.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curoverse.com>
diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index d72effc..c29ae7d 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -118,7 +118,7 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo
var collection *arvados.Collection
if pdh != "" {
- collection = c.lookupCollection(pdh)
+ collection = c.lookupCollection(arv.ApiToken + "\000" + pdh)
}
if collection != nil && permOK {
@@ -150,7 +150,7 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo
// PDH changed, but now we know we have
// permission -- and maybe we already have the
// new PDH in the cache.
- if coll := c.lookupCollection(current.PortableDataHash); coll != nil {
+ if coll := c.lookupCollection(arv.ApiToken + "\000" + current.PortableDataHash); coll != nil {
return coll, nil
}
}
@@ -170,14 +170,13 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo
expire: exp,
pdh: collection.PortableDataHash,
})
- // Disabled, see #11945
- // c.collections.Add(collection.PortableDataHash, &cachedCollection{
- // expire: exp,
- // collection: collection,
- // })
- // if int64(len(collection.ManifestText)) > c.MaxCollectionBytes/int64(c.MaxCollectionEntries) {
- // go c.pruneCollections()
- // }
+ c.collections.Add(arv.ApiToken+"\000"+collection.PortableDataHash, &cachedCollection{
+ expire: exp,
+ collection: collection,
+ })
+ if int64(len(collection.ManifestText)) > c.MaxCollectionBytes/int64(c.MaxCollectionEntries) {
+ go c.pruneCollections()
+ }
return collection, nil
}
@@ -238,15 +237,13 @@ func (c *cache) collectionBytes() uint64 {
return size
}
-func (c *cache) lookupCollection(pdh string) *arvados.Collection {
- if pdh == "" {
- return nil
- } else if ent, cached := c.collections.Get(pdh); !cached {
+func (c *cache) lookupCollection(key string) *arvados.Collection {
+ if ent, cached := c.collections.Get(key); !cached {
return nil
} else {
ent := ent.(*cachedCollection)
if ent.expire.Before(time.Now()) {
- c.collections.Remove(pdh)
+ c.collections.Remove(key)
return nil
} else {
atomic.AddUint64(&c.stats.CollectionHits, 1)
diff --git a/services/keep-web/cache_test.go b/services/keep-web/cache_test.go
index 0532527..cddeaf4 100644
--- a/services/keep-web/cache_test.go
+++ b/services/keep-web/cache_test.go
@@ -5,14 +5,13 @@
package main
import (
+ "git.curoverse.com/arvados.git/sdk/go/arvados"
"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
"git.curoverse.com/arvados.git/sdk/go/arvadostest"
"gopkg.in/check.v1"
)
func (s *UnitSuite) TestCache(c *check.C) {
- c.Skip("see #11945")
-
arv, err := arvadosclient.MakeArvadosClient()
c.Assert(err, check.Equals, nil)
@@ -22,8 +21,9 @@ func (s *UnitSuite) TestCache(c *check.C) {
// the first req should cause an API call; the next 4 should
// hit all caches.
arv.ApiToken = arvadostest.AdminToken
+ var coll *arvados.Collection
for i := 0; i < 5; i++ {
- coll, err := cache.Get(arv, arvadostest.FooCollection, false)
+ coll, err = cache.Get(arv, arvadostest.FooCollection, false)
c.Check(err, check.Equals, nil)
c.Assert(coll, check.NotNil)
c.Check(coll.PortableDataHash, check.Equals, arvadostest.FooPdh)
@@ -37,18 +37,32 @@ func (s *UnitSuite) TestCache(c *check.C) {
// Hit the same collection 2 more times, this time requesting
// it by PDH and using a different token. The first req should
- // miss the permission cache. Both reqs should hit the
- // Collection cache and skip the API lookup.
+ // miss the permission cache and fetch the new manifest; the
+ // second should hit the Collection cache and skip the API
+ // lookup.
arv.ApiToken = arvadostest.ActiveToken
- for i := 0; i < 2; i++ {
- coll, err := cache.Get(arv, arvadostest.FooPdh, false)
- c.Check(err, check.Equals, nil)
- c.Assert(coll, check.NotNil)
- c.Check(coll.PortableDataHash, check.Equals, arvadostest.FooPdh)
- c.Check(coll.ManifestText[:2], check.Equals, ". ")
- }
+
+ coll2, err := cache.Get(arv, arvadostest.FooPdh, false)
+ c.Check(err, check.Equals, nil)
+ c.Assert(coll2, check.NotNil)
+ c.Check(coll2.PortableDataHash, check.Equals, arvadostest.FooPdh)
+ c.Check(coll2.ManifestText[:2], check.Equals, ". ")
+ c.Check(coll2.ManifestText, check.Not(check.Equals), coll.ManifestText)
+
+ c.Check(cache.Stats().Requests, check.Equals, uint64(5+1))
+ c.Check(cache.Stats().CollectionHits, check.Equals, uint64(4+0))
+ c.Check(cache.Stats().PermissionHits, check.Equals, uint64(4+0))
+ c.Check(cache.Stats().PDHHits, check.Equals, uint64(4+0))
+ c.Check(cache.Stats().APICalls, check.Equals, uint64(1+1))
+
+ coll2, err = cache.Get(arv, arvadostest.FooPdh, false)
+ c.Check(err, check.Equals, nil)
+ c.Assert(coll2, check.NotNil)
+ c.Check(coll2.PortableDataHash, check.Equals, arvadostest.FooPdh)
+ c.Check(coll2.ManifestText[:2], check.Equals, ". ")
+
c.Check(cache.Stats().Requests, check.Equals, uint64(5+2))
- c.Check(cache.Stats().CollectionHits, check.Equals, uint64(4+2))
+ c.Check(cache.Stats().CollectionHits, check.Equals, uint64(4+1))
c.Check(cache.Stats().PermissionHits, check.Equals, uint64(4+1))
c.Check(cache.Stats().PDHHits, check.Equals, uint64(4+0))
c.Check(cache.Stats().APICalls, check.Equals, uint64(1+1))
@@ -67,15 +81,13 @@ func (s *UnitSuite) TestCache(c *check.C) {
c.Check(err, check.Equals, nil)
}
c.Check(cache.Stats().Requests, check.Equals, uint64(5+2+20))
- c.Check(cache.Stats().CollectionHits, check.Equals, uint64(4+2+18))
+ c.Check(cache.Stats().CollectionHits, check.Equals, uint64(4+1+18))
c.Check(cache.Stats().PermissionHits, check.Equals, uint64(4+1+18))
c.Check(cache.Stats().PDHHits, check.Equals, uint64(4+0+18))
c.Check(cache.Stats().APICalls, check.Equals, uint64(1+1+2))
}
func (s *UnitSuite) TestCacheForceReloadByPDH(c *check.C) {
- c.Skip("see #11945")
-
arv, err := arvadosclient.MakeArvadosClient()
c.Assert(err, check.Equals, nil)
@@ -94,8 +106,6 @@ func (s *UnitSuite) TestCacheForceReloadByPDH(c *check.C) {
}
func (s *UnitSuite) TestCacheForceReloadByUUID(c *check.C) {
- c.Skip("see #11945")
-
arv, err := arvadosclient.MakeArvadosClient()
c.Assert(err, check.Equals, nil)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list