Skip to content

Commit 3228b18

Browse files
committed
Give frozen files their own filesystem.FileReader type
We currently have Close() and ReadAt() operations on the file itself. This is fairly confusing, considering that those should only be invoked when the file is frozen.
1 parent 99cd358 commit 3228b18

File tree

1 file changed

+56
-40
lines changed

1 file changed

+56
-40
lines changed

pkg/filesystem/virtual/pool_backed_file_allocator.go

Lines changed: 56 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -128,29 +128,15 @@ func (f *fileBackedFile) lockMutatingData() {
128128
}
129129
}
130130

131-
func (f *fileBackedFile) acquireFrozenDescriptorLocked() (success bool) {
131+
func (f *fileBackedFile) acquireFrozenDescriptorLocked() (filesystem.FileReader, bool) {
132132
if f.referenceCount == 0 {
133-
return false
133+
return nil, false
134134
}
135135
f.referenceCount++
136136
f.frozenDescriptorsCount++
137-
return true
138-
}
139-
140-
func (f *fileBackedFile) releaseFrozenDescriptor() {
141-
f.lock.Lock()
142-
defer f.lock.Unlock()
143-
144-
if f.frozenDescriptorsCount == 0 {
145-
panic("Invalid open frozen descriptor count")
146-
}
147-
f.frozenDescriptorsCount--
148-
if f.frozenDescriptorsCount == 0 && f.unfreezeWakeup != nil {
149-
close(f.unfreezeWakeup)
150-
f.unfreezeWakeup = nil
151-
}
152-
153-
f.releaseReferencesLocked(1)
137+
return &frozenFileBackedFile{
138+
file: f,
139+
}, true
154140
}
155141

156142
func (f *fileBackedFile) acquireShareAccessLocked(shareAccess ShareMask) {
@@ -199,15 +185,15 @@ func (f *fileBackedFile) getCachedDigest() digest.Digest {
199185
// a cached value, or computes the digest and caches it. It is only safe
200186
// to call this function while the file is frozen (i.e., calling
201187
// f.acquireFrozenDescriptorLocked()).
202-
func (f *fileBackedFile) updateCachedDigest(digestFunction digest.Function) (digest.Digest, error) {
188+
func (f *fileBackedFile) updateCachedDigest(digestFunction digest.Function, frozenFile filesystem.FileReader) (digest.Digest, error) {
203189
// Check whether the cached digest we have is still valid.
204190
if cachedDigest := f.getCachedDigest(); cachedDigest != digest.BadDigest && cachedDigest.UsesDigestFunction(digestFunction) {
205191
return cachedDigest, nil
206192
}
207193

208194
// If not, compute a new digest.
209195
digestGenerator := digestFunction.NewGenerator(math.MaxInt64)
210-
if _, err := io.Copy(digestGenerator, io.NewSectionReader(f, 0, math.MaxInt64)); err != nil {
196+
if _, err := io.Copy(digestGenerator, io.NewSectionReader(frozenFile, 0, math.MaxInt64)); err != nil {
211197
return digest.BadDigest, util.StatusWrapWithCode(err, codes.Internal, "Failed to compute file digest")
212198
}
213199
newDigest := digestGenerator.Sum()
@@ -250,22 +236,22 @@ func (f *fileBackedFile) uploadFile(ctx context.Context, contentAddressableStora
250236
poolBackedFileAllocatorWritableFileUploadDelaySeconds.Observe(time.Now().Sub(start).Seconds())
251237
}
252238
}
253-
success := f.acquireFrozenDescriptorLocked()
239+
frozenFile, success := f.acquireFrozenDescriptorLocked()
254240
f.lock.Unlock()
255241
if !success {
256242
return digest.BadDigest, status.Error(codes.NotFound, "File was unlinked before uploading could start")
257243
}
258244

259-
blobDigest, err := f.updateCachedDigest(digestFunction)
245+
blobDigest, err := f.updateCachedDigest(digestFunction, frozenFile)
260246
if err != nil {
261-
f.Close()
247+
frozenFile.Close()
262248
return digest.BadDigest, err
263249
}
264250

265251
if err := contentAddressableStorage.Put(
266252
ctx,
267253
blobDigest,
268-
buffer.NewValidatedBufferFromReaderAt(f, blobDigest.GetSizeBytes())); err != nil {
254+
buffer.NewValidatedBufferFromReaderAt(frozenFile, blobDigest.GetSizeBytes())); err != nil {
269255
return digest.BadDigest, util.StatusWrap(err, "Failed to upload file")
270256
}
271257
return blobDigest, nil
@@ -275,14 +261,14 @@ func (f *fileBackedFile) getBazelOutputServiceStat(digestFunction *digest.Functi
275261
var locator *anypb.Any
276262
f.lock.Lock()
277263
if f.writableDescriptorsCount == 0 {
278-
success := f.acquireFrozenDescriptorLocked()
264+
frozenFile, success := f.acquireFrozenDescriptorLocked()
279265
f.lock.Unlock()
280266
if !success {
281267
return nil, status.Error(codes.NotFound, "File was unlinked before digest computation could start")
282268
}
283-
defer f.releaseFrozenDescriptor()
269+
defer frozenFile.Close()
284270

285-
blobDigest, err := f.updateCachedDigest(*digestFunction)
271+
blobDigest, err := f.updateCachedDigest(*digestFunction, frozenFile)
286272
if err != nil {
287273
return nil, err
288274
}
@@ -312,18 +298,6 @@ func (f *fileBackedFile) getBazelOutputServiceStat(digestFunction *digest.Functi
312298
}, nil
313299
}
314300

315-
func (f *fileBackedFile) Close() error {
316-
f.releaseFrozenDescriptor()
317-
return nil
318-
}
319-
320-
func (f *fileBackedFile) ReadAt(b []byte, off int64) (int, error) {
321-
f.lock.Lock()
322-
defer f.lock.Unlock()
323-
324-
return f.file.ReadAt(b, off)
325-
}
326-
327301
func (f *fileBackedFile) VirtualAllocate(off, size uint64) Status {
328302
f.lockMutatingData()
329303
defer f.lock.Unlock()
@@ -540,3 +514,45 @@ func (f *fileBackedFile) VirtualWrite(buf []byte, offset uint64) (int, Status) {
540514
}
541515
return nWritten, StatusOK
542516
}
517+
518+
// frozenFileBackedFile is a handle to a file whose contents have been
519+
// frozen and has been opened for reading. The file will be unfrozen
520+
// when closed.
521+
type frozenFileBackedFile struct {
522+
file *fileBackedFile
523+
}
524+
525+
func (ff *frozenFileBackedFile) Close() error {
526+
f := ff.file
527+
f.lock.Lock()
528+
defer f.lock.Unlock()
529+
530+
if f.frozenDescriptorsCount == 0 {
531+
panic("Invalid open frozen descriptor count")
532+
}
533+
f.frozenDescriptorsCount--
534+
if f.frozenDescriptorsCount == 0 && f.unfreezeWakeup != nil {
535+
close(f.unfreezeWakeup)
536+
f.unfreezeWakeup = nil
537+
}
538+
539+
f.releaseReferencesLocked(1)
540+
ff.file = nil
541+
return nil
542+
}
543+
544+
func (ff *frozenFileBackedFile) GetNextRegionOffset(offset int64, regionType filesystem.RegionType) (int64, error) {
545+
f := ff.file
546+
f.lock.Lock()
547+
defer f.lock.Unlock()
548+
549+
return f.file.GetNextRegionOffset(offset, regionType)
550+
}
551+
552+
func (ff *frozenFileBackedFile) ReadAt(b []byte, off int64) (int, error) {
553+
f := ff.file
554+
f.lock.Lock()
555+
defer f.lock.Unlock()
556+
557+
return f.file.ReadAt(b, off)
558+
}

0 commit comments

Comments
 (0)