webdav: answer PROPFIND child stats from the listing (#10492)

* webdav: answer PROPFIND child stats from the listing

golang.org/x/net/webdav discards the FileInfo that Readdir returned and stats
every child again, five times over, so a PROPFIND on a directory costs five
sequential filer lookups per entry: 15006 lookups and 1.4s for 3000
subdirectories, 24s for 60000. Windows Explorer times out well before that.

Hold the entries a listing already fetched for the lifetime of the request and
serve those stats from them - 6 lookups and 0.03s for the same 3000 entries.
WebDavFile.Stat has to stop dropping its request context for the held entries
to be reachable.

* webdav: keep the request context on the lookups a listing drives

stat, Readdir and Seek reached the filer on context.Background(), so a client
that walked away left the listing streaming and the lookups running. Seek also
missed the entries the listing had already fetched.

Write and cleanup paths keep their own context - a cancelled request must not
abandon a flush half done.

weed/server/webdav_listed_entries.go

@@ -0,0 +1,72 @@
+package weed_server
+
+import (
+	"context"
+	"net/http"
+	"strings"
+	"sync"
+)
+
+// A PROPFIND on a directory lists it once, then golang.org/x/net/webdav's walkFS
+// throws the listing's FileInfo away and stats every child again - one filer
+// lookup per entry, serially. listedEntries hands those stats back the entries the
+// listing already fetched, for the lifetime of a single request.
+type listedEntries struct {
+	mu     sync.Mutex
+	byPath map[string]*FileInfo
+}
+
+// maxListedEntries bounds the per-request memory. Past it, children fall back to
+// individual lookups.
+const maxListedEntries = 100000
+
+type listedEntriesKey struct{}
+
+func withListedEntries(ctx context.Context) context.Context {
+	return context.WithValue(ctx, listedEntriesKey{}, &listedEntries{byPath: make(map[string]*FileInfo)})
+}
+
+func listedEntriesFrom(ctx context.Context) *listedEntries {
+	if ctx == nil {
+		return nil
+	}
+	listed, _ := ctx.Value(listedEntriesKey{}).(*listedEntries)
+	return listed
+}
+
+// trailing slashes are optional on directory paths, so both sides normalize
+func listedEntryKey(fullPath string) string {
+	if len(fullPath) > 1 {
+		return strings.TrimSuffix(fullPath, "/")
+	}
+	return fullPath
+}
+
+func (listed *listedEntries) put(fullPath string, fi *FileInfo) {
+	if listed == nil {
+		return
+	}
+	listed.mu.Lock()
+	defer listed.mu.Unlock()
+	if len(listed.byPath) >= maxListedEntries {
+		return
+	}
+	listed.byPath[listedEntryKey(fullPath)] = fi
+}
+
+func (listed *listedEntries) get(fullPath string) *FileInfo {
+	if listed == nil {
+		return nil
+	}
+	listed.mu.Lock()
+	defer listed.mu.Unlock()
+	return listed.byPath[listedEntryKey(fullPath)]
+}
+
+type listedEntriesHandler struct {
+	next http.Handler
+}
+
+func (h listedEntriesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	h.next.ServeHTTP(w, r.WithContext(withListedEntries(r.Context())))
+}

weed/server/webdav_listed_entries_test.go

@@ -0,0 +1,69 @@
+package weed_server
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestListedEntriesTrailingSlash(t *testing.T) {
+	ctx := withListedEntries(context.Background())
+	listed := listedEntriesFrom(ctx)
+
+	listed.put("/dir/child", &FileInfo{name: "/dir/child", isDirectory: true})
+
+	if listed.get("/dir/child") == nil {
+		t.Error("exact path missed")
+	}
+	if listed.get("/dir/child/") == nil {
+		t.Error("trailing slash missed: OpenFile stats directories as dir/")
+	}
+	if listed.get("/dir/other") != nil {
+		t.Error("unlisted path hit")
+	}
+}
+
+func TestListedEntriesWithoutCache(t *testing.T) {
+	var missing *listedEntries
+
+	missing.put("/dir/child", &FileInfo{name: "/dir/child"})
+	if missing.get("/dir/child") != nil {
+		t.Error("nil cache returned an entry")
+	}
+	if listedEntriesFrom(context.Background()) != nil {
+		t.Error("plain context carried a cache")
+	}
+	if listedEntriesFrom(nil) != nil {
+		t.Error("nil context carried a cache")
+	}
+}
+
+func TestListedEntriesCap(t *testing.T) {
+	listed := &listedEntries{byPath: make(map[string]*FileInfo)}
+	for i := 0; i < maxListedEntries+10; i++ {
+		listed.put(string(rune('a'+i%26))+string(rune(i)), &FileInfo{})
+	}
+	if len(listed.byPath) > maxListedEntries {
+		t.Errorf("cache grew to %d, past the %d cap", len(listed.byPath), maxListedEntries)
+	}
+}
+
+func TestListedEntriesPerRequest(t *testing.T) {
+	var caches []*listedEntries
+
+	handler := listedEntriesHandler{next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		caches = append(caches, listedEntriesFrom(r.Context()))
+	})}
+
+	for i := 0; i < 2; i++ {
+		handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("PROPFIND", "/dir/", nil))
+	}
+
+	if caches[0] == nil || caches[1] == nil {
+		t.Fatal("request context carried no cache")
+	}
+	if caches[0] == caches[1] {
+		t.Error("two requests shared one cache")
+	}
+}

