Skip to content

Commit d2c0160

Browse files
fix(disk.Store): make File.Size() work after Close() is called
While Kraken code itself never calls Size() after Close() is called, we use a library to expose the Docker Registry V2 API and we sometimes pass disk.File to that library, after which it may call Size() after Close(). This breaks the whole image push flow, as PATCH requests when uploading a blob need to know how far they've gotten with pushing, so they call Size(), after which they pass the resulting offset to the next PATCH request, which uses it to know which bytes to send to the server. Currently, Size() returns 0 if Close() is called beforehand, even if 0 is not the right offset, causing errors for the push flow.
1 parent 90bd4bc commit d2c0160

2 files changed

Lines changed: 19 additions & 6 deletions

File tree

lib/store/disk/file.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,28 @@ package disk
33
import (
44
"os"
55

6+
"github.com/docker/distribution/registry/storage/driver"
67
storelib "github.com/uber/kraken/lib/store"
8+
"github.com/uber/kraken/utils/log"
79
)
810

9-
func newFile(f *os.File) *File {
11+
func newFile(f *os.File, path string) *File {
1012
return &File{
11-
fd: f,
13+
fd: f,
14+
path: path,
1215
}
1316
}
1417

15-
var _ storelib.FileReadWriter = &File{}
18+
var (
19+
_ storelib.FileReadWriter = &File{}
20+
_ driver.FileWriter = &File{}
21+
)
1622

1723
// File represends an open file descriptor to a blob in [Store].
1824
type File struct {
1925
fd *os.File
26+
// May become stale after a while, but that's ok, as clients don't call Size() after MarkComplete/RenameKey.
27+
path string
2028
}
2129

2230
func (f *File) Read(p []byte) (n int, err error) { return f.fd.Read(p) }
@@ -28,8 +36,13 @@ func (f *File) Close() error { return f.fd.Clo
2836

2937
// Size returns the number of bytes the file contains.
3038
func (f *File) Size() int64 {
31-
info, err := f.fd.Stat()
39+
// Size is sometimes called after Close() is called, meaning that fd.Stat() will return ErrClosed.
40+
info, err := os.Stat(f.path)
3241
if err != nil {
42+
log.Default().With(
43+
"path", f.path,
44+
"error", err,
45+
).Error("disk.File Stat failed as os.Stat failed")
3346
return 0
3447
}
3548
return info.Size()

lib/store/disk/store.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func (s *store) Open(key string, scope storelib.BlobScope) (*File, error) {
120120
if err != nil {
121121
return nil, fmt.Errorf("open: %w", err)
122122
}
123-
return newFile(f), nil
123+
return newFile(f, path), nil
124124
}
125125

126126
func (s *store) Has(key string, scope storelib.BlobScope) (inStore bool, inScope bool) {
@@ -195,7 +195,7 @@ func (s *store) Create(key string, size uint64) (*File, error) {
195195
}
196196

197197
s.emitUsageMetrics()
198-
return newFile(f), nil
198+
return newFile(f, blobPath), nil
199199
}
200200

201201
func (s *store) persistBlobSize(key string, size uint64) error {

0 commit comments

Comments
 (0)