diff --git a/weed/server/webdav_listed_entries.go b/weed/server/webdav_listed_entries.go new file mode 100644 index 000000000..84ac1ab69 --- /dev/null +++ b/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()))) +} diff --git a/weed/server/webdav_listed_entries_test.go b/weed/server/webdav_listed_entries_test.go new file mode 100644 index 000000000..0e1e03603 --- /dev/null +++ b/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") + } +} diff --git a/weed/server/webdav_server.go b/weed/server/webdav_server.go index 52b52859d..f9c818353 100644 --- a/weed/server/webdav_server.go +++ b/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() }