Skip to content

Commit 28e2d4d

Browse files
committed
gateway: cache local mounts from fs operations
Previously, each call to `ReadFile`/`ReadDir`/`StatFile` would trigger an entire `Mount`+`Unmount` operation. This could result in terrible performance for repeated filesystem accesses - potentially worse that one `Mount`+`Unmount` per file, if the file is read in ranges. To resolve this, we now directly store the `localMounter` in the gateway, and rely on the fact that repeated calls to it return the same value. These mounts will only be removed when the gateway itself is released. Signed-off-by: Justin Chadwell <[email protected]>
1 parent 0233532 commit 28e2d4d

File tree

3 files changed

+165
-118
lines changed

3 files changed

+165
-118
lines changed

cache/util/fsutil.go

Lines changed: 56 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"path/filepath"
88

99
"github.com/containerd/continuity/fs"
10-
"github.com/moby/buildkit/snapshot"
1110
"github.com/pkg/errors"
1211
"github.com/tonistiigi/fsutil"
1312
fstypes "github.com/tonistiigi/fsutil/types"
@@ -23,121 +22,89 @@ type FileRange struct {
2322
Length int
2423
}
2524

26-
func withMount(mount snapshot.Mountable, cb func(string) error) error {
27-
lm := snapshot.LocalMounter(mount)
28-
29-
root, err := lm.Mount()
25+
func ReadFile(ctx context.Context, root string, req ReadRequest) ([]byte, error) {
26+
fp, err := fs.RootPath(root, req.Filename)
3027
if err != nil {
31-
return err
28+
return nil, errors.WithStack(err)
3229
}
3330

34-
defer func() {
35-
if lm != nil {
36-
lm.Unmount()
31+
f, err := os.Open(fp)
32+
if err != nil {
33+
// The filename here is internal to the mount, so we can restore
34+
// the request base path for error reporting.
35+
// See os.DirFS.Open for details.
36+
pe := &os.PathError{}
37+
if errors.As(err, &pe) {
38+
pe.Path = req.Filename
3739
}
38-
}()
39-
40-
if err := cb(root); err != nil {
41-
return err
40+
return nil, errors.WithStack(err)
4241
}
42+
defer f.Close()
4343

44-
if err := lm.Unmount(); err != nil {
45-
return err
44+
var rdr io.Reader = f
45+
if req.Range != nil {
46+
rdr = io.NewSectionReader(f, int64(req.Range.Offset), int64(req.Range.Length))
4647
}
47-
lm = nil
48-
return nil
49-
}
50-
51-
func ReadFile(ctx context.Context, mount snapshot.Mountable, req ReadRequest) ([]byte, error) {
52-
var dt []byte
53-
54-
err := withMount(mount, func(root string) error {
55-
fp, err := fs.RootPath(root, req.Filename)
56-
if err != nil {
57-
return errors.WithStack(err)
58-
}
59-
60-
f, err := os.Open(fp)
61-
if err != nil {
62-
// The filename here is internal to the mount, so we can restore
63-
// the request base path for error reporting.
64-
// See os.DirFS.Open for details.
65-
pe := &os.PathError{}
66-
if errors.As(err, &pe) {
67-
pe.Path = req.Filename
68-
}
69-
return errors.WithStack(err)
70-
}
71-
defer f.Close()
72-
73-
var rdr io.Reader = f
74-
if req.Range != nil {
75-
rdr = io.NewSectionReader(f, int64(req.Range.Offset), int64(req.Range.Length))
76-
}
77-
dt, err = io.ReadAll(rdr)
78-
if err != nil {
79-
return errors.WithStack(err)
80-
}
81-
return nil
82-
})
83-
return dt, err
48+
dt, err := io.ReadAll(rdr)
49+
if err != nil {
50+
return nil, errors.WithStack(err)
51+
}
52+
return dt, nil
8453
}
8554

8655
type ReadDirRequest struct {
8756
Path string
8857
IncludePattern string
8958
}
9059

91-
func ReadDir(ctx context.Context, mount snapshot.Mountable, req ReadDirRequest) ([]*fstypes.Stat, error) {
60+
func ReadDir(ctx context.Context, root string, req ReadDirRequest) ([]*fstypes.Stat, error) {
9261
var (
9362
rd []*fstypes.Stat
9463
fo fsutil.FilterOpt
9564
)
9665
if req.IncludePattern != "" {
9766
fo.IncludePatterns = append(fo.IncludePatterns, req.IncludePattern)
9867
}
99-
err := withMount(mount, func(root string) error {
100-
fp, err := fs.RootPath(root, req.Path)
68+
fp, err := fs.RootPath(root, req.Path)
69+
if err != nil {
70+
return nil, errors.WithStack(err)
71+
}
72+
err = fsutil.Walk(ctx, fp, &fo, func(path string, info os.FileInfo, err error) error {
10173
if err != nil {
102-
return errors.WithStack(err)
74+
return errors.Wrapf(err, "walking %q", root)
10375
}
104-
return fsutil.Walk(ctx, fp, &fo, func(path string, info os.FileInfo, err error) error {
105-
if err != nil {
106-
return errors.Wrapf(err, "walking %q", root)
107-
}
108-
stat, ok := info.Sys().(*fstypes.Stat)
109-
if !ok {
110-
// This "can't happen(tm)".
111-
return errors.Errorf("expected a *fsutil.Stat but got %T", info.Sys())
112-
}
113-
rd = append(rd, stat)
114-
115-
if info.IsDir() {
116-
return filepath.SkipDir
117-
}
118-
return nil
119-
})
120-
})
121-
return rd, err
122-
}
123-
124-
func StatFile(ctx context.Context, mount snapshot.Mountable, path string) (*fstypes.Stat, error) {
125-
var st *fstypes.Stat
126-
err := withMount(mount, func(root string) error {
127-
fp, err := fs.RootPath(root, path)
128-
if err != nil {
129-
return errors.WithStack(err)
76+
stat, ok := info.Sys().(*fstypes.Stat)
77+
if !ok {
78+
// This "can't happen(tm)".
79+
return errors.Errorf("expected a *fsutil.Stat but got %T", info.Sys())
13080
}
131-
if st, err = fsutil.Stat(fp); err != nil {
132-
// The filename here is internal to the mount, so we can restore
133-
// the request base path for error reporting.
134-
// See os.DirFS.Open for details.
135-
replaceErrorPath(err, path)
136-
return errors.WithStack(err)
81+
rd = append(rd, stat)
82+
83+
if info.IsDir() {
84+
return filepath.SkipDir
13785
}
13886
return nil
13987
})
140-
return st, err
88+
if err != nil {
89+
return nil, err
90+
}
91+
return rd, nil
92+
}
93+
94+
func StatFile(ctx context.Context, root string, path string) (*fstypes.Stat, error) {
95+
fp, err := fs.RootPath(root, path)
96+
if err != nil {
97+
return nil, errors.WithStack(err)
98+
}
99+
st, err := fsutil.Stat(fp)
100+
if err != nil {
101+
// The filename here is internal to the mount, so we can restore
102+
// the request base path for error reporting.
103+
// See os.DirFS.Open for details.
104+
replaceErrorPath(err, path)
105+
return nil, errors.WithStack(err)
106+
}
107+
return st, nil
141108
}
142109

143110
// replaceErrorPath will override the path in an os.PathError in the error chain.

frontend/gateway/forwarder/forward.go

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,14 @@ func (r *ref) ReadFile(ctx context.Context, req client.ReadRequest) ([]byte, err
358358
Length: r.Length,
359359
}
360360
}
361-
return cacheutil.ReadFile(ctx, m, newReq)
361+
362+
var dt []byte
363+
err = withMount(m, func(root string) error {
364+
var err error
365+
dt, err = cacheutil.ReadFile(ctx, root, newReq)
366+
return err
367+
})
368+
return dt, err
362369
}
363370

364371
func (r *ref) ReadDir(ctx context.Context, req client.ReadDirRequest) ([]*fstypes.Stat, error) {
@@ -370,15 +377,29 @@ func (r *ref) ReadDir(ctx context.Context, req client.ReadDirRequest) ([]*fstype
370377
Path: req.Path,
371378
IncludePattern: req.IncludePattern,
372379
}
373-
return cacheutil.ReadDir(ctx, m, newReq)
380+
381+
var rd []*fstypes.Stat
382+
err = withMount(m, func(root string) error {
383+
var err error
384+
rd, err = cacheutil.ReadDir(ctx, root, newReq)
385+
return err
386+
})
387+
return rd, err
374388
}
375389

376390
func (r *ref) StatFile(ctx context.Context, req client.StatRequest) (*fstypes.Stat, error) {
377391
m, err := r.getMountable(ctx)
378392
if err != nil {
379393
return nil, err
380394
}
381-
return cacheutil.StatFile(ctx, m, req.Path)
395+
396+
var st *fstypes.Stat
397+
err = withMount(m, func(root string) error {
398+
var err error
399+
st, err = cacheutil.StatFile(ctx, root, req.Path)
400+
return err
401+
})
402+
return st, err
382403
}
383404

384405
func (r *ref) getMountable(ctx context.Context) (snapshot.Mountable, error) {
@@ -392,3 +413,28 @@ func (r *ref) getMountable(ctx context.Context) (snapshot.Mountable, error) {
392413
}
393414
return ref.ImmutableRef.Mount(ctx, true, r.session)
394415
}
416+
417+
func withMount(mount snapshot.Mountable, cb func(string) error) error {
418+
lm := snapshot.LocalMounter(mount)
419+
420+
root, err := lm.Mount()
421+
if err != nil {
422+
return err
423+
}
424+
425+
defer func() {
426+
if lm != nil {
427+
lm.Unmount()
428+
}
429+
}()
430+
431+
if err := cb(root); err != nil {
432+
return err
433+
}
434+
435+
if err := lm.Unmount(); err != nil {
436+
return err
437+
}
438+
lm = nil
439+
return nil
440+
}

0 commit comments

Comments
 (0)