Repository: cockroachdb/pebble
Pull request #3690 (2024-06-24 16:45:40)

crl-release-24.1: provider: consult readahead config when initializing handle
Backport of #3668

#### Makefile: add gen-bazel target

This change adds a `gen-bazel` target which generates a `WORKSPACE`
file and runs gazelle to generate `BUILD.bazel `files. Generating
these files  allows a local clone of the repository to be used
directly when building Cockroach using the `override_repository` flag,
for example:

```
./dev build short -- --override_repository=com_github_cockroachdb_pebble=~/go/src/github.com/cockroachdb/pebble
```

The new files are added to `.gitignore` so that generating them
doesn't interfere with commits.

We also add a `clean-bazel` target that cleans up these files (useful
when switching back to an older branch).

#### provider: consult readahead config when initializing handle

We currently retrieve the readahead config when we create a `Readable`
and then we use that config for all read handles. This makes the
settings not responsive, as any tables already in the table cache will
have the `Readable` created.

We change `ReadaheadConfig` to an atomic that is loaded every time we
initialize a read handle.


cockroach-teamcity: 

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/cockroachdb/pebble/3690)
<!-- Reviewable:end -->

RaduBerinde: TFTRs!

Patch:
diff --git a/.gitignore b/.gitignore
index 87ef192f..7c353d25 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,3 +7,9 @@ mutex.prof
 coverprofile.out
 # Testing artifacts
 meta.*.test
+
+# Bazel files, generated with 'make gen-bazel'.
+/WORKSPACE
+BUILD.bazel
+
+
diff --git a/Makefile b/Makefile
index 31134566..c3eeb906 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,7 @@ all:
 	@echo "  make crossversion-meta"
 	@echo "  make testcoverage"
 	@echo "  make mod-update"
+	@echo "  make gen-bazel"
 	@echo "  make generate"
 	@echo "  make generate-test-data"
 	@echo "  make clean"
@@ -73,6 +74,19 @@ crossversion-meta:
 stress-crossversion:
 	STRESS=1 ./scripts/run-crossversion-meta.sh crl-release-22.1 crl-release-22.2 crl-release-23.1 crl-release-23.2 master
 
+.PHONY: gen-bazel
+gen-bazel:
+	@echo "Generating WORKSPACE"
+	@echo 'workspace(name = "com_github_cockroachdb_pebble")' > WORKSPACE
+	@echo 'Running gazelle...'
+	${GO} run github.com/bazelbuild/bazel-gazelle/cmd/gazelle@v0.37.0 update --go_prefix=github.com/cockroachdb/pebble --repo_root=.
+	@echo 'You should now be able to build Cockroach using:'
+	@echo '  ./dev build short -- --override_repository=com_github_cockroachdb_pebble=${CURDIR}'
+
+.PHONY: clean-bazel
+clean-bazel:
+	git clean -dxf WORKSPACE BUILD.bazel '**/BUILD.bazel'
+
 .PHONY: generate
 generate:
 	${GO} generate ${PKG}
