From 639cd577e50f23e8d32b11dfce80ebbc39bce7d8 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Wed, 20 Dec 2023 15:03:19 -0800 Subject: [PATCH] db: add version checks around shared files We now require the `VirtualSSTables` format to use shared files. This helps with replication via shared ingest and excise, which requires V4 table format. We update the metamorphic tests to use the correct version when shared storage is enabled. --- format_major_version.go | 4 ++++ ingest.go | 6 ++++++ metamorphic/options.go | 17 ++++++++++++----- open.go | 21 +++++++++++++++++---- options.go | 8 ++++++++ scan_internal_test.go | 2 +- testdata/batch_reader | 1 - testdata/scan_statistics | 2 +- 8 files changed, 49 insertions(+), 12 deletions(-) diff --git a/format_major_version.go b/format_major_version.go index 6f8576dcf8..fc6c8dd17b 100644 --- a/format_major_version.go +++ b/format_major_version.go @@ -204,6 +204,10 @@ const ( // Pebble version. const FormatMinSupported = FormatFlushableIngest +// FormatMinForSharedObjects it the minimum format version that supports shared +// objects (see CreateOnShared option). +const FormatMinForSharedObjects = FormatVirtualSSTables + // IsSupported returns true if the version is supported by the current Pebble // version. func (v FormatMajorVersion) IsSupported() bool { diff --git a/ingest.go b/ingest.go index 2373b07907..2152a07557 100644 --- a/ingest.go +++ b/ingest.go @@ -1184,6 +1184,12 @@ func (d *DB) IngestAndExcise( panic("IngestAndExcise called with suffixed end key") } } + if v := d.FormatMajorVersion(); v < FormatMinForSharedObjects { + return IngestOperationStats{}, errors.Errorf( + "store has format major version %d; IngestAndExise requires at least %d", + v, FormatMinForSharedObjects, + ) + } return d.ingest(paths, ingestTargetLevel, shared, exciseSpan, nil /* external */) } diff --git a/metamorphic/options.go b/metamorphic/options.go index 646cc8540f..af82c75d14 100644 --- a/metamorphic/options.go +++ b/metamorphic/options.go @@ -459,11 +459,13 @@ func standardOptions() []*TestOptions { [Options] format_major_version=%s `, newestFormatMajorVersionToTest), - 27: ` + 27: fmt.Sprintf(` +[Options] + format_major_version=%s [TestOptions] shared_storage_enabled=true secondary_cache_enabled=true -`, +`, pebble.FormatMinForSharedObjects), } opts := make([]*TestOptions, len(stdOpts)) @@ -618,6 +620,9 @@ func RandomOptions( // 20% of time, enable shared storage. if rng.Intn(5) == 0 { testOpts.sharedStorageEnabled = true + if testOpts.Opts.FormatMajorVersion < pebble.FormatMinForSharedObjects { + testOpts.Opts.FormatMajorVersion = pebble.FormatMinForSharedObjects + } inMemShared := remote.NewInMem() testOpts.Opts.Experimental.RemoteStorage = remote.MakeSimpleFactory(map[remote.Locator]remote.Storage{ "": inMemShared, @@ -645,13 +650,15 @@ func RandomOptions( // testOpts.ingestSplit = rng.Intn(2) == 0 opts.Experimental.IngestSplit = func() bool { return testOpts.ingestSplit } testOpts.useExcise = rng.Intn(2) == 0 - if testOpts.useExcise || testOpts.useSharedReplicate { - testOpts.efosAlwaysCreatesIters = rng.Intn(2) == 0 - opts.TestingAlwaysCreateEFOSIterators(testOpts.efosAlwaysCreatesIters) + if testOpts.useExcise { if testOpts.Opts.FormatMajorVersion < pebble.FormatVirtualSSTables { testOpts.Opts.FormatMajorVersion = pebble.FormatVirtualSSTables } } + if testOpts.useExcise || testOpts.useSharedReplicate { + testOpts.efosAlwaysCreatesIters = rng.Intn(2) == 0 + opts.TestingAlwaysCreateEFOSIterators(testOpts.efosAlwaysCreatesIters) + } testOpts.Opts.EnsureDefaults() return testOpts } diff --git a/open.go b/open.go index 33616d1aa2..c8681d8d2f 100644 --- a/open.go +++ b/open.go @@ -28,6 +28,7 @@ import ( "github.com/cockroachdb/pebble/internal/manual" "github.com/cockroachdb/pebble/objstorage" "github.com/cockroachdb/pebble/objstorage/objstorageprovider" + "github.com/cockroachdb/pebble/objstorage/remote" "github.com/cockroachdb/pebble/record" "github.com/cockroachdb/pebble/sstable" "github.com/cockroachdb/pebble/vfs" @@ -138,6 +139,12 @@ func Open(dirname string, opts *Options) (db *DB, err error) { noFormatVersionMarker := formatVersion == FormatDefault if noFormatVersionMarker { + // We will initialize the store at the minimum possible format, then upgrade + // the format to the desired one. This helps test the format upgrade code. + formatVersion = FormatMinSupported + if opts.Experimental.CreateOnShared != remote.CreateOnSharedNone { + formatVersion = FormatMinForSharedObjects + } // There is no format version marker file. There are three cases: // - we are trying to open an existing store that was created at // FormatMostCompatible (the only one without a version marker file) @@ -146,14 +153,21 @@ func Open(dirname string, opts *Options) (db *DB, err error) { // // To error in the first case, we set ErrorIfNotPristine. opts.ErrorIfNotPristine = true - formatVersion = FormatMinSupported defer func() { if err != nil && errors.Is(err, ErrDBNotPristine) { // We must be trying to open an existing store at FormatMostCompatible. // Correct the error in this case -we - err = errors.Newf("pebble: database %q written in format major version 1 which is no longer supported", dirname) + err = errors.Newf( + "pebble: database %q written in format major version 1 which is no longer supported", + dirname) } }() + } else { + if opts.Experimental.CreateOnShared != remote.CreateOnSharedNone && formatVersion < FormatMinForSharedObjects { + return nil, errors.Newf( + "pebble: database %q configured with shared objects but written in too old format major version %d", + formatVersion) + } } // Find the currently active manifest, if there is one. @@ -517,8 +531,7 @@ func Open(dirname string, opts *Options) (db *DB, err error) { return nil, err } } else if noFormatVersionMarker { - // We are creating a new store at MinSupported. Create the format version - // marker file. + // We are creating a new store. Create the format version marker file. if err := d.writeFormatVersionMarker(d.FormatMajorVersion()); err != nil { return nil, err } diff --git a/options.go b/options.go index 3451375d44..7fabd9ea8c 100644 --- a/options.go +++ b/options.go @@ -1123,6 +1123,9 @@ func (o *Options) EnsureDefaults() *Options { if o.FormatMajorVersion == FormatDefault { o.FormatMajorVersion = FormatMinSupported + if o.Experimental.CreateOnShared != remote.CreateOnSharedNone { + o.FormatMajorVersion = FormatMinForSharedObjects + } } if o.FS == nil { @@ -1744,6 +1747,11 @@ func (o *Options) Validate() error { fmt.Fprintf(&buf, "FormatMajorVersion (%d) must be between %d and %d\n", o.FormatMajorVersion, FormatMinSupported, internalFormatNewest) } + if o.Experimental.CreateOnShared != remote.CreateOnSharedNone && o.FormatMajorVersion < FormatMinForSharedObjects { + fmt.Fprintf(&buf, "FormatMajorVersion (%d) when CreateOnShared is set must be at least %d\n", + o.FormatMajorVersion, FormatMinForSharedObjects) + + } if o.TableCache != nil && o.Cache != o.TableCache.cache { fmt.Fprintf(&buf, "underlying cache in the TableCache and the Cache dont match\n") } diff --git a/scan_internal_test.go b/scan_internal_test.go index 47964e930d..6087b2e01a 100644 --- a/scan_internal_test.go +++ b/scan_internal_test.go @@ -46,7 +46,7 @@ func TestScanStatistics(t *testing.T) { FS: vfs.NewMem(), Logger: testLogger{t: t}, Comparer: testkeys.Comparer, - FormatMajorVersion: FormatMinSupported, + FormatMajorVersion: FormatMinForSharedObjects, BlockPropertyCollectors: []func() BlockPropertyCollector{ sstable.NewTestKeysBlockPropertyCollector, }, diff --git a/testdata/batch_reader b/testdata/batch_reader index 01ea8a2ad0..f3ac59697c 100644 --- a/testdata/batch_reader +++ b/testdata/batch_reader @@ -95,4 +95,3 @@ scan ---- Count: 1 err: decoding user key: pebble: invalid batch - diff --git a/testdata/scan_statistics b/testdata/scan_statistics index a080099644..771e364c93 100644 --- a/testdata/scan_statistics +++ b/testdata/scan_statistics @@ -143,4 +143,4 @@ compact a-z scan-statistics lower=a upper=z show-snapshot-pinned ---- Aggregate: - snapshot pinned count: 1 + snapshot pinned count: 0