Merge branch '19428-webdav-performance'

refs #19428

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

diff --git a/sdk/go/arvados/fs_base.go b/sdk/go/arvados/fs_base.go
index 2ad4d1f859f1141035c04cb4180c5ef623d1fa04..6da639edaf73c3731501246de556f1c7c1932e16 100644 (file)
--- a/sdk/go/arvados/fs_base.go
+++ b/sdk/go/arvados/fs_base.go
@@ -420,10 +420,20 @@ func (n *treenode) Sync() error {
 }
 
 func (n *treenode) MemorySize() (size int64) {
+       // To avoid making other callers wait while we count the
+       // entire filesystem size, we lock the node only long enough
+       // to copy the list of children. We accept that the resulting
+       // size will sometimes be misleading (e.g., we will
+       // double-count an item that moves from A to B after we check
+       // A's size but before we check B's size).
        n.RLock()
-       defer n.RUnlock()
        debugPanicIfNotLocked(n, false)
+       todo := make([]inode, 0, len(n.inodes))
        for _, inode := range n.inodes {
+               todo = append(todo, inode)
+       }
+       n.RUnlock()
+       for _, inode := range todo {
                size += inode.MemorySize()
        }
        return 64 + size

diff --git a/sdk/go/arvados/fs_collection.go b/sdk/go/arvados/fs_collection.go
index 26012e240603d0be43a1019346c4e946e2821790..f09c60a57192a7e654c03a63407045ac302c6d65 100644 (file)
--- a/sdk/go/arvados/fs_collection.go
+++ b/sdk/go/arvados/fs_collection.go
@@ -443,8 +443,6 @@ func (fs *collectionFileSystem) Flush(path string, shortBlocks bool) error {
 }
 
 func (fs *collectionFileSystem) MemorySize() int64 {
-       fs.fileSystem.root.Lock()
-       defer fs.fileSystem.root.Unlock()
        return fs.fileSystem.root.(*dirnode).MemorySize()
 }
 
@@ -576,6 +574,19 @@ func (fn *filenode) FS() FileSystem {
        return fn.fs
 }
 
+func (fn *filenode) MemorySize() (size int64) {
+       fn.RLock()
+       defer fn.RUnlock()
+       size = 64
+       for _, seg := range fn.segments {
+               size += 64
+               if seg, ok := seg.(*memSegment); ok {
+                       size += int64(seg.Len())
+               }
+       }
+       return
+}
+
 // Read reads file data from a single segment, starting at startPtr,
 // into p. startPtr is assumed not to be up-to-date. Caller must have
 // RLock or Lock.
@@ -1150,27 +1161,18 @@ func (dn *dirnode) flush(ctx context.Context, names []string, opts flushOpts) er
        return cg.Wait()
 }
 
-// caller must have write lock.
 func (dn *dirnode) MemorySize() (size int64) {
-       for _, name := range dn.sortedNames() {
-               node := dn.inodes[name]
-               node.Lock()
-               defer node.Unlock()
-               switch node := node.(type) {
-               case *dirnode:
-                       size += node.MemorySize()
-               case *filenode:
-                       size += 64
-                       for _, seg := range node.segments {
-                               switch seg := seg.(type) {
-                               case *memSegment:
-                                       size += int64(seg.Len())
-                               }
-                               size += 64
-                       }
-               }
+       dn.RLock()
+       todo := make([]inode, 0, len(dn.inodes))
+       for _, node := range dn.inodes {
+               todo = append(todo, node)
+       }
+       dn.RUnlock()
+       size = 64
+       for _, node := range todo {
+               size += node.MemorySize()
        }
-       return 64 + size
+       return
 }
 
 // caller must have write lock.

diff --git a/sdk/go/arvados/fs_project.go b/sdk/go/arvados/fs_project.go
index bea1f76e24f24faffa38fbccd7b6b880ffb2d22e..faab6e4f04d36b7caffdc9ce39abf946e9e6c2a4 100644 (file)
--- a/sdk/go/arvados/fs_project.go
+++ b/sdk/go/arvados/fs_project.go
@@ -79,6 +79,7 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
                return nil, err
        }
 
+       pagesize := 100000
        var inodes []inode
 
        // When #17424 is resolved, remove the outer loop here and use
@@ -100,6 +101,7 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
                        Filters: filters,
                        Order:   "uuid",
                        Select:  []string{"uuid", "name", "modified_at", "properties"},
+                       Limit:   &pagesize,
                }
 
                for {

diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index d5fdc4997ecee3c67d1cc8ea4003c2c1e3f6cac4..7ec8639abaa2cf1f99b22dd8d37cbb787475bcfe 100644 (file)
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -274,9 +274,10 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, *cachedSessi
 // until approximate remaining size <= maxsize/2
 func (c *cache) pruneSessions() {
        now := time.Now()
-       var size int64
        keys := c.sessions.Keys()
-       for _, token := range keys {
+       sizes := make([]int64, len(keys))
+       var size int64
+       for i, token := range keys {
                ent, ok := c.sessions.Peek(token)
                if !ok {
                        continue
@@ -287,27 +288,17 @@ func (c *cache) pruneSessions() {
                        continue
                }
                if fs, ok := s.fs.Load().(arvados.CustomFileSystem); ok {
-                       size += fs.MemorySize()
+                       sizes[i] = fs.MemorySize()
+                       size += sizes[i]
                }
        }
        // Remove tokens until reaching size limit, starting with the
        // least frequently used entries (which Keys() returns last).
-       for i := len(keys) - 1; i >= 0; i-- {
-               token := keys[i]
-               if size <= c.cluster.Collections.WebDAVCache.MaxCollectionBytes/2 {
-                       break
-               }
-               ent, ok := c.sessions.Peek(token)
-               if !ok {
-                       continue
-               }
-               s := ent.(*cachedSession)
-               fs, _ := s.fs.Load().(arvados.CustomFileSystem)
-               if fs == nil {
-                       continue
+       for i := len(keys) - 1; i >= 0 && size > c.cluster.Collections.WebDAVCache.MaxCollectionBytes/2; i-- {
+               if sizes[i] > 0 {
+                       c.sessions.Remove(keys[i])
+                       size -= sizes[i]
                }
-               c.sessions.Remove(token)
-               size -= fs.MemorySize()
        }
 }
 
@@ -500,7 +491,7 @@ func (c *cache) GetTokenUser(token string) (*arvados.User, error) {
        c.metrics.apiCalls.Inc()
        var current arvados.User
 
-       err = sess.client.RequestAndDecode(&current, "GET", "/arvados/v1/users/current", nil, nil)
+       err = sess.client.RequestAndDecode(&current, "GET", "arvados/v1/users/current", nil, nil)
        if err != nil {
                return nil, err
        }

diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 3a1d9acde7e7d33208958e467931aef2b1474853..b04add1c494eca08a46c6ea542d5b23448c687e4 100644 (file)
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -486,7 +486,13 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 
        // Check configured permission
        _, sess, err := h.Cache.GetSession(arv.ApiToken)
+       if err != nil {
+               http.Error(w, "session cache: "+err.Error(), http.StatusInternalServerError)
+       }
        tokenUser, err = h.Cache.GetTokenUser(arv.ApiToken)
+       if err != nil {
+               http.Error(w, "user lookup: "+err.Error(), http.StatusInternalServerError)
+       }
 
        if webdavMethod[r.Method] {
                if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) {