12216: Avoid opening all files when generating webdav dir lists.
authorTom Clegg <tclegg@veritasgenetics.com>
Sat, 14 Oct 2017 04:29:38 +0000 (00:29 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Sat, 14 Oct 2017 04:37:04 +0000 (00:37 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

sdk/go/arvados/collection_fs.go
services/keep-web/handler.go
services/keep-web/webdav.go

index 8ecbe9c197de49da13a6a8d4ded90d1cba83e198..1acf27442c3dc514abe1c3b2538549cba9bbcd9f 100644 (file)
@@ -146,7 +146,13 @@ func (e collectionDirent) Sys() interface{} {
        return nil
 }
 
-// collectionFS implements http.FileSystem.
+// A CollectionFileSystem is an http.Filesystem with an added Stat() method.
+type CollectionFileSystem interface {
+       http.FileSystem
+       Stat(name string) (os.FileInfo, error)
+}
+
+// collectionFS implements CollectionFileSystem.
 type collectionFS struct {
        collection *Collection
        client     *Client
@@ -155,8 +161,8 @@ type collectionFS struct {
        sizesOnce  sync.Once
 }
 
-// FileSystem returns an http.FileSystem for the collection.
-func (c *Collection) FileSystem(client *Client, kc keepClient) http.FileSystem {
+// FileSystem returns a CollectionFileSystem for the collection.
+func (c *Collection) FileSystem(client *Client, kc keepClient) CollectionFileSystem {
        return &collectionFS{
                collection: c,
                client:     client,
@@ -164,21 +170,44 @@ func (c *Collection) FileSystem(client *Client, kc keepClient) http.FileSystem {
        }
 }
 
+func (c *collectionFS) Stat(name string) (os.FileInfo, error) {
+       name = canonicalName(name)
+       if name == "." {
+               return collectionDirent{
+                       collection: c.collection,
+                       name:       "/",
+                       isDir:      true,
+               }, nil
+       }
+       if size, ok := c.fileSizes()[name]; ok {
+               return collectionDirent{
+                       collection: c.collection,
+                       name:       path.Base(name),
+                       size:       size,
+                       isDir:      false,
+               }, nil
+       }
+       for fnm := range c.fileSizes() {
+               if !strings.HasPrefix(fnm, name+"/") {
+                       continue
+               }
+               return collectionDirent{
+                       collection: c.collection,
+                       name:       path.Base(name),
+                       isDir:      true,
+               }, nil
+       }
+       return nil, os.ErrNotExist
+}
+
 func (c *collectionFS) Open(name string) (http.File, error) {
        // Ensure name looks the way it does in a manifest.
-       name = path.Clean("/" + name)
-       if name == "/" || name == "./" {
-               name = "."
-       } else if strings.HasPrefix(name, "/") {
-               name = "." + name
-       }
+       name = canonicalName(name)
 
        m := manifest.Manifest{Text: c.collection.ManifestText}
 
-       filesizes := c.fileSizes()
-
        // Return a file if it exists.
-       if size, ok := filesizes[name]; ok {
+       if size, ok := c.fileSizes()[name]; ok {
                reader, err := c.kc.ManifestFileReader(m, name)
                if err != nil {
                        return nil, err
@@ -194,7 +223,7 @@ func (c *collectionFS) Open(name string) (http.File, error) {
        // Return a directory if it's the root dir or there are file
        // entries below it.
        children := map[string]collectionDirent{}
-       for fnm, size := range filesizes {
+       for fnm, size := range c.fileSizes() {
                if !strings.HasPrefix(fnm, name+"/") {
                        continue
                }
@@ -239,3 +268,13 @@ func (c *collectionFS) fileSizes() map[string]int64 {
        })
        return c.sizes
 }
+
+func canonicalName(name string) string {
+       name = path.Clean("/" + name)
+       if name == "/" || name == "./" {
+               name = "."
+       } else if strings.HasPrefix(name, "/") {
+               name = "." + name
+       }
+       return name
+}
index 28dbb66ab1f3704fa555f656995f46ec536da78b..ab04568afebdd3bf789c42a75ebb12988c4a22df 100644 (file)
@@ -357,7 +357,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
        if webdavMethod[r.Method] {
                h := webdav.Handler{
                        Prefix:     "/" + strings.Join(pathParts[:stripParts], "/"),
-                       FileSystem: &webdavFS{httpfs: fs},
+                       FileSystem: &webdavFS{collfs: fs},
                        LockSystem: h.webdavLS,
                        Logger: func(_ *http.Request, err error) {
                                if os.IsNotExist(err) {
index 0a7b7822b20c7f8f5be5cea10eff434481cfe3f8..57f3f53a99ef6a73944c6cb600c1672c67cb6696 100644 (file)
@@ -11,9 +11,12 @@ import (
        prand "math/rand"
        "net/http"
        "os"
+       "sync"
        "sync/atomic"
        "time"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
+
        "golang.org/x/net/context"
        "golang.org/x/net/webdav"
 )
@@ -24,10 +27,10 @@ var (
        errReadOnly           = errors.New("read-only filesystem")
 )
 
-// webdavFS implements a read-only webdav.FileSystem by wrapping
-// http.Filesystem.
+// webdavFS implements a read-only webdav.FileSystem by wrapping an
+// arvados.CollectionFilesystem.
 type webdavFS struct {
-       httpfs http.FileSystem
+       collfs arvados.CollectionFileSystem
 }
 
 var _ webdav.FileSystem = &webdavFS{}
@@ -37,11 +40,11 @@ func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) er
 }
 
 func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (webdav.File, error) {
-       f, err := fs.httpfs.Open(name)
+       fi, err := fs.collfs.Stat(name)
        if err != nil {
                return nil, err
        }
-       return &webdavFile{File: f}, nil
+       return &webdavFile{collfs: fs.collfs, fileInfo: fi, name: name}, nil
 }
 
 func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
@@ -53,23 +56,76 @@ func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
 }
 
 func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {
-       if f, err := fs.httpfs.Open(name); err != nil {
-               return nil, err
-       } else {
-               return f.Stat()
-       }
+       return fs.collfs.Stat(name)
 }
 
 // webdavFile implements a read-only webdav.File by wrapping
-// http.File. Writes fail.
+// http.File.
+//
+// The http.File is opened from an arvados.CollectionFileSystem, but
+// not until Seek, Read, or Readdir is called. This deferred-open
+// strategy makes webdav's OpenFile-Stat-Close cycle fast even though
+// the collfs's Open method is slow. This is relevant because webdav
+// does OpenFile-Stat-Close on each file when preparing directory
+// listings.
+//
+// Writes to a webdavFile always fail.
 type webdavFile struct {
-       http.File
+       // fields populated by (*webdavFS).OpenFile()
+       collfs   http.FileSystem
+       fileInfo os.FileInfo
+       name     string
+
+       // internal fields
+       file     http.File
+       loadOnce sync.Once
+       err      error
+}
+
+func (f *webdavFile) load() {
+       f.file, f.err = f.collfs.Open(f.name)
 }
 
 func (f *webdavFile) Write([]byte) (int, error) {
        return 0, errReadOnly
 }
 
+func (f *webdavFile) Seek(offset int64, whence int) (int64, error) {
+       f.loadOnce.Do(f.load)
+       if f.err != nil {
+               return 0, f.err
+       }
+       return f.file.Seek(offset, whence)
+}
+
+func (f *webdavFile) Read(buf []byte) (int, error) {
+       f.loadOnce.Do(f.load)
+       if f.err != nil {
+               return 0, f.err
+       }
+       return f.file.Read(buf)
+}
+
+func (f *webdavFile) Close() error {
+       if f.file == nil {
+               // We never called load(), or load() failed
+               return f.err
+       }
+       return f.file.Close()
+}
+
+func (f *webdavFile) Readdir(n int) ([]os.FileInfo, error) {
+       f.loadOnce.Do(f.load)
+       if f.err != nil {
+               return nil, f.err
+       }
+       return f.file.Readdir(n)
+}
+
+func (f *webdavFile) Stat() (os.FileInfo, error) {
+       return f.fileInfo, nil
+}
+
 // noLockSystem implements webdav.LockSystem by returning success for
 // every possible locking operation, even though it has no side
 // effects such as actually locking anything. This works for a