Skip to content

Commit 1b262df

Browse files
jbowensjbowens
committed
db: consistently use obsoleteFile struct
Previously obsolete tables and blobs were maintained as objectInfos until they were being passed off to the CleanupManager. This commit converts them to obsoleteFiles, allowing the removal and cleanup of some code.
1 parent b99e8be commit 1b262df

File tree

3 files changed

+46
-59
lines changed

3 files changed

+46
-59
lines changed

‎obsolete_files.go‎

Lines changed: 30 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,8 @@ func (d *DB) scanObsoleteFiles(list []string, flushableIngests []*ingestedFlusha
365365

366366
manifestFileNum := d.mu.versions.manifestFileNum
367367

368-
var obsoleteTables []objectInfo
369-
var obsoleteBlobs []objectInfo
368+
var obsoleteTables []obsoleteFile
369+
var obsoleteBlobs []obsoleteFile
370370
var obsoleteOptions []obsoleteFile
371371
var obsoleteManifests []obsoleteFile
372372

@@ -411,32 +411,32 @@ func (d *DB) scanObsoleteFiles(list []string, flushableIngests []*ingestedFlusha
411411
if _, ok := liveFileNums[obj.DiskFileNum]; ok {
412412
continue
413413
}
414-
makeObjectInfo := func() objectInfo {
415-
fileInfo := fileInfo{FileNum: obj.DiskFileNum}
416-
if size, err := d.objProvider.Size(obj); err == nil {
417-
fileInfo.FileSize = uint64(size)
418-
}
419-
return objectInfo{
420-
fileInfo: fileInfo,
421-
isLocal: !obj.IsRemote(),
422-
}
423-
}
424-
425-
switch obj.FileType {
426-
case base.FileTypeTable:
427-
obsoleteTables = append(obsoleteTables, makeObjectInfo())
428-
case base.FileTypeBlob:
429-
obsoleteBlobs = append(obsoleteBlobs, makeObjectInfo())
430-
default:
414+
if obj.FileType != base.FileTypeTable && obj.FileType != base.FileTypeBlob {
431415
// Ignore object types we don't know about.
416+
continue
417+
}
418+
of := obsoleteFile{
419+
fileType: obj.FileType,
420+
fs: d.opts.FS,
421+
path: base.MakeFilepath(d.opts.FS, d.dirname, obj.FileType, obj.DiskFileNum),
422+
fileNum: obj.DiskFileNum,
423+
isLocal: true,
424+
}
425+
if size, err := d.objProvider.Size(obj); err == nil {
426+
of.fileSize = uint64(size)
427+
}
428+
if obj.FileType == base.FileTypeTable {
429+
obsoleteTables = append(obsoleteTables, of)
430+
} else {
431+
obsoleteBlobs = append(obsoleteBlobs, of)
432432
}
433433
}
434434

435-
d.mu.versions.obsoleteTables = mergeObjectInfos(d.mu.versions.obsoleteTables, obsoleteTables)
436-
d.mu.versions.obsoleteBlobs = mergeObjectInfos(d.mu.versions.obsoleteBlobs, obsoleteBlobs)
437-
d.mu.versions.updateObsoleteObjectMetricsLocked()
435+
d.mu.versions.obsoleteTables = mergeObsoleteFiles(d.mu.versions.obsoleteTables, obsoleteTables)
436+
d.mu.versions.obsoleteBlobs = mergeObsoleteFiles(d.mu.versions.obsoleteBlobs, obsoleteBlobs)
438437
d.mu.versions.obsoleteManifests = mergeObsoleteFiles(d.mu.versions.obsoleteManifests, obsoleteManifests)
439438
d.mu.versions.obsoleteOptions = mergeObsoleteFiles(d.mu.versions.obsoleteOptions, obsoleteOptions)
439+
d.mu.versions.updateObsoleteObjectMetricsLocked()
440440
}
441441

442442
// disableFileDeletions disables file deletions and then waits for any
@@ -504,9 +504,9 @@ func (d *DB) deleteObsoleteFiles(jobID JobID) {
504504
d.opts.Logger.Fatalf("obsoleteManifests is not sorted")
505505
case !slices.IsSortedFunc(d.mu.versions.obsoleteOptions, cmpObsoleteFileNumbers):
506506
d.opts.Logger.Fatalf("obsoleteOptions is not sorted")
507-
case !slices.IsSortedFunc(obsoleteTables, cmpObjectFileNums):
507+
case !slices.IsSortedFunc(obsoleteTables, cmpObsoleteFileNumbers):
508508
d.opts.Logger.Fatalf("obsoleteTables is not sorted")
509-
case !slices.IsSortedFunc(obsoleteBlobs, cmpObjectFileNums):
509+
case !slices.IsSortedFunc(obsoleteBlobs, cmpObsoleteFileNumbers):
510510
d.opts.Logger.Fatalf("obsoleteBlobs is not sorted")
511511
}
512512
}
@@ -529,9 +529,12 @@ func (d *DB) deleteObsoleteFiles(jobID JobID) {
529529
d.mu.Unlock()
530530
defer d.mu.Lock()
531531

532-
filesToDelete := make([]obsoleteFile, 0, len(obsoleteLogs)+len(obsoleteTables)+len(obsoleteManifests)+len(obsoleteOptions))
532+
n := len(obsoleteLogs) + len(obsoleteTables) + len(obsoleteBlobs) + len(obsoleteManifests) + len(obsoleteOptions)
533+
filesToDelete := make([]obsoleteFile, 0, n)
533534
filesToDelete = append(filesToDelete, obsoleteManifests...)
534535
filesToDelete = append(filesToDelete, obsoleteOptions...)
536+
filesToDelete = append(filesToDelete, obsoleteTables...)
537+
filesToDelete = append(filesToDelete, obsoleteBlobs...)
535538
for _, f := range obsoleteLogs {
536539
filesToDelete = append(filesToDelete, obsoleteFile{
537540
fileType: base.FileTypeLog,
@@ -543,14 +546,11 @@ func (d *DB) deleteObsoleteFiles(jobID JobID) {
543546
})
544547
}
545548
for _, f := range obsoleteTables {
546-
d.fileCache.Evict(f.FileNum, base.FileTypeTable)
547-
filesToDelete = append(filesToDelete, f.asObsoleteFile(d.opts.FS, base.FileTypeTable, d.dirname))
549+
d.fileCache.Evict(f.fileNum, base.FileTypeTable)
548550
}
549551
for _, f := range obsoleteBlobs {
550-
d.fileCache.Evict(f.FileNum, base.FileTypeBlob)
551-
filesToDelete = append(filesToDelete, f.asObsoleteFile(d.opts.FS, base.FileTypeBlob, d.dirname))
552+
d.fileCache.Evict(f.fileNum, base.FileTypeBlob)
552553
}
553-
554554
if len(filesToDelete) > 0 {
555555
d.cleanupManager.EnqueueJob(jobID, filesToDelete)
556556
}
@@ -682,18 +682,3 @@ func (z *zombieObjects) TotalSize() uint64 {
682682
func (z *zombieObjects) LocalStats() (count uint64, size uint64) {
683683
return z.localCount, z.localSize
684684
}
685-
686-
func mergeObjectInfos(a, b []objectInfo) []objectInfo {
687-
if len(b) == 0 {
688-
return a
689-
}
690-
a = append(a, b...)
691-
slices.SortFunc(a, cmpObjectFileNums)
692-
return slices.CompactFunc(a, func(a, b objectInfo) bool {
693-
return a.FileNum == b.FileNum
694-
})
695-
}
696-
697-
func cmpObjectFileNums(a, b objectInfo) int {
698-
return cmp.Compare(a.FileNum, b.FileNum)
699-
}

‎version_set.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ type versionSet struct {
7575
// on the creation of every version.
7676
obsoleteFn func(manifest.ObsoleteFiles)
7777
// obsolete{Tables,Blobs,Manifests,Options} are sorted by file number ascending.
78-
obsoleteTables []objectInfo
79-
obsoleteBlobs []objectInfo
78+
obsoleteTables []obsoleteFile
79+
obsoleteBlobs []obsoleteFile
8080
obsoleteManifests []obsoleteFile
8181
obsoleteOptions []obsoleteFile
8282

@@ -1167,17 +1167,19 @@ func (vs *versionSet) addObsoleteLocked(obsolete manifest.ObsoleteFiles) {
11671167
// Note that the zombie objects transition from zombie *to* obsolete, and
11681168
// will no longer be considered zombie.
11691169

1170-
newlyObsoleteTables := make([]objectInfo, len(obsolete.TableBackings))
1170+
newlyObsoleteTables := make([]obsoleteFile, len(obsolete.TableBackings))
11711171
for i, bs := range obsolete.TableBackings {
1172-
newlyObsoleteTables[i] = vs.zombieTables.Extract(bs.DiskFileNum)
1172+
newlyObsoleteTables[i] = vs.zombieTables.Extract(bs.DiskFileNum).
1173+
asObsoleteFile(vs.fs, base.FileTypeTable, vs.dirname)
11731174
}
1174-
vs.obsoleteTables = mergeObjectInfos(vs.obsoleteTables, newlyObsoleteTables)
1175+
vs.obsoleteTables = mergeObsoleteFiles(vs.obsoleteTables, newlyObsoleteTables)
11751176

1176-
newlyObsoleteBlobFiles := make([]objectInfo, len(obsolete.BlobFiles))
1177+
newlyObsoleteBlobFiles := make([]obsoleteFile, len(obsolete.BlobFiles))
11771178
for i, bf := range obsolete.BlobFiles {
1178-
newlyObsoleteBlobFiles[i] = vs.zombieBlobs.Extract(base.DiskFileNum(bf.FileID))
1179+
newlyObsoleteBlobFiles[i] = vs.zombieBlobs.Extract(base.DiskFileNum(bf.FileID)).
1180+
asObsoleteFile(vs.fs, base.FileTypeBlob, vs.dirname)
11791181
}
1180-
vs.obsoleteBlobs = mergeObjectInfos(vs.obsoleteBlobs, newlyObsoleteBlobFiles)
1182+
vs.obsoleteBlobs = mergeObsoleteFiles(vs.obsoleteBlobs, newlyObsoleteBlobFiles)
11811183
vs.updateObsoleteObjectMetricsLocked()
11821184
}
11831185

@@ -1195,9 +1197,9 @@ func (vs *versionSet) updateObsoleteObjectMetricsLocked() {
11951197
vs.metrics.Table.Local.ObsoleteSize = 0
11961198
vs.metrics.Table.Local.ObsoleteCount = 0
11971199
for _, fi := range vs.obsoleteTables {
1198-
vs.metrics.Table.ObsoleteSize += fi.FileSize
1200+
vs.metrics.Table.ObsoleteSize += fi.fileSize
11991201
if fi.isLocal {
1200-
vs.metrics.Table.Local.ObsoleteSize += fi.FileSize
1202+
vs.metrics.Table.Local.ObsoleteSize += fi.fileSize
12011203
vs.metrics.Table.Local.ObsoleteCount++
12021204
}
12031205
}
@@ -1206,9 +1208,9 @@ func (vs *versionSet) updateObsoleteObjectMetricsLocked() {
12061208
vs.metrics.BlobFiles.Local.ObsoleteSize = 0
12071209
vs.metrics.BlobFiles.Local.ObsoleteCount = 0
12081210
for _, fi := range vs.obsoleteBlobs {
1209-
vs.metrics.BlobFiles.ObsoleteSize += fi.FileSize
1211+
vs.metrics.BlobFiles.ObsoleteSize += fi.fileSize
12101212
if fi.isLocal {
1211-
vs.metrics.BlobFiles.Local.ObsoleteSize += fi.FileSize
1213+
vs.metrics.BlobFiles.Local.ObsoleteSize += fi.fileSize
12121214
vs.metrics.BlobFiles.Local.ObsoleteCount++
12131215
}
12141216
}

‎version_set_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func TestVersionSet(t *testing.T) {
222222
}
223223
}
224224
buf.WriteString(vs.virtualBackings.String())
225-
printObjectBreakdown := func(kind string, zombies zombieObjects, obsolete []objectInfo) {
225+
printObjectBreakdown := func(kind string, zombies zombieObjects, obsolete []obsoleteFile) {
226226
if zombies.Count() == 0 {
227227
buf.WriteString(fmt.Sprintf("no zombie %s\n", kind))
228228
} else {
@@ -239,7 +239,7 @@ func TestVersionSet(t *testing.T) {
239239
} else {
240240
buf.WriteString(fmt.Sprintf("obsolete %s:", kind))
241241
for _, fi := range obsolete {
242-
fmt.Fprintf(&buf, " %s", fi.FileNum)
242+
fmt.Fprintf(&buf, " %s", fi.fileNum)
243243
}
244244
buf.WriteString("\n")
245245
}

0 commit comments

Comments
 (0)