objstorage: local settings cleanup

Cleanup for local settings.

Author: RaduBerinde

blob_rewrite_test.go

@@ -52,9 +52,9 @@ func TestBlobRewrite(t *testing.T) {
 		})
 	)
 	ctx := context.Background()
-	objStore, err := objstorageprovider.Open(objstorageprovider.Settings{
-		FS: fs,
-	})
+	var st objstorageprovider.Settings
+	st.Local.FS = fs
+	objStore, err := objstorageprovider.Open(st)
 	require.NoError(t, err)
 
 	initRawWriter := func() {
@@ -261,9 +261,9 @@ func TestBlobRewriteRandomized(t *testing.T) {
 	}()
 
 	ctx := context.Background()
-	objStore, err := objstorageprovider.Open(objstorageprovider.Settings{
-		FS: vfs.NewMem(),
-	})
+	var st objstorageprovider.Settings
+	st.Local.FS = vfs.NewMem()
+	objStore, err := objstorageprovider.Open(st)
 	require.NoError(t, err)
 
 	// Write the source blob file.

download_test.go

@@ -207,8 +207,8 @@ func TestDownloadTask(t *testing.T) {
 func initDownloadTestProvider(t *testing.T) objstorage.Provider {
 	providerSettings := objstorageprovider.Settings{
 		Logger: base.DefaultLogger,
-		FS:     vfs.NewMem(),
 	}
+	providerSettings.Local.FS = vfs.NewMem()
 	providerSettings.Remote.StorageFactory = remote.MakeSimpleFactory(map[remote.Locator]remote.Storage{
 		"": remote.NewInMem(),
 	})

ingest_test.go

@@ -430,7 +430,7 @@ func TestIngestLinkFallback(t *testing.T) {
 	opts.WithFSDefaults()
 	objSettings := objstorageprovider.DefaultSettings(opts.FS, "")
 	// Prevent the provider from listing the dir (where we may get an injected error).
-	objSettings.FSDirInitialListing = []string{}
+	objSettings.Local.FSDirInitialListing = []string{}
 	objProvider, err := objstorageprovider.Open(objSettings)
 	require.NoError(t, err)
 	defer objProvider.Close()
@@ -1025,14 +1025,14 @@ func TestSimpleIngestShared(t *testing.T) {
 	// Create an objProvider where we will fake-create some sstables that can
 	// then be shared back to the db instance.
 	providerSettings := objstorageprovider.Settings{
-		Logger:              opts2.Logger,
-		FS:                  opts2.FS,
-		FSDirName:           "",
-		FSDirInitialListing: nil,
-		FSCleaner:           opts2.Cleaner,
-		NoSyncOnClose:       opts2.NoSyncOnClose,
-		BytesPerSync:        opts2.BytesPerSync,
+		Logger: opts2.Logger,
 	}
+	providerSettings.Local.FS = opts2.FS
+	providerSettings.Local.FSDirName = ""
+	providerSettings.Local.FSDirInitialListing = nil
+	providerSettings.Local.FSCleaner = opts2.Cleaner
+	providerSettings.Local.NoSyncOnClose = opts2.NoSyncOnClose
+	providerSettings.Local.BytesPerSync = opts2.BytesPerSync
 	providerSettings.Remote.StorageFactory = remote.MakeSimpleFactory(map[remote.Locator]remote.Storage{
 		"": remote.NewInMem(),
 	})

objstorage/objstorageprovider/provider.go

@@ -57,37 +57,33 @@ type Settings struct {
 	Logger base.Logger
 
 	// Local filesystem configuration.
-	FS        vfs.FS
-	FSDirName string
-
-	// FSDirInitialListing is a listing of FSDirName at the time of calling Open.
-	//
-	// This is an optional optimization to avoid double listing on Open when the
-	// higher layer already has a listing. When nil, we obtain the listing on
-	// Open.
-	FSDirInitialListing []string
-
-	// Cleaner cleans obsolete files from the local filesystem.
-	//
-	// The default cleaner uses the DeleteCleaner.
-	FSCleaner base.Cleaner
-
-	// NoSyncOnClose decides whether the implementation will enforce a
-	// close-time synchronization (e.g., fdatasync() or sync_file_range())
-	// on files it writes to. Setting this to true removes the guarantee for a
-	// sync on close. Some implementations can still issue a non-blocking sync.
-	NoSyncOnClose bool
-
-	// BytesPerSync enables periodic syncing of files in order to smooth out
-	// writes to disk. This option does not provide any persistence guarantee, but
-	// is used to avoid latency spikes if the OS automatically decides to write
-	// out a large chunk of dirty filesystem buffers.
-	BytesPerSync int
-
-	// Local contains fields that are only relevant for files stored on the local
-	// filesystem.
 	Local struct {
-		// TODO(radu): move FSCleaner, NoSyncOnClose, BytesPerSync here.
+		FS        vfs.FS
+		FSDirName string
+
+		// FSDirInitialListing is a listing of FSDirName at the time of calling Open.
+		//
+		// This is an optional optimization to avoid double listing on Open when the
+		// higher layer already has a listing. When nil, we obtain the listing on
+		// Open.
+		FSDirInitialListing []string
+
+		// Cleaner cleans obsolete files from the local filesystem.
+		//
+		// The default cleaner uses the DeleteCleaner.
+		FSCleaner base.Cleaner
+
+		// NoSyncOnClose decides whether the implementation will enforce a
+		// close-time synchronization (e.g., fdatasync() or sync_file_range())
+		// on files it writes to. Setting this to true removes the guarantee for a
+		// sync on close. Some implementations can still issue a non-blocking sync.
+		NoSyncOnClose bool
+
+		// BytesPerSync enables periodic syncing of files in order to smooth out
+		// writes to disk. This option does not provide any persistence guarantee, but
+		// is used to avoid latency spikes if the OS automatically decides to write
+		// out a large chunk of dirty filesystem buffers.
+		BytesPerSync int
 
 		// ReadaheadConfig is used to retrieve the current readahead mode; it is
 		// consulted whenever a read handle is initialized.
@@ -198,14 +194,15 @@ const (
 // DefaultSettings initializes default settings (with no remote storage),
 // suitable for tests and tools.
 func DefaultSettings(fs vfs.FS, dirName string) Settings {
-	return Settings{
-		Logger:        base.DefaultLogger,
-		FS:            fs,
-		FSDirName:     dirName,
-		FSCleaner:     base.DeleteCleaner{},
-		NoSyncOnClose: false,
-		BytesPerSync:  512 * 1024, // 512KB
-	}
+	st := Settings{
+		Logger: base.DefaultLogger,
+	}
+	st.Local.FS = fs
+	st.Local.FSDirName = dirName
+	st.Local.FSCleaner = base.DeleteCleaner{}
+	st.Local.NoSyncOnClose = false
+	st.Local.BytesPerSync = 512 * 1024
+	return st
 }
 
 // Open creates the provider.
@@ -231,7 +228,7 @@ func open(settings Settings) (p *provider, _ error) {
 	p.mu.protectedObjects = make(map[base.DiskFileNum]int)
 
 	if objiotracing.Enabled {
-		p.tracer = objiotracing.Open(settings.FS, settings.FSDirName)
+		p.tracer = objiotracing.Open(settings.Local.FS, settings.Local.FSDirName)
 	}
 
 	// Initialize local subsystem and add local vfs.FS objects.
@@ -402,12 +399,12 @@ func (p *provider) LinkOrCopyFromLocal(
 	opts objstorage.CreateOptions,
 ) (objstorage.ObjectMetadata, error) {
 	shared := opts.PreferSharedStorage && p.st.Remote.CreateOnShared != remote.CreateOnSharedNone
-	if !shared && srcFS == p.st.FS {
+	if !shared && srcFS == p.st.Local.FS {
 		// Wrap the normal filesystem with one which wraps newly created files with
 		// vfs.NewSyncingFile.
-		fs := vfs.NewSyncingFS(p.st.FS, vfs.SyncingFileOptions{
-			NoSyncOnClose: p.st.NoSyncOnClose,
-			BytesPerSync:  p.st.BytesPerSync,
+		fs := vfs.NewSyncingFS(p.st.Local.FS, vfs.SyncingFileOptions{
+			NoSyncOnClose: p.st.Local.NoSyncOnClose,
+			BytesPerSync:  p.st.Local.BytesPerSync,
 		})
 		dstPath := p.localPath(dstFileType, dstFileNum)
 		if err := vfs.LinkOrCopy(fs, srcFilePath, dstPath); err != nil {

objstorage/objstorageprovider/provider_test.go

@@ -101,7 +101,7 @@ func TestProvider(t *testing.T) {
 			case "close":
 				require.NoError(t, curProvider.Sync())
 				require.NoError(t, curProvider.Close())
-				delete(providers, curProvider.(*provider).st.FSDirName)
+				delete(providers, curProvider.(*provider).st.Local.FSDirName)
 				curProvider = nil
 
 				return log.String()
@@ -703,7 +703,7 @@ func TestParallelSync(t *testing.T) {
 			require.NoError(t, p.Close())
 
 			if !shared {
-				st.FS = crashFS
+				st.Local.FS = crashFS
 			}
 			p, err = Open(st)
 			require.NoError(t, err)

objstorage/objstorageprovider/remote.go

@@ -89,7 +89,7 @@ func (p *provider) remoteInit() error {
 	if p.st.Remote.StorageFactory == nil {
 		return nil
 	}
-	catalog, contents, err := remoteobjcat.Open(p.st.FS, p.st.FSDirName)
+	catalog, contents, err := remoteobjcat.Open(p.st.Local.FS, p.st.Local.FSDirName)
 	if err != nil {
 		return errors.Wrapf(err, "pebble: could not open remote object catalog")
 	}
@@ -123,7 +123,7 @@ func (p *provider) remoteInit() error {
 		}
 
 		p.remote.cache, err = sharedcache.Open(
-			p.st.FS, p.st.Logger, p.st.FSDirName, blockSize, shardingBlockSize, p.st.Remote.CacheSizeBytes, numShards)
+			p.st.Local.FS, p.st.Logger, p.st.Local.FSDirName, blockSize, shardingBlockSize, p.st.Remote.CacheSizeBytes, numShards)
 		if err != nil {
 			return errors.Wrapf(err, "pebble: could not open remote object cache")
 		}

objstorage/objstorageprovider/vfs.go

@@ -28,7 +28,7 @@ type localLockedState struct {
 }
 
 func (p *provider) localPath(fileType base.FileType, fileNum base.DiskFileNum) string {
-	return base.MakeFilepath(p.st.FS, p.st.FSDirName, fileType, fileNum)
+	return base.MakeFilepath(p.st.Local.FS, p.st.Local.FSDirName, fileType, fileNum)
 }
 
 func (p *provider) localOpenForReading(
@@ -38,15 +38,15 @@ func (p *provider) localOpenForReading(
 	opts objstorage.OpenOptions,
 ) (objstorage.Readable, error) {
 	filename := p.localPath(fileType, fileNum)
-	file, err := p.st.FS.Open(filename, vfs.RandomReadsOption)
+	file, err := p.st.Local.FS.Open(filename, vfs.RandomReadsOption)
 	if err != nil {
 		if opts.MustExist && p.IsNotExistError(err) {
-			err = base.AddDetailsToNotExistError(p.st.FS, filename, err)
+			err = base.AddDetailsToNotExistError(p.st.Local.FS, filename, err)
 			err = base.MarkCorruptionError(err)
 		}
 		return nil, err
 	}
-	return newFileReadable(file, p.st.FS, p.st.Local.ReadaheadConfig, filename)
+	return newFileReadable(file, p.st.Local.FS, p.st.Local.ReadaheadConfig, filename)
 }
 
 func (p *provider) vfsCreate(
@@ -56,13 +56,13 @@ func (p *provider) vfsCreate(
 	category vfs.DiskWriteCategory,
 ) (objstorage.Writable, objstorage.ObjectMetadata, error) {
 	filename := p.localPath(fileType, fileNum)
-	file, err := p.st.FS.Create(filename, category)
+	file, err := p.st.Local.FS.Create(filename, category)
 	if err != nil {
 		return nil, objstorage.ObjectMetadata{}, err
 	}
 	file = vfs.NewSyncingFile(file, vfs.SyncingFileOptions{
-		NoSyncOnClose: p.st.NoSyncOnClose,
-		BytesPerSync:  p.st.BytesPerSync,
+		NoSyncOnClose: p.st.Local.NoSyncOnClose,
+		BytesPerSync:  p.st.Local.BytesPerSync,
 	})
 	meta := objstorage.ObjectMetadata{
 		DiskFileNum: fileNum,
@@ -72,27 +72,27 @@ func (p *provider) vfsCreate(
 }
 
 func (p *provider) localRemove(fileType base.FileType, fileNum base.DiskFileNum) error {
-	return p.st.FSCleaner.Clean(p.st.FS, fileType, p.localPath(fileType, fileNum))
+	return p.st.Local.FSCleaner.Clean(p.st.Local.FS, fileType, p.localPath(fileType, fileNum))
 }
 
 // localInit finds any local FS objects.
 func (p *provider) localInit() error {
-	fsDir, err := p.st.FS.OpenDir(p.st.FSDirName)
+	fsDir, err := p.st.Local.FS.OpenDir(p.st.Local.FSDirName)
 	if err != nil {
 		return err
 	}
 	p.local.fsDir = fsDir
-	listing := p.st.FSDirInitialListing
+	listing := p.st.Local.FSDirInitialListing
 	if listing == nil {
-		listing, err = p.st.FS.List(p.st.FSDirName)
+		listing, err = p.st.Local.FS.List(p.st.Local.FSDirName)
 		if err != nil {
 			_ = p.localClose()
 			return errors.Wrapf(err, "pebble: could not list store directory")
 		}
 	}
 
 	for _, filename := range listing {
-		fileType, fileNum, ok := base.ParseFilename(p.st.FS, filename)
+		fileType, fileNum, ok := base.ParseFilename(p.st.Local.FS, filename)
 		if ok {
 			switch fileType {
 			case base.FileTypeTable, base.FileTypeBlob:
@@ -140,7 +140,7 @@ func (p *provider) localSync() error {
 
 func (p *provider) localSize(fileType base.FileType, fileNum base.DiskFileNum) (int64, error) {
 	filename := p.localPath(fileType, fileNum)
-	stat, err := p.st.FS.Stat(filename)
+	stat, err := p.st.Local.FS.Stat(filename)
 	if err != nil {
 		return 0, err
 	}

options.go

@@ -2735,13 +2735,13 @@ func (d *DB) makeBlobWriterOptions(level int) blob.FileWriterOptions {
 
 func (o *Options) MakeObjStorageProviderSettings(dirname string) objstorageprovider.Settings {
 	s := objstorageprovider.Settings{
-		Logger:        o.Logger,
-		FS:            o.FS,
-		FSDirName:     dirname,
-		FSCleaner:     o.Cleaner,
-		NoSyncOnClose: o.NoSyncOnClose,
-		BytesPerSync:  o.BytesPerSync,
+		Logger: o.Logger,
 	}
+	s.Local.FS = o.FS
+	s.Local.FSDirName = dirname
+	s.Local.FSCleaner = o.Cleaner
+	s.Local.NoSyncOnClose = o.NoSyncOnClose
+	s.Local.BytesPerSync = o.BytesPerSync
 	s.Local.ReadaheadConfig = o.Local.ReadaheadConfig
 	s.Remote.StorageFactory = o.Experimental.RemoteStorage
 	s.Remote.CreateOnShared = o.Experimental.CreateOnShared

recovery.go

@@ -62,7 +62,7 @@ func (rs *recoveredState) init(opts *Options, dirname string) error {
 
 	// Open the object storage provider.
 	providerSettings := opts.MakeObjStorageProviderSettings(dirname)
-	providerSettings.FSDirInitialListing = rs.ls
+	providerSettings.Local.FSDirInitialListing = rs.ls
 	rs.objProvider, err = objstorageprovider.Open(providerSettings)
 	if err != nil {
 		return errors.Wrapf(err, "pebble: database %q", dirname)

valsep/value_separation_test.go

@@ -44,9 +44,9 @@ func TestValueSeparationPolicy(t *testing.T) {
 		})
 	)
 	ctx := context.Background()
-	objStore, err := objstorageprovider.Open(objstorageprovider.Settings{
-		FS: fs,
-	})
+	st := objstorageprovider.Settings{}
+	st.Local.FS = fs
+	objStore, err := objstorageprovider.Open(st)
 	require.NoError(t, err)
 
 	initRawWriter := func() {