storage: keep a certain amount of pack files open by default

The MaxOpenDescriptors option provides a middle ground solution between keeping
all pack files open (as offered by the KeepDescriptors option) and keeping none
open.

By default, 100 pack files can be kept open.

Signed-off-by: Arran Walker <arran.walker@fiveturns.org>

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)
 }

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,16 +137,18 @@ 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
 				continue
 			}
 		}
 
-		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

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()

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)
 	}
 

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)