Skip to content

Commit 818109f

Browse files
RaduBerindeRaduBerinde
committed
objstorageprovider: minor local subsystem cleanup
Minor cleanup of the local/vfs subsystem.
1 parent 936d14c commit 818109f

File tree

2 files changed

+60
-55
lines changed

2 files changed

+60
-55
lines changed

‎objstorage/objstorageprovider/provider.go‎

Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,17 @@ import (
2929
type provider struct {
3030
st Settings
3131

32-
fsDir vfs.File
33-
3432
tracer *objiotracing.Tracer
3533

34+
local localSubsystem
3635
remote remoteSubsystem
3736

3837
mu struct {
3938
sync.RWMutex
4039

40+
local localLockedState
4141
remote remoteLockedState
4242

43-
// TODO(radu): move these fields to a localLockedState struct.
44-
// localObjectsChanged is incremented whenever non-remote objects are created.
45-
// The purpose of this counter is to avoid syncing the local filesystem when
46-
// only remote objects are changed.
47-
localObjectsChangeCounter uint64
48-
// localObjectsChangeCounterSynced is the value of localObjectsChangeCounter
49-
// value at the time the last completed sync was launched.
50-
localObjectsChangeCounterSynced uint64
51-
5243
// knownObjects maintains information about objects that are known to the provider.
5344
// It is initialized with the list of files in the manifest when we open a DB.
5445
knownObjects map[base.DiskFileNum]objstorage.ObjectMetadata
@@ -229,24 +220,12 @@ func Open(settings Settings) (objstorage.Provider, error) {
229220
}
230221

231222
func open(settings Settings) (p *provider, _ error) {
232-
fsDir, err := settings.FS.OpenDir(settings.FSDirName)
233-
if err != nil {
234-
return nil, err
235-
}
236-
237-
defer func() {
238-
if p == nil {
239-
fsDir.Close()
240-
}
241-
}()
242-
243223
if settings.Local.ReadaheadConfig == nil {
244224
settings.Local.ReadaheadConfig = NewReadaheadConfig()
245225
}
246226

247227
p = &provider{
248-
st: settings,
249-
fsDir: fsDir,
228+
st: settings,
250229
}
251230
p.mu.knownObjects = make(map[base.DiskFileNum]objstorage.ObjectMetadata)
252231
p.mu.protectedObjects = make(map[base.DiskFileNum]int)
@@ -255,13 +234,14 @@ func open(settings Settings) (p *provider, _ error) {
255234
p.tracer = objiotracing.Open(settings.FS, settings.FSDirName)
256235
}
257236

258-
// Add local FS objects.
259-
if err := p.vfsInit(); err != nil {
237+
// Initialize local subsystem and add local vfs.FS objects.
238+
if err := p.localInit(); err != nil {
260239
return nil, err
261240
}
262241

263242
// Initialize remote subsystem (if configured) and add remote objects.
264243
if err := p.remoteInit(); err != nil {
244+
_ = p.localClose()
265245
return nil, err
266246
}
267247

@@ -271,10 +251,7 @@ func open(settings Settings) (p *provider, _ error) {
271251
// Close is part of the objstorage.Provider interface.
272252
func (p *provider) Close() error {
273253
err := p.sharedClose()
274-
if p.fsDir != nil {
275-
err = firstError(err, p.fsDir.Close())
276-
p.fsDir = nil
277-
}
254+
err = firstError(err, p.localClose())
278255
if objiotracing.Enabled {
279256
if p.tracer != nil {
280257
p.tracer.Close()
@@ -301,7 +278,7 @@ func (p *provider) OpenForReading(
301278

302279
var r objstorage.Readable
303280
if !meta.IsRemote() {
304-
r, err = p.vfsOpenForReading(ctx, fileType, fileNum, opts)
281+
r, err = p.localOpenForReading(ctx, fileType, fileNum, opts)
305282
} else {
306283
r, err = p.remoteOpenForReading(ctx, meta, opts)
307284
if err != nil && p.isNotExistError(meta, err) {
@@ -365,7 +342,7 @@ func (p *provider) Remove(fileType base.FileType, fileNum base.DiskFileNum) erro
365342
}
366343

367344
if !meta.IsRemote() {
368-
err = p.vfsRemove(fileType, fileNum)
345+
err = p.localRemove(fileType, fileNum)
369346
} else {
370347
// TODO(radu): implement remote object removal (i.e. deref).
371348
err = p.sharedUnref(meta)
@@ -401,7 +378,7 @@ func (p *provider) IsNotExistError(err error) bool {
401378

402379
// Sync flushes the metadata from creation or removal of objects since the last Sync.
403380
func (p *provider) Sync() error {
404-
if err := p.vfsSync(); err != nil {
381+
if err := p.localSync(); err != nil {
405382
return err
406383
}
407384
if err := p.sharedSync(); err != nil {
@@ -432,7 +409,7 @@ func (p *provider) LinkOrCopyFromLocal(
432409
NoSyncOnClose: p.st.NoSyncOnClose,
433410
BytesPerSync: p.st.BytesPerSync,
434411
})
435-
dstPath := p.vfsPath(dstFileType, dstFileNum)
412+
dstPath := p.localPath(dstFileType, dstFileNum)
436413
if err := vfs.LinkOrCopy(fs, srcFilePath, dstPath); err != nil {
437414
return objstorage.ObjectMetadata{}, err
438415
}
@@ -505,15 +482,15 @@ func (p *provider) Lookup(
505482
// Path is part of the objstorage.Provider interface.
506483
func (p *provider) Path(meta objstorage.ObjectMetadata) string {
507484
if !meta.IsRemote() {
508-
return p.vfsPath(meta.FileType, meta.DiskFileNum)
485+
return p.localPath(meta.FileType, meta.DiskFileNum)
509486
}
510487
return p.remotePath(meta)
511488
}
512489

513490
// Size returns the size of the object.
514491
func (p *provider) Size(meta objstorage.ObjectMetadata) (int64, error) {
515492
if !meta.IsRemote() {
516-
return p.vfsSize(meta.FileType, meta.DiskFileNum)
493+
return p.localSize(meta.FileType, meta.DiskFileNum)
517494
}
518495
return p.remoteSize(meta)
519496
}
@@ -587,7 +564,7 @@ func (p *provider) addMetadataLocked(meta objstorage.ObjectMetadata) {
587564
p.mu.remote.addExternalObject(meta)
588565
}
589566
} else {
590-
p.mu.localObjectsChangeCounter++
567+
p.mu.local.objChangeCounter++
591568
}
592569
}
593570

@@ -606,7 +583,7 @@ func (p *provider) removeMetadata(fileNum base.DiskFileNum) {
606583
if meta.IsRemote() {
607584
p.mu.remote.catalogBatch.DeleteObject(fileNum)
608585
} else {
609-
p.mu.localObjectsChangeCounter++
586+
p.mu.local.objChangeCounter++
610587
}
611588
}
612589

‎objstorage/objstorageprovider/vfs.go‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,31 @@ import (
1313
"github.com/cockroachdb/pebble/vfs"
1414
)
1515

16-
func (p *provider) vfsPath(fileType base.FileType, fileNum base.DiskFileNum) string {
16+
type localSubsystem struct {
17+
fsDir vfs.File
18+
}
19+
20+
type localLockedState struct {
21+
// objChangeCounter is incremented whenever non-remote objects are created.
22+
// The purpose of this counter is to avoid syncing the local filesystem when
23+
// only remote objects are changed.
24+
objChangeCounter uint64
25+
// objChangeCounterLastSync is the value of objChangeCounter at the time the
26+
// last completed sync was launched.
27+
objChangeCounterLastSync uint64
28+
}
29+
30+
func (p *provider) localPath(fileType base.FileType, fileNum base.DiskFileNum) string {
1731
return base.MakeFilepath(p.st.FS, p.st.FSDirName, fileType, fileNum)
1832
}
1933

20-
func (p *provider) vfsOpenForReading(
34+
func (p *provider) localOpenForReading(
2135
ctx context.Context,
2236
fileType base.FileType,
2337
fileNum base.DiskFileNum,
2438
opts objstorage.OpenOptions,
2539
) (objstorage.Readable, error) {
26-
filename := p.vfsPath(fileType, fileNum)
40+
filename := p.localPath(fileType, fileNum)
2741
file, err := p.st.FS.Open(filename, vfs.RandomReadsOption)
2842
if err != nil {
2943
if opts.MustExist && p.IsNotExistError(err) {
@@ -41,7 +55,7 @@ func (p *provider) vfsCreate(
4155
fileNum base.DiskFileNum,
4256
category vfs.DiskWriteCategory,
4357
) (objstorage.Writable, objstorage.ObjectMetadata, error) {
44-
filename := p.vfsPath(fileType, fileNum)
58+
filename := p.localPath(fileType, fileNum)
4559
file, err := p.st.FS.Create(filename, category)
4660
if err != nil {
4761
return nil, objstorage.ObjectMetadata{}, err
@@ -57,17 +71,22 @@ func (p *provider) vfsCreate(
5771
return newFileBufferedWritable(file), meta, nil
5872
}
5973

60-
func (p *provider) vfsRemove(fileType base.FileType, fileNum base.DiskFileNum) error {
61-
return p.st.FSCleaner.Clean(p.st.FS, fileType, p.vfsPath(fileType, fileNum))
74+
func (p *provider) localRemove(fileType base.FileType, fileNum base.DiskFileNum) error {
75+
return p.st.FSCleaner.Clean(p.st.FS, fileType, p.localPath(fileType, fileNum))
6276
}
6377

64-
// vfsInit finds any local FS objects.
65-
func (p *provider) vfsInit() error {
78+
// localInit finds any local FS objects.
79+
func (p *provider) localInit() error {
80+
fsDir, err := p.st.FS.OpenDir(p.st.FSDirName)
81+
if err != nil {
82+
return err
83+
}
84+
p.local.fsDir = fsDir
6685
listing := p.st.FSDirInitialListing
6786
if listing == nil {
68-
var err error
6987
listing, err = p.st.FS.List(p.st.FSDirName)
7088
if err != nil {
89+
_ = p.localClose()
7190
return errors.Wrapf(err, "pebble: could not list store directory")
7291
}
7392
}
@@ -88,30 +107,39 @@ func (p *provider) vfsInit() error {
88107
return nil
89108
}
90109

91-
func (p *provider) vfsSync() error {
110+
func (p *provider) localClose() error {
111+
var err error
112+
if p.local.fsDir != nil {
113+
err = p.local.fsDir.Close()
114+
p.local.fsDir = nil
115+
}
116+
return err
117+
}
118+
119+
func (p *provider) localSync() error {
92120
p.mu.Lock()
93-
counterVal := p.mu.localObjectsChangeCounter
94-
lastSynced := p.mu.localObjectsChangeCounterSynced
121+
counterVal := p.mu.local.objChangeCounter
122+
lastSynced := p.mu.local.objChangeCounterLastSync
95123
p.mu.Unlock()
96124

97125
if lastSynced >= counterVal {
98126
return nil
99127
}
100-
if err := p.fsDir.Sync(); err != nil {
128+
if err := p.local.fsDir.Sync(); err != nil {
101129
return err
102130
}
103131

104132
p.mu.Lock()
105-
if p.mu.localObjectsChangeCounterSynced < counterVal {
106-
p.mu.localObjectsChangeCounterSynced = counterVal
133+
if p.mu.local.objChangeCounterLastSync < counterVal {
134+
p.mu.local.objChangeCounterLastSync = counterVal
107135
}
108136
p.mu.Unlock()
109137

110138
return nil
111139
}
112140

113-
func (p *provider) vfsSize(fileType base.FileType, fileNum base.DiskFileNum) (int64, error) {
114-
filename := p.vfsPath(fileType, fileNum)
141+
func (p *provider) localSize(fileType base.FileType, fileNum base.DiskFileNum) (int64, error) {
142+
filename := p.localPath(fileType, fileNum)
115143
stat, err := p.st.FS.Stat(filename)
116144
if err != nil {
117145
return 0, err

0 commit comments

Comments
 (0)