Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
60 changes: 47 additions & 13 deletions storage/filesystem/dotgit/dotgit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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
Expand Down
40 changes: 40 additions & 0 deletions storage/filesystem/dotgit/dotgit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion storage/filesystem/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
8 changes: 6 additions & 2 deletions storage/filesystem/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)

Expand Down