Skip to content

metamorphic: remote storage fixes for crossversion #4760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion metamorphic/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ func openExternalObj(
rangeDelIter keyspan.FragmentIterator,
rangeKeyIter keyspan.FragmentIterator,
) {
objReader, objSize, err := t.externalStorage.ReadObject(context.Background(), externalObjName(externalObjID))
objMeta := t.getExternalObj(externalObjID)
objReader, objSize, err := t.externalStorage.ReadObject(context.Background(), objMeta.objName)
panicIfErr(err)
opts := t.opts.MakeReaderOptions()
reader, err = sstable.NewReader(
Expand Down
36 changes: 20 additions & 16 deletions metamorphic/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ package metamorphic
import (
"bytes"
"context"
"crypto/rand"
cryptorand "crypto/rand"
"encoding/binary"
"fmt"
"io"
"math/rand/v2"
"path"
"path/filepath"
"slices"
Expand Down Expand Up @@ -1053,7 +1054,7 @@ func (o *ingestExternalFilesOp) run(t *Test, h historyRecorder) {
meta := t.getExternalObj(obj.externalObjID)
external[i] = pebble.ExternalFile{
Locator: "external",
ObjName: externalObjName(obj.externalObjID),
ObjName: meta.objName,
Size: meta.sstMeta.Size,
StartKey: obj.bounds.Start,
EndKey: obj.bounds.End,
Expand Down Expand Up @@ -1199,8 +1200,8 @@ func (o *newIterOp) run(t *Test, h historyRecorder) {
// Trash the bounds to ensure that Pebble doesn't rely on the stability of
// the user-provided bounds.
if opts != nil {
rand.Read(opts.LowerBound[:])
rand.Read(opts.UpperBound[:])
cryptorand.Read(opts.LowerBound[:])
cryptorand.Read(opts.UpperBound[:])
}
h.Recordf("%s // %v", o.formattedString(t.testOpts.KeyFormat), i.Error())
}
Expand Down Expand Up @@ -1318,8 +1319,8 @@ func (o *iterSetBoundsOp) run(t *Test, h historyRecorder) {

// Trash the bounds to ensure that Pebble doesn't rely on the stability of
// the user-provided bounds.
rand.Read(lower[:])
rand.Read(upper[:])
cryptorand.Read(lower[:])
cryptorand.Read(upper[:])

h.Recordf("%s // %v", o.formattedString(t.testOpts.KeyFormat), i.Error())
}
Expand Down Expand Up @@ -1363,8 +1364,8 @@ func (o *iterSetOptionsOp) run(t *Test, h historyRecorder) {

// Trash the bounds to ensure that Pebble doesn't rely on the stability of
// the user-provided bounds.
rand.Read(opts.LowerBound[:])
rand.Read(opts.UpperBound[:])
cryptorand.Read(opts.LowerBound[:])
cryptorand.Read(opts.UpperBound[:])

h.Recordf("%s // %v", o.formattedString(t.testOpts.KeyFormat), i.Error())
}
Expand Down Expand Up @@ -1872,18 +1873,20 @@ type newExternalObjOp struct {
externalObjID objID
}

func externalObjName(externalObjID objID) string {
if externalObjID.tag() != externalObjTag {
panic(fmt.Sprintf("invalid externalObjID %s", externalObjID))
}
return fmt.Sprintf("external-for-ingest-%d.sst", externalObjID.slot())
}

func (o *newExternalObjOp) run(t *Test, h historyRecorder) {
b := t.getBatch(o.batchID)
t.clearObj(o.batchID)

writeCloser, err := t.externalStorage.CreateObject(externalObjName(o.externalObjID))
if o.externalObjID.tag() != externalObjTag {
panic(fmt.Sprintf("invalid externalObjID %s", o.externalObjID))
}
// We add a unique number to the object name to avoid collisions with existing
// external objects (when using an initial starting state).
//
// Note that the number is not based on the seed, in case we run using the
// same seed that was used in a previous run with the same store.
objName := fmt.Sprintf("external-for-ingest-%d-%d.sst", o.externalObjID.slot(), rand.Uint64())
writeCloser, err := t.externalStorage.CreateObject(objName)
if err != nil {
panic(err)
}
Expand All @@ -1908,6 +1911,7 @@ func (o *newExternalObjOp) run(t *Test, h historyRecorder) {
panic("metamorphic test internal error: external object empty")
}
t.setExternalObj(o.externalObjID, externalObjMeta{
objName: objName,
sstMeta: sstMeta,
})
h.Recordf("%s", o.formattedString(t.testOpts.KeyFormat))
Expand Down
22 changes: 0 additions & 22 deletions metamorphic/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,12 @@ func parseOptions(
return true
case "TestOptions.shared_storage_enabled":
opts.sharedStorageEnabled = true
opts.sharedStorageFS = remote.NewInMem()
if opts.Opts.Experimental.CreateOnShared == remote.CreateOnSharedNone {
opts.Opts.Experimental.CreateOnShared = remote.CreateOnSharedAll
}
return true
case "TestOptions.external_storage_enabled":
opts.externalStorageEnabled = true
opts.externalStorageFS = remote.NewInMem()
return true
case "TestOptions.secondary_cache_enabled":
opts.secondaryCacheEnabled = true
Expand Down Expand Up @@ -226,7 +224,6 @@ func parseOptions(
if opts.Opts.WALFailover != nil {
opts.Opts.WALFailover.Secondary.FS = opts.Opts.FS
}
opts.InitRemoteStorageFactory()
opts.Opts.EnsureDefaults()
return err
}
Expand Down Expand Up @@ -435,10 +432,8 @@ type TestOptions struct {
asyncApplyToDB bool
// Enable the use of shared storage.
sharedStorageEnabled bool
sharedStorageFS remote.Storage
// Enable the use of shared storage for external file ingestion.
externalStorageEnabled bool
externalStorageFS remote.Storage
// Enables the use of shared replication in TestOptions.
useSharedReplicate bool
// Enables the use of external replication in TestOptions.
Expand Down Expand Up @@ -473,20 +468,6 @@ type TestOptions struct {
disableDownloads bool
}

// InitRemoteStorageFactory initializes Opts.Experimental.RemoteStorage.
func (testOpts *TestOptions) InitRemoteStorageFactory() {
if testOpts.sharedStorageEnabled || testOpts.externalStorageEnabled {
m := make(map[remote.Locator]remote.Storage)
if testOpts.sharedStorageEnabled {
m[""] = testOpts.sharedStorageFS
}
if testOpts.externalStorageEnabled {
m["external"] = testOpts.externalStorageFS
}
testOpts.Opts.Experimental.RemoteStorage = remote.MakeSimpleFactory(m)
}
}

// CustomOption defines a custom option that configures the behavior of an
// individual test run. Like all test options, custom options are serialized to
// the OPTIONS file even if they're not options ordinarily understood by Pebble.
Expand Down Expand Up @@ -883,7 +864,6 @@ func RandomOptions(
if testOpts.Opts.FormatMajorVersion < pebble.FormatMinForSharedObjects {
testOpts.Opts.FormatMajorVersion = pebble.FormatMinForSharedObjects
}
testOpts.sharedStorageFS = remote.NewInMem()
// If shared storage is enabled, pick between writing all files on shared
// vs. lower levels only, 50% of the time.
testOpts.Opts.Experimental.CreateOnShared = remote.CreateOnSharedAll
Expand All @@ -906,7 +886,6 @@ func RandomOptions(
if testOpts.Opts.FormatMajorVersion < pebble.FormatSyntheticPrefixSuffix {
testOpts.Opts.FormatMajorVersion = pebble.FormatSyntheticPrefixSuffix
}
testOpts.externalStorageFS = remote.NewInMem()
}

// 75% of the time, randomize value separation parameters.
Expand Down Expand Up @@ -939,7 +918,6 @@ func RandomOptions(
return testOpts.useDeleteOnlyCompactionExcises
}
testOpts.disableDownloads = rng.IntN(2) == 0
testOpts.InitRemoteStorageFactory()
testOpts.Opts.EnsureDefaults()
return testOpts
}
Expand Down
Loading