weed/server/webdav_server.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"path"
 	"strings"
@@ -45,7 +46,7 @@ type WebDavOption struct {
 type WebDavServer struct {
 	option         *WebDavOption
 	grpcDialOption grpc.DialOption
-	Handler        *webdav.Handler
+	Handler        http.Handler
 }
 
 func max(x, y int64) int64 {
@@ -71,9 +72,11 @@ func NewWebDavServer(option *WebDavOption) (ws *WebDavServer, err error) {
 	ws = &WebDavServer{
 		option:         option,
 		grpcDialOption: security.LoadClientTLS(util.GetViper(), "grpc.filer"),
-		Handler: &webdav.Handler{
-			FileSystem: fs,
-			LockSystem: webdav.NewMemLS(),
+		Handler: listedEntriesHandler{
+			next: &webdav.Handler{
+				FileSystem: fs,
+				LockSystem: webdav.NewMemLS(),
+			},
 		},
 	}
 
@@ -370,30 +373,43 @@ func (fs *WebDavFileSystem) stat(ctx context.Context, fullFilePath string) (os.F
 
 	fullpath := util.FullPath(fullFilePath)
 
-	var fi FileInfo
-	entry, _, _, err := filer_pb.GetEntry(context.Background(), fs, fullpath)
+	if listedFi := listedEntriesFrom(ctx).get(string(fullpath)); listedFi != nil {
+		// the caller's spelling of the path, trailing slash and all, is what a
+		// lookup would have reported back
+		fi := *listedFi
+		fi.name = string(fullpath)
+		return &fi, nil
+	}
+
+	entry, _, _, err := filer_pb.GetEntry(ctx, fs, fullpath)
 	if err != nil {
 		if err == filer_pb.ErrNotFound {
 			return nil, os.ErrNotExist
 		}
-		fi.err = err
-		return &fi, nil
+		return &FileInfo{err: err}, nil
 	}
 	if entry == nil {
 		return nil, os.ErrNotExist
 	}
-	fi.size = int64(filer.FileSize(entry))
-	fi.name = string(fullpath)
-	fi.mode = os.FileMode(entry.Attributes.FileMode)
-	fi.modifiedTime = time.Unix(entry.Attributes.Mtime, 0)
-	fi.etag = filer.ETag(entry)
-	fi.isDirectory = entry.IsDirectory
+
+	return toFileInfo(fullpath, entry), nil
+}
+
+func toFileInfo(fullpath util.FullPath, entry *filer_pb.Entry) *FileInfo {
+	fi := &FileInfo{
+		size:         int64(filer.FileSize(entry)),
+		name:         string(fullpath),
+		mode:         os.FileMode(entry.Attributes.FileMode),
+		modifiedTime: time.Unix(entry.Attributes.Mtime, 0),
+		etag:         filer.ETag(entry),
+		isDirectory:  entry.IsDirectory,
+	}
 
 	if fi.name == "/" {
 		fi.modifiedTime = time.Now()
 		fi.isDirectory = true
 	}
-	return &fi, nil
+	return fi
 }
 
 func (fs *WebDavFileSystem) Stat(ctx context.Context, name string) (os.FileInfo, error) {
@@ -566,7 +582,10 @@ func (f *WebDavFile) Readdir(count int) (ret []os.FileInfo, err error) {
 
 	dir, _ := util.FullPath(f.name).DirAndName()
 
-	err = filer_pb.ReadDirAllEntries(context.Background(), f.fs, util.FullPath(dir), "", func(entry *filer_pb.Entry, isLast bool) error {
+	ctx := f.requestContext()
+	listed := listedEntriesFrom(ctx)
+
+	err = filer_pb.ReadDirAllEntries(ctx, f.fs, util.FullPath(dir), "", func(entry *filer_pb.Entry, isLast bool) error {
 		fi := FileInfo{
 			size:         int64(filer.FileSize(entry)),
 			name:         entry.Name,
@@ -578,7 +597,12 @@ func (f *WebDavFile) Readdir(count int) (ret []os.FileInfo, err error) {
 		if !strings.HasSuffix(fi.name, "/") && fi.IsDir() {
 			fi.name += "/"
 		}
+
 		glog.V(4).Infof("entry: %v", fi.name)
+
+		childPath := util.NewFullPath(dir, entry.Name)
+		listed.put(string(childPath), toFileInfo(childPath, entry))
+
 		ret = append(ret, &fi)
 		return nil
 	})
@@ -610,7 +634,7 @@ func (f *WebDavFile) Seek(offset int64, whence int) (int64, error) {
 
 	glog.V(2).Infof("WebDavFile.Seek %v %v %v", f.name, offset, whence)
 
-	ctx := context.Background()
+	ctx := f.requestContext()
 
 	var err error
 	switch whence {
@@ -631,7 +655,14 @@ func (f *WebDavFile) Stat() (os.FileInfo, error) {
 
 	glog.V(2).Infof("WebDavFile.Stat %v", f.name)
 
-	ctx := context.Background()
+	return f.fs.stat(f.requestContext(), f.name)
+}
 
-	return f.fs.stat(ctx, f.name)
+// the context of the request that opened the file, or a bare one for files
+// opened outside a request
+func (f *WebDavFile) requestContext() context.Context {
+	if f.ctx != nil {
+		return f.ctx
+	}
+	return context.Background()
 }