diff --git a/repository.go b/repository.go index e5b12b0c5..32e6b366c 100644 --- a/repository.go +++ b/repository.go @@ -224,7 +224,9 @@ func PlainInit(path string, isBare bool) (*Repository, error) { dot, _ = wt.Chroot(GitDirName) } - s := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) + s := filesystem.NewStorageWithOptions(dot, cache.NewObjectLRUDefault(), filesystem.Options{ + MaxOpenDescriptors: 100, + }) return Init(s, wt) } @@ -252,7 +254,9 @@ func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) return nil, err } - s := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) + s := filesystem.NewStorageWithOptions(dot, cache.NewObjectLRUDefault(), filesystem.Options{ + MaxOpenDescriptors: 100, + }) return Open(s, wt) } diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index ba9667e65..33c8d9880 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -66,6 +66,9 @@ type Options struct { // KeepDescriptors makes the file descriptors to be reused but they will // need to be manually closed calling Close(). KeepDescriptors bool + // MaxOpenDescriptors is the max number of file descriptors to keep + // open. If KeepDescriptors is true, all file descriptors will remain open. + MaxOpenDescriptors int } // The DotGit type represents a local git repository on disk. This @@ -83,7 +86,9 @@ type DotGit struct { packList []plumbing.Hash packMap map[plumbing.Hash]struct{} - files map[string]billy.File + fileList []plumbing.Hash + fileListIdx int + fileMap map[plumbing.Hash]billy.File } // New returns a DotGit value ready to be used. The path argument must @@ -132,8 +137,8 @@ func (d *DotGit) Initialize() error { // Close closes all opened files. func (d *DotGit) Close() error { var firstError error - if d.files != nil { - for _, f := range d.files { + if d.fileMap != nil { + for _, f := range d.fileMap { err := f.Close() if err != nil && firstError == nil { firstError = err @@ -141,7 +146,9 @@ func (d *DotGit) Close() error { } } - d.files = nil + d.fileMap = nil + d.fileList = nil + d.fileListIdx = 0 } if firstError != nil { @@ -245,8 +252,20 @@ func (d *DotGit) objectPackPath(hash plumbing.Hash, extension string) string { } func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.File, error) { - if d.files == nil { - d.files = make(map[string]billy.File) + if d.fileMap == nil { + if d.options.MaxOpenDescriptors > 0 { + d.fileList = make([]plumbing.Hash, d.options.MaxOpenDescriptors) + d.fileMap = make(map[plumbing.Hash]billy.File, d.options.MaxOpenDescriptors) + } else { + d.fileMap = make(map[plumbing.Hash]billy.File) + } + } + + if extension == "pack" { + f, ok := d.fileMap[hash] + if ok { + return f, nil + } } err := d.hasPack(hash) @@ -255,11 +274,6 @@ func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.Fil } path := d.objectPackPath(hash, extension) - f, ok := d.files[path] - if ok { - return f, nil - } - pack, err := d.fs.Open(path) if err != nil { if os.IsNotExist(err) { @@ -269,8 +283,28 @@ func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.Fil return nil, err } - if d.options.KeepDescriptors && extension == "pack" { - d.files[path] = pack + if extension == "pack" { + if d.options.KeepDescriptors { + d.fileMap[hash] = pack + } else if d.options.MaxOpenDescriptors > 0 { + if next := d.fileList[d.fileListIdx]; !next.IsZero() { + open := d.fileMap[next] + delete(d.fileMap, next) + if open != nil { + if err = open.Close(); err != nil { + return nil, err + } + } + } + + d.fileList[d.fileListIdx] = hash + d.fileMap[hash] = pack + + d.fileListIdx++ + if d.fileListIdx >= len(d.fileList) { + d.fileListIdx = 0 + } + } } return pack, nil diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index 73b0291a7..c15f2e9a9 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -519,6 +519,46 @@ func (s *SuiteDotGit) TestObjectPackWithKeepDescriptors(c *C) { } +func (s *SuiteDotGit) TestObjectPackWithMaxOpenDescriptors(c *C) { + f := fixtures.ByTag("multi-packfile").One() + fs := f.DotGit() + dir := NewWithOptions(fs, Options{MaxOpenDescriptors: 1}) + + hashes, err := dir.ObjectPacks() + c.Assert(hashes, HasLen, 2) + + pack, err := dir.ObjectPack(hashes[0]) + c.Assert(err, IsNil) + c.Assert(filepath.Ext(pack.Name()), Equals, ".pack") + + // Move to an specific offset + pack.Seek(42, os.SEEK_SET) + + pack, err = dir.ObjectPack(hashes[0]) + c.Assert(err, IsNil) + + // If the file is the same the offset should be the same + offset, err := pack.Seek(0, os.SEEK_CUR) + c.Assert(err, IsNil) + c.Assert(offset, Equals, int64(42)) + + // Open second pack file, this should close the first + pack2, err := dir.ObjectPack(hashes[1]) + c.Assert(err, IsNil) + c.Assert(filepath.Ext(pack.Name()), Equals, ".pack") + + // Move second pack file to a specific offset + _, err = pack2.Seek(42, os.SEEK_SET) + c.Assert(err, IsNil) + + // Seeking in first pack file (now closed) should error + _, err = pack.Seek(42, os.SEEK_SET) + c.Assert(err, NotNil) + + err = dir.Close() + c.Assert(err, IsNil) +} + func (s *SuiteDotGit) TestObjectPackIdx(c *C) { f := fixtures.Basic().ByTag(".git").One() fs := f.DotGit() diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 3eb62a22f..0d962179e 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -366,7 +366,7 @@ func (s *ObjectStorage) getFromPackfile(h plumbing.Hash, canBeDelta bool) ( return nil, err } - if !s.options.KeepDescriptors { + if !s.options.KeepDescriptors && s.options.MaxOpenDescriptors == 0 { defer ioutil.CheckClose(f, &err) } diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index 370f7bd34..b7f665ecb 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -31,6 +31,9 @@ type Options struct { // KeepDescriptors makes the file descriptors to be reused but they will // need to be manually closed calling Close(). KeepDescriptors bool + // MaxOpenDescriptors is the max number of file descriptors to keep + // open. If KeepDescriptors is true, all file descriptors will remain open. + MaxOpenDescriptors int } // NewStorage returns a new Storage backed by a given `fs.Filesystem` and cache. @@ -42,8 +45,9 @@ func NewStorage(fs billy.Filesystem, cache cache.Object) *Storage { // backed by a given `fs.Filesystem` and cache. func NewStorageWithOptions(fs billy.Filesystem, cache cache.Object, ops Options) *Storage { dirOps := dotgit.Options{ - ExclusiveAccess: ops.ExclusiveAccess, - KeepDescriptors: ops.KeepDescriptors, + ExclusiveAccess: ops.ExclusiveAccess, + KeepDescriptors: ops.KeepDescriptors, + MaxOpenDescriptors: ops.MaxOpenDescriptors, } dir := dotgit.NewWithOptions(fs, dirOps)