From 58b1de3ceeb36f95b1f53c83b0485c14fffa52ce Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Tue, 28 Apr 2020 21:23:38 -0400 Subject: [PATCH] storage: change default engine type from Pebble to RocksDB This flips the switch to use Pebble by default. This will cause all tests which don't explicitly specify RocksDB (i.e. the vast majority) to now use Pebble. One exception is certain roachtests which create store directories using older versions of CRDB and then upgrade to the current version. Those tests will continue to use RocksDB (for now) due to the nature of how `--storage-engine=...` is "sticky" if not specified, defaulting to the previously used storage engine. Release note (general change): Change the default engine type for new storage directories from RocksDB to Pebble. Existing stores will continue to use the previously specified storage engine, and an explicit specification (via `--storage-engine=...`) will override the default. --- pkg/cli/start.go | 6 +++--- pkg/server/config.go | 10 +++++----- pkg/storage/engine.go | 4 ++-- pkg/storage/in_mem.go | 4 ++-- pkg/storage/temp_engine.go | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/cli/start.go b/pkg/cli/start.go index 50e8f0909c3a..a4b668579ddc 100644 --- a/pkg/cli/start.go +++ b/pkg/cli/start.go @@ -417,12 +417,12 @@ func initTempStorageConfig( // a store wasn't found). func resolveStorageEngineType(engineType enginepb.EngineType, dir string) enginepb.EngineType { if engineType == enginepb.EngineTypeDefault { - engineType = enginepb.EngineTypeRocksDB + engineType = enginepb.EngineTypePebble // 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 + if strings.HasPrefix(version, "rocksdb") { + engineType = enginepb.EngineTypeRocksDB } } } diff --git a/pkg/server/config.go b/pkg/server/config.go index 39e54458da79..96d8086242d5 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -437,13 +437,13 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { var cache storage.RocksDBCache var pebbleCache *pebble.Cache - if cfg.StorageEngine == enginepb.EngineTypePebble || cfg.StorageEngine == enginepb.EngineTypeTeePebbleRocksDB { + if cfg.StorageEngine == enginepb.EngineTypeDefault || + cfg.StorageEngine == enginepb.EngineTypePebble || cfg.StorageEngine == enginepb.EngineTypeTeePebbleRocksDB { details = append(details, fmt.Sprintf("Pebble cache size: %s", humanizeutil.IBytes(cfg.CacheSize))) pebbleCache = pebble.NewCache(cfg.CacheSize) defer pebbleCache.Unref() } - if cfg.StorageEngine == enginepb.EngineTypeDefault || - cfg.StorageEngine == enginepb.EngineTypeRocksDB || cfg.StorageEngine == enginepb.EngineTypeTeePebbleRocksDB { + if cfg.StorageEngine == enginepb.EngineTypeRocksDB || cfg.StorageEngine == enginepb.EngineTypeTeePebbleRocksDB { details = append(details, fmt.Sprintf("RocksDB cache size: %s", humanizeutil.IBytes(cfg.CacheSize))) cache = storage.NewRocksDBCache(cfg.CacheSize) defer cache.Release() @@ -518,7 +518,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { UseFileRegistry: spec.UseFileRegistry, ExtraOptions: spec.ExtraOptions, } - if cfg.StorageEngine == enginepb.EngineTypePebble { + if cfg.StorageEngine == enginepb.EngineTypePebble || cfg.StorageEngine == enginepb.EngineTypeDefault { // TODO(itsbilal): Tune these options, and allow them to be overridden // in the spec (similar to the existing spec.RocksDBOptions and others). pebbleConfig := storage.PebbleConfig{ @@ -528,7 +528,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { pebbleConfig.Opts.Cache = pebbleCache pebbleConfig.Opts.MaxOpenFiles = int(openFileLimitPerStore) eng, err = storage.NewPebble(ctx, pebbleConfig) - } else if cfg.StorageEngine == enginepb.EngineTypeRocksDB || cfg.StorageEngine == enginepb.EngineTypeDefault { + } else if cfg.StorageEngine == enginepb.EngineTypeRocksDB { rocksDBConfig := storage.RocksDBConfig{ StorageConfig: storageConfig, MaxOpenFiles: openFileLimitPerStore, diff --git a/pkg/storage/engine.go b/pkg/storage/engine.go index 8767c8df8b6b..d268786247db 100644 --- a/pkg/storage/engine.go +++ b/pkg/storage/engine.go @@ -564,7 +564,7 @@ func NewEngine( } return NewTee(ctx, rocksDB, pebbleDB), nil - case enginepb.EngineTypePebble: + case enginepb.EngineTypeDefault, enginepb.EngineTypePebble: pebbleConfig := PebbleConfig{ StorageConfig: storageConfig, Opts: DefaultPebbleOptions(), @@ -573,7 +573,7 @@ func NewEngine( defer pebbleConfig.Opts.Cache.Unref() return NewPebble(context.Background(), pebbleConfig) - case enginepb.EngineTypeDefault, enginepb.EngineTypeRocksDB: + case enginepb.EngineTypeRocksDB: cache := NewRocksDBCache(cacheSize) defer cache.Release() diff --git a/pkg/storage/in_mem.go b/pkg/storage/in_mem.go index dda9617f6709..93e38aeab8df 100644 --- a/pkg/storage/in_mem.go +++ b/pkg/storage/in_mem.go @@ -28,9 +28,9 @@ func NewInMem( switch engine { case enginepb.EngineTypeTeePebbleRocksDB: return newTeeInMem(ctx, attrs, cacheSize) - case enginepb.EngineTypePebble: + case enginepb.EngineTypeDefault, enginepb.EngineTypePebble: return newPebbleInMem(ctx, attrs, cacheSize) - case enginepb.EngineTypeDefault, enginepb.EngineTypeRocksDB: + case enginepb.EngineTypeRocksDB: return newRocksDBInMem(attrs, cacheSize) } panic(fmt.Sprintf("unknown engine type: %d", engine)) diff --git a/pkg/storage/temp_engine.go b/pkg/storage/temp_engine.go index c299768ffbf6..c6601ca91a5d 100644 --- a/pkg/storage/temp_engine.go +++ b/pkg/storage/temp_engine.go @@ -35,9 +35,9 @@ func NewTempEngine( switch engine { case enginepb.EngineTypeTeePebbleRocksDB: fallthrough - case enginepb.EngineTypePebble: + case enginepb.EngineTypeDefault, enginepb.EngineTypePebble: return NewPebbleTempEngine(ctx, tempStorage, storeSpec) - case enginepb.EngineTypeDefault, enginepb.EngineTypeRocksDB: + case enginepb.EngineTypeRocksDB: return NewRocksDBTempEngine(tempStorage, storeSpec) } panic(fmt.Sprintf("unknown engine type: %d", engine))