Merge #148631

148631: storage: disable short attribute extractor in temp engine r=RaduBerinde a=jbowens

Disable the short attribute extractor in temporary engines. The short attribute extractor is only capable of parsing Cockroach MVCC keys. Additionally, rename some temp engine identifiers that needlessly were prefixed with 'Pebble'. These were vestiges from when Pebble and RocksDB implementations co-existed.

Epic: none
Release note: none

Co-authored-by: Jackson Owens <jackson@cockroachlabs.com>

Author: craig[bot]

pkg/storage/disk_map_test.go

@@ -40,7 +40,7 @@ func runTestForEngine(ctx context.Context, t *testing.T, filename string, engine
 			// creation and usage code involves too much test-specific code, so use
 			// a type switch with implementation-specific code instead.
 			switch e := engine.(type) {
-			case *pebbleTempEngine:
+			case *tempEngine:
 				iter, err := e.db.NewIter(&pebble.IterOptions{UpperBound: roachpb.KeyMax})
 				if err != nil {
 					t.Fatal(err)
@@ -167,14 +167,14 @@ func runTestForEngine(ctx context.Context, t *testing.T, filename string, engine
 	})
 }
 
-func TestPebbleMap(t *testing.T) {
+func TestTempEngineMap(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 	ctx := context.Background()
 	dir, cleanup := testutils.TempDir(t)
 	defer cleanup()
 
-	e, _, err := NewPebbleTempEngine(ctx, base.TempStorageConfig{
+	e, _, err := NewTempEngine(ctx, base.TempStorageConfig{
 		Path:     dir,
 		Settings: cluster.MakeClusterSettings(),
 	}, base.StoreSpec{}, disk.NewWriteStatsManager(vfs.Default))
@@ -193,7 +193,7 @@ func TestPebbleMultiMap(t *testing.T) {
 	dir, cleanup := testutils.TempDir(t)
 	defer cleanup()
 
-	e, _, err := NewPebbleTempEngine(ctx, base.TempStorageConfig{
+	e, _, err := NewTempEngine(ctx, base.TempStorageConfig{
 		Path:     dir,
 		Settings: cluster.MakeClusterSettings(),
 	}, base.StoreSpec{}, disk.NewWriteStatsManager(vfs.Default))
@@ -212,7 +212,7 @@ func TestPebbleMapClose(t *testing.T) {
 	ctx := context.Background()
 	dir, cleanup := testutils.TempDir(t)
 	defer cleanup()
-	e, _, err := newPebbleTempEngine(ctx, base.TempStorageConfig{
+	e, _, err := newTempEngine(ctx, base.TempStorageConfig{
 		Path:     dir,
 		Settings: cluster.MakeClusterSettings(),
 	}, base.StoreSpec{}, disk.NewWriteStatsManager(vfs.Default))
@@ -319,7 +319,7 @@ func TestPebbleMapClose(t *testing.T) {
 func BenchmarkPebbleMapWrite(b *testing.B) {
 	dir := b.TempDir()
 	ctx := context.Background()
-	tempEngine, _, err := NewPebbleTempEngine(ctx, base.TempStorageConfig{
+	tempEngine, _, err := NewTempEngine(ctx, base.TempStorageConfig{
 		Path:     dir,
 		Settings: cluster.MakeClusterSettings(),
 	}, base.DefaultTestStoreSpec, disk.NewWriteStatsManager(vfs.Default))
@@ -360,7 +360,7 @@ func BenchmarkPebbleMapIteration(b *testing.B) {
 	skip.UnderShort(b)
 	dir := b.TempDir()
 	ctx := context.Background()
-	tempEngine, _, err := NewPebbleTempEngine(ctx, base.TempStorageConfig{
+	tempEngine, _, err := NewTempEngine(ctx, base.TempStorageConfig{
 		Path:     dir,
 		Settings: cluster.MakeClusterSettings(),
 	}, base.DefaultTestStoreSpec, disk.NewWriteStatsManager(vfs.Default))

pkg/storage/temp_engine.go

@@ -26,49 +26,38 @@ func NewTempEngine(
 	storeSpec base.StoreSpec,
 	diskWriteStats disk.WriteStatsManager,
 ) (diskmap.Factory, vfs.FS, error) {
-	return NewPebbleTempEngine(ctx, tempStorage, storeSpec, diskWriteStats)
+	return newTempEngine(ctx, tempStorage, storeSpec, diskWriteStats)
 }
 
-type pebbleTempEngine struct {
+type tempEngine struct {
 	db  *pebble.DB
 	env *fs.Env
 }
 
 // Close implements the diskmap.Factory interface.
-func (r *pebbleTempEngine) Close() {
+func (r *tempEngine) Close() {
 	if err := r.db.Close(); err != nil {
 		log.Fatalf(context.TODO(), "%v", err)
 	}
 	r.env.Close()
 }
 
 // NewSortedDiskMap implements the diskmap.Factory interface.
-func (r *pebbleTempEngine) NewSortedDiskMap() diskmap.SortedDiskMap {
+func (r *tempEngine) NewSortedDiskMap() diskmap.SortedDiskMap {
 	return newPebbleMap(r.db, false /* allowDuplications */)
 }
 
 // NewSortedDiskMultiMap implements the diskmap.Factory interface.
-func (r *pebbleTempEngine) NewSortedDiskMultiMap() diskmap.SortedDiskMap {
+func (r *tempEngine) NewSortedDiskMultiMap() diskmap.SortedDiskMap {
 	return newPebbleMap(r.db, true /* allowDuplicates */)
 }
 
-// NewPebbleTempEngine creates a new Pebble engine for DistSQL processors to use
-// when the working set is larger than can be stored in memory.
-func NewPebbleTempEngine(
+func newTempEngine(
 	ctx context.Context,
 	tempStorage base.TempStorageConfig,
 	storeSpec base.StoreSpec,
 	diskWriteStats disk.WriteStatsManager,
-) (diskmap.Factory, vfs.FS, error) {
-	return newPebbleTempEngine(ctx, tempStorage, storeSpec, diskWriteStats)
-}
-
-func newPebbleTempEngine(
-	ctx context.Context,
-	tempStorage base.TempStorageConfig,
-	storeSpec base.StoreSpec,
-	diskWriteStats disk.WriteStatsManager,
-) (*pebbleTempEngine, vfs.FS, error) {
+) (*tempEngine, vfs.FS, error) {
 	var baseFS vfs.FS
 	var dir string
 	var cacheSize int64 = 128 << 20 // 128 MiB, arbitrary, but not "too big"
@@ -110,6 +99,7 @@ func newPebbleTempEngine(
 			cfg.opts.KeySchema = ""
 			cfg.opts.DisableWAL = true
 			cfg.opts.Experimental.UserKeyCategories = pebble.UserKeyCategories{}
+			cfg.opts.Experimental.ShortAttributeExtractor = nil
 			cfg.opts.Experimental.SpanPolicyFunc = nil
 			cfg.opts.BlockPropertyCollectors = nil
 			cfg.opts.EnableSQLRowSpillMetrics = true
@@ -124,7 +114,7 @@ func newPebbleTempEngine(
 	// Set store ID for the pebble engine. We are not using shared storage for
 	// temp stores so this cannot error out.
 	_ = p.SetStoreID(ctx, base.TempStoreID)
-	return &pebbleTempEngine{
+	return &tempEngine{
 		db:  p.db,
 		env: env,
 	}, env, nil

pkg/storage/temp_engine_test.go

@@ -19,7 +19,7 @@ import (
 	"github.com/cockroachdb/pebble/vfs"
 )
 
-func TestNewPebbleTempEngine(t *testing.T) {
+func TestNewTempEngine(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
@@ -28,12 +28,12 @@ func TestNewPebbleTempEngine(t *testing.T) {
 	defer tempDirCleanup()
 
 	diskWriteStats := disk.NewWriteStatsManager(vfs.Default)
-	db, filesystem, err := NewPebbleTempEngine(context.Background(), base.TempStorageConfig{
+	db, filesystem, err := NewTempEngine(context.Background(), base.TempStorageConfig{
 		Path:     tempDir,
 		Settings: cluster.MakeTestingClusterSettings(),
 	}, base.StoreSpec{Path: tempDir}, diskWriteStats)
 	if err != nil {
-		t.Fatalf("error encountered when invoking NewRocksDBTempEngine: %+v", err)
+		t.Fatalf("error encountered when invoking NewTempEngine: %+v", err)
 	}
 	defer db.Close()