diff --git a/objstorage/objstorageprovider/provider.go b/objstorage/objstorageprovider/provider.go
index ea22bbbf..336221da 100644
--- a/objstorage/objstorageprovider/provider.go
+++ b/objstorage/objstorageprovider/provider.go
@@ -11,6 +11,7 @@ import (
 	"os"
 	"slices"
 	"sync"
+	"sync/atomic"
 
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/errors/oserror"
@@ -92,10 +93,9 @@ type Settings struct {
 	Local struct {
 		// TODO(radu): move FSCleaner, NoSyncOnClose, BytesPerSync here.
 
-		// ReadaheadConfigFn is a function used to retrieve the current readahead
-		// mode. This function is run whenever a local object is open for reading.
-		// If it is nil, DefaultReadaheadConfig is used.
-		ReadaheadConfigFn func() ReadaheadConfig
+		// ReadaheadConfig is used to retrieve the current readahead mode; it is
+		// consulted whenever a read handle is initialized.
+		ReadaheadConfig *ReadaheadConfig
 	}
 
 	// Fields here are set only if the provider is to support remote objects
@@ -135,22 +135,49 @@ type Settings struct {
 	}
 }
 
-// ReadaheadConfig controls the use of read-ahead.
+// ReadaheadConfig is a container for the settings that control the use of
+// read-ahead.
+//
+// It stores two ReadaheadModes:
+//   - Informed is the type of read-ahead for operations that are known to read a
+//     large consecutive chunk of a file.
+//   - Speculative is the type of read-ahead used automatically, when consecutive
+//     reads are detected.
+//
+// The settings can be changed and read atomically.
 type ReadaheadConfig struct {
-	// Informed is the type of read-ahead for operations that are known to read a
-	// large consecutive chunk of a file.
-	Informed ReadaheadMode
+	value atomic.Uint32
+}
+
+// These are the default readahead modes when a config is not specified.
+const (
+	defaultReadaheadInformed    = FadviseSequential
+	defaultReadaheadSpeculative = FadviseSequential
+)
 
-	// Speculative is the type of read-ahead used automatically, when consecutive
-	// reads are detected.
-	Speculative ReadaheadMode
+// NewReadaheadConfig returns a new readahead config container initialized with
+// default values.
+func NewReadaheadConfig() *ReadaheadConfig {
+	rc := &ReadaheadConfig{}
+	rc.Set(defaultReadaheadInformed, defaultReadaheadSpeculative)
+	return rc
 }
 
-// DefaultReadaheadConfig is the readahead config used when ReadaheadConfigFn is
-// not specified.
-var DefaultReadaheadConfig = ReadaheadConfig{
-	Informed:    FadviseSequential,
-	Speculative: FadviseSequential,
+// Set the informed and speculative readahead modes.
+func (rc *ReadaheadConfig) Set(informed, speculative ReadaheadMode) {
+	rc.value.Store(uint32(speculative)<<8 | uint32(informed))
+}
+
+// Informed returns the type of read-ahead for operations that are known to read
+// a large consecutive chunk of a file.
+func (rc *ReadaheadConfig) Informed() ReadaheadMode {
+	return ReadaheadMode(rc.value.Load() & 0xff)
+}
+
+// Speculative returns the type of read-ahead used automatically, when
+// consecutive reads are detected.
+func (rc *ReadaheadConfig) Speculative() ReadaheadMode {
+	return ReadaheadMode(rc.value.Load() >> 8)
 }
 
 // ReadaheadMode indicates the type of read-ahead to use, either for informed
@@ -208,6 +235,10 @@ func open(settings Settings) (p *provider, _ error) {
 		}
 	}()
 
+	if settings.Local.ReadaheadConfig == nil {
+		settings.Local.ReadaheadConfig = NewReadaheadConfig()
+	}
+
 	p = &provider{
 		st:    settings,
 		fsDir: fsDir,
diff --git a/objstorage/objstorageprovider/vfs.go b/objstorage/objstorageprovider/vfs.go
index 9e23815a..7f73a237 100644
--- a/objstorage/objstorageprovider/vfs.go
+++ b/objstorage/objstorageprovider/vfs.go
@@ -31,11 +31,7 @@ func (p *provider) vfsOpenForReading(
 		}
 		return nil, err
 	}
-	readaheadConfig := DefaultReadaheadConfig
-	if f := p.st.Local.ReadaheadConfigFn; f != nil {
-		readaheadConfig = f()
-	}
-	return newFileReadable(file, p.st.FS, readaheadConfig, filename)
+	return newFileReadable(file, p.st.FS, p.st.Local.ReadaheadConfig, filename)
 }
 
 func (p *provider) vfsCreate(
diff --git a/objstorage/objstorageprovider/vfs_readable.go b/objstorage/objstorageprovider/vfs_readable.go
index 584f6a90..79527f12 100644
--- a/objstorage/objstorageprovider/vfs_readable.go
+++ b/objstorage/objstorageprovider/vfs_readable.go
@@ -25,17 +25,18 @@ type fileReadable struct {
 	file vfs.File
 	size int64
 
+	readaheadConfig *ReadaheadConfig
+
 	// The following fields are used to possibly open the file again using the
 	// sequential reads option (see vfsReadHandle).
-	filename        string
-	fs              vfs.FS
-	readaheadConfig ReadaheadConfig
+	filename string
+	fs       vfs.FS
 }
 
 var _ objstorage.Readable = (*fileReadable)(nil)
 
 func newFileReadable(
-	file vfs.File, fs vfs.FS, readaheadConfig ReadaheadConfig, filename string,
+	file vfs.File, fs vfs.FS, readaheadConfig *ReadaheadConfig, filename string,
 ) (*fileReadable, error) {
 	info, err := file.Stat()
 	if err != nil {
@@ -116,7 +117,7 @@ func (rh *vfsReadHandle) init(r *fileReadable) {
 	*rh = vfsReadHandle{
 		r:             r,
 		rs:            makeReadaheadState(fileMaxReadaheadSize),
-		readaheadMode: r.readaheadConfig.Speculative,
+		readaheadMode: r.readaheadConfig.Speculative(),
 	}
 }
 
@@ -161,7 +162,7 @@ func (rh *vfsReadHandle) ReadAt(_ context.Context, p []byte, offset int64) error
 
 // SetupForCompaction is part of the objstorage.ReadHandle interface.
 func (rh *vfsReadHandle) SetupForCompaction() {
-	rh.readaheadMode = rh.r.readaheadConfig.Informed
+	rh.readaheadMode = rh.r.readaheadConfig.Informed()
 	if rh.readaheadMode == FadviseSequential {
 		rh.switchToOSReadahead()
 	}
diff --git a/open.go b/open.go
index b2dd9c6f..dcd06a07 100644
--- a/open.go
+++ b/open.go
@@ -297,7 +297,7 @@ func Open(dirname string, opts *Options) (db *DB, err error) {
 		NoSyncOnClose:       opts.NoSyncOnClose,
 		BytesPerSync:        opts.BytesPerSync,
 	}
-	providerSettings.Local.ReadaheadConfigFn = opts.Local.ReadaheadConfigFn
+	providerSettings.Local.ReadaheadConfig = opts.Local.ReadaheadConfig
 	providerSettings.Remote.StorageFactory = opts.Experimental.RemoteStorage
 	providerSettings.Remote.CreateOnShared = opts.Experimental.CreateOnShared
 	providerSettings.Remote.CreateOnSharedLocator = opts.Experimental.CreateOnSharedLocator
diff --git a/options.go b/options.go
index eda2a6bb..03071d26 100644
--- a/options.go
+++ b/options.go
@@ -497,9 +497,9 @@ type Options struct {
 
 	// Local contains option that pertain to files stored on the local filesystem.
 	Local struct {
-		// ReadaheadConfigFn is a function used to retrieve the current readahead
-		// mode. This function is consulted when a table enters the table cache.
-		ReadaheadConfigFn func() ReadaheadConfig
+		// ReadaheadConfig is used to retrieve the current readahead mode; it is
+		// consulted whenever a read handle is initialized.
+		ReadaheadConfig *ReadaheadConfig
 
 		// TODO(radu): move BytesPerSync, LoadBlockSema, Cleaner here.
 	}


Test patch:
diff --git a/objstorage/objstorageprovider/provider_test.go b/objstorage/objstorageprovider/provider_test.go
index 1f0c0a69..b34949d0 100644
--- a/objstorage/objstorageprovider/provider_test.go
+++ b/objstorage/objstorageprovider/provider_test.go
@@ -40,9 +40,9 @@ func TestProvider(t *testing.T) {
 		backings := make(map[string]objstorage.RemoteObjectBacking)
 		backingHandles := make(map[string]objstorage.RemoteObjectBackingHandle)
 		var curProvider objstorage.Provider
-		readaheadConfig := DefaultReadaheadConfig
+		readaheadConfig := NewReadaheadConfig()
 		datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
-			readaheadConfig = DefaultReadaheadConfig
+			readaheadConfig.Set(defaultReadaheadInformed, defaultReadaheadSpeculative)
 			scanArgs := func(desc string, args ...interface{}) {
 				t.Helper()
 				if len(d.CmdArgs) != len(args) {
@@ -70,9 +70,7 @@ func TestProvider(t *testing.T) {
 					st.Remote.CreateOnShared = remote.CreateOnSharedAll
 					st.Remote.CreateOnSharedLocator = ""
 				}
-				st.Local.ReadaheadConfigFn = func() ReadaheadConfig {
-					return readaheadConfig
-				}
+				st.Local.ReadaheadConfig = readaheadConfig
 				require.NoError(t, fs.MkdirAll(fsDir, 0755))
 				p, err := Open(st)
 				require.NoError(t, err)
@@ -189,9 +187,9 @@ func TestProvider(t *testing.T) {
 						d.Fatalf(t, "unknown readahead mode %s", arg.Vals[0])
 					}
 					if forCompaction {
-						readaheadConfig.Informed = mode
+						readaheadConfig.Set(mode, defaultReadaheadSpeculative)
 					} else {
-						readaheadConfig.Speculative = mode
+						readaheadConfig.Set(defaultReadaheadInformed, mode)
 					}
 				}