Merge #45512

45512: server: Add EngineTypeDefault, resolve it to Pebble if last used r=itsbilal a=itsbilal

Adds a new engine type called EngineTypeDefault, that resolves
at start time to RocksDB, or Pebble if that was last used
to write to the first store. This effectively makes the
--storage-engine flag sticky for future runs where no
engine is specified. If the flag is passed in again, that
specified engine type takes priority over the one used last.

Fixes #42445 .

Release note (cli change): Add new "default" option for
--storage-engine flag that respects the engine used last.


Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>

pkg/cli/cliflags/flags.go

@@ -715,8 +715,12 @@ Also, if you use equal signs in the file path to a store, you must use the
 	StorageEngine = FlagInfo{
 		Name: "storage-engine",
 		Description: `
-Storage engine to use for all stores on this cockroach node. Options are rocksdb,
-or pebble.`,
+Storage engine to use for all stores on this cockroach node. Options are default,
+rocksdb, or pebble.
+
+If default is specified, the storage engine last used to write to the first
+store directory is used (see --store). If the store directory is uninitialized
+and default is specified, rocksdb is used as the default storage engine.`,
 	}
 
 	Size = FlagInfo{

pkg/cli/debug.go

@@ -140,8 +140,9 @@ func OpenEngine(dir string, stopper *stop.Stopper, opts OpenEngineOptions) (engi
 	}
 
 	var db engine.Engine
+	storageEngine := resolveStorageEngineType(engine.DefaultStorageEngine, storageConfig.Dir)
 
-	switch engine.DefaultStorageEngine {
+	switch storageEngine {
 	case enginepb.EngineTypePebble:
 		cfg := engine.PebbleConfig{
 			StorageConfig: storageConfig,

pkg/cli/start.go

@@ -39,6 +39,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/status"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage/engine"
+	"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/grpcutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -51,6 +52,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/sysutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
+	"github.com/cockroachdb/pebble"
+	"github.com/cockroachdb/pebble/vfs"
 	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -407,6 +410,23 @@ func initTempStorageConfig(
 	return tempStorageConfig, nil
 }
 
+// Checks if the passed-in engine type is default, and if so, resolves it to
+// the storage engine last used to write to the store at dir (or rocksdb if
+// a store wasn't found).
+func resolveStorageEngineType(engineType enginepb.EngineType, dir string) enginepb.EngineType {
+	if engineType == enginepb.EngineTypeDefault {
+		engineType = enginepb.EngineTypeRocksDB
+		// Check if this storage directory was last written to by pebble. In that
+		// case, default to opening a Pebble engine.
+		if version, err := pebble.GetVersion(dir, vfs.Default); err == nil {
+			if version != "" && !strings.HasPrefix(version, "rocksdb") {
+				engineType = enginepb.EngineTypePebble
+			}
+		}
+	}
+	return engineType
+}
+
 var errCannotUseJoin = errors.New("cannot use --join with 'cockroach start-single-node' -- use 'cockroach start' instead")
 
 func runStartSingleNode(cmd *cobra.Command, args []string) error {
@@ -530,6 +550,10 @@ func runStart(cmd *cobra.Command, args []string, disableReplication bool) error
 	if serverCfg.Settings.ExternalIODir, err = initExternalIODir(ctx, serverCfg.Stores.Specs[0]); err != nil {
 		return err
 	}
+	// If the storage engine is set to "default", check the engine type used in
+	// this store directory in a past run. If this check fails for any reason,
+	// use RocksDB as the default engine type.
+	serverCfg.StorageEngine = resolveStorageEngineType(serverCfg.StorageEngine, serverCfg.Stores.Specs[0].Path)
 	// Find a StoreSpec that has encryption at rest turned on. If can't find
 	// one, use the first StoreSpec in the list.
 	var specIdx = 0

pkg/server/config.go

@@ -437,7 +437,9 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) {
 		details = append(details, fmt.Sprintf("Pebble cache size: %s", humanizeutil.IBytes(cfg.CacheSize)))
 		pebbleCache = pebble.NewCache(cfg.CacheSize)
 		defer pebbleCache.Unref()
-	} else if cfg.StorageEngine == enginepb.EngineTypeRocksDB || cfg.StorageEngine == enginepb.EngineTypeTeePebbleRocksDB {
+	}
+	if cfg.StorageEngine == enginepb.EngineTypeDefault ||
+		cfg.StorageEngine == enginepb.EngineTypeRocksDB || cfg.StorageEngine == enginepb.EngineTypeTeePebbleRocksDB {
 		details = append(details, fmt.Sprintf("RocksDB cache size: %s", humanizeutil.IBytes(cfg.CacheSize)))
 		cache = engine.NewRocksDBCache(cfg.CacheSize)
 		defer cache.Release()
@@ -522,7 +524,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) {
 				pebbleConfig.Opts.Cache = pebbleCache
 				pebbleConfig.Opts.MaxOpenFiles = int(openFileLimitPerStore)
 				eng, err = engine.NewPebble(ctx, pebbleConfig)
-			} else if cfg.StorageEngine == enginepb.EngineTypeRocksDB {
+			} else if cfg.StorageEngine == enginepb.EngineTypeRocksDB || cfg.StorageEngine == enginepb.EngineTypeDefault {
 				rocksDBConfig := engine.RocksDBConfig{
 					StorageConfig:           storageConfig,
 					MaxOpenFiles:            openFileLimitPerStore,

pkg/server/server.go

@@ -1749,6 +1749,8 @@ func (s *Server) Start(ctx context.Context) error {
 	switch s.cfg.StorageEngine {
 	case enginepb.EngineTypePebble:
 		nodeStartCounter += "pebble."
+	case enginepb.EngineTypeDefault:
+		nodeStartCounter += "default."
 	case enginepb.EngineTypeRocksDB:
 		nodeStartCounter += "rocksdb."
 	case enginepb.EngineTypeTeePebbleRocksDB:

pkg/storage/below_raft_protos_test.go

@@ -120,7 +120,7 @@ var belowRaftGoldenProtos = map[reflect.Type]fixture{
 }
 
 func init() {
-	if engine.DefaultStorageEngine != enginepb.EngineTypeRocksDB {
+	if engine.DefaultStorageEngine != enginepb.EngineTypeRocksDB && engine.DefaultStorageEngine != enginepb.EngineTypeDefault {
 		// This is marshaled below Raft by the Pebble merge operator. The Pebble
 		// merge operator can be called below Raft whenever a Pebble Iterator is
 		// used. Note that we only see this proto marshaled below Raft when the

pkg/storage/engine/engine.go

@@ -34,7 +34,7 @@ import (
 var DefaultStorageEngine enginepb.EngineType
 
 func init() {
-	_ = DefaultStorageEngine.Set(envutil.EnvOrDefaultString("COCKROACH_STORAGE_ENGINE", "rocksdb"))
+	_ = DefaultStorageEngine.Set(envutil.EnvOrDefaultString("COCKROACH_STORAGE_ENGINE", "default"))
 }
 
 // SimpleIterator is an interface for iterating over key/value pairs in an
@@ -555,7 +555,7 @@ func NewEngine(
 		defer pebbleConfig.Opts.Cache.Unref()
 
 		return NewPebble(context.Background(), pebbleConfig)
-	case enginepb.EngineTypeRocksDB:
+	case enginepb.EngineTypeDefault, enginepb.EngineTypeRocksDB:
 		cache := NewRocksDBCache(cacheSize)
 		defer cache.Release()
 

pkg/storage/engine/enginepb/engine.go

@@ -18,6 +18,8 @@ func (e *EngineType) Type() string { return "string" }
 // String implements the pflag.Value interface.
 func (e *EngineType) String() string {
 	switch *e {
+	case EngineTypeDefault:
+		return "default"
 	case EngineTypeRocksDB:
 		return "rocksdb"
 	case EngineTypePebble:
@@ -31,6 +33,8 @@ func (e *EngineType) String() string {
 // Set implements the pflag.Value interface.
 func (e *EngineType) Set(s string) error {
 	switch s {
+	case "default":
+		*e = EngineTypeDefault
 	case "rocksdb":
 		*e = EngineTypeRocksDB
 	case "pebble":

pkg/storage/engine/enginepb/engine.proto

@@ -19,11 +19,15 @@ enum EngineType {
   option (gogoproto.goproto_enum_prefix) = false;
   option (gogoproto.goproto_enum_stringer) = false;
 
+  // Denotes the default engine as the underlying storage engine type. Resolves
+  // during start to the engine type last used. If left unresolved, it's treated
+  // the same as EngineTypeRocksDB.
+  EngineTypeDefault = 0;
   // Denotes RocksDB as the underlying storage engine type.
-  EngineTypeRocksDB = 0;
+  EngineTypeRocksDB = 1;
   // Denotes Pebble as the underlying storage engine type.
-  EngineTypePebble = 1;
+  EngineTypePebble = 2;
   // Denotes TeePebbleRocksDB as the underlying storage engine type. Only use
   // for testing purposes.
-  EngineTypeTeePebbleRocksDB = 2;
+  EngineTypeTeePebbleRocksDB = 3;
 }

pkg/storage/engine/in_mem.go

@@ -30,7 +30,7 @@ func NewInMem(
 		return newTeeInMem(ctx, attrs, cacheSize)
 	case enginepb.EngineTypePebble:
 		return newPebbleInMem(ctx, attrs, cacheSize)
-	case enginepb.EngineTypeRocksDB:
+	case enginepb.EngineTypeDefault, enginepb.EngineTypeRocksDB:
 		return newRocksDBInMem(attrs, cacheSize)
 	}
 	panic(fmt.Sprintf("unknown engine type: %d", engine))

pkg/storage/engine/temp_engine.go

@@ -37,7 +37,7 @@ func NewTempEngine(
 		fallthrough
 	case enginepb.EngineTypePebble:
 		return NewPebbleTempEngine(ctx, tempStorage, storeSpec)
-	case enginepb.EngineTypeRocksDB:
+	case enginepb.EngineTypeDefault, enginepb.EngineTypeRocksDB:
 		return NewRocksDBTempEngine(tempStorage, storeSpec)
 	}
 	panic(fmt.Sprintf("unknown engine type: %d", engine))