diff --git a/pkg/server/config.go b/pkg/server/config.go index 9d17b2b03a93..e4bde5252d70 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -484,6 +484,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { log.Eventf(ctx, "initializing %+v", spec) var sizeInBytes = spec.Size.InBytes if spec.InMemory { + log.Infof(ctx, "cache sizeInBytes: %d, percent: %f", sizeInBytes, spec.Size.Percent) if spec.Size.Percent > 0 { sysMem, err := status.GetTotalMemory(ctx) if err != nil { @@ -516,6 +517,8 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { engines = append(engines, e) } else { engines = append(engines, storage.NewInMem(ctx, spec.Attributes, sizeInBytes, cfg.Settings)) + // engines = append(engines, + // storage.NewInMemWithCache(ctx, spec.Attributes, pebbleCache, cfg.Settings)) } } else { if spec.Size.Percent > 0 { @@ -557,6 +560,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { if len(spec.RocksDBOptions) > 0 { return nil, errors.Errorf("store %d: using Pebble storage engine but StoreSpec provides RocksDB options", i) } + log.Infof(ctx, "calling NewPebble with cache") eng, err := storage.NewPebble(ctx, pebbleConfig) if err != nil { return Engines{}, err diff --git a/pkg/server/settingswatcher/BUILD.bazel b/pkg/server/settingswatcher/BUILD.bazel index 556e8f83d5e6..045134fa2b77 100644 --- a/pkg/server/settingswatcher/BUILD.bazel +++ b/pkg/server/settingswatcher/BUILD.bazel @@ -48,6 +48,7 @@ go_test( "//pkg/settings", "//pkg/settings/cluster", "//pkg/sql", + "//pkg/storage", "//pkg/testutils", "//pkg/testutils/serverutils", "//pkg/testutils/sqlutils", diff --git a/pkg/server/settingswatcher/settings_watcher_external_test.go b/pkg/server/settingswatcher/settings_watcher_external_test.go index 65a903bf1503..9bd804e8764a 100644 --- a/pkg/server/settingswatcher/settings_watcher_external_test.go +++ b/pkg/server/settingswatcher/settings_watcher_external_test.go @@ -21,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" + "github.com/cockroachdb/cockroach/pkg/storage" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" @@ -92,6 +93,10 @@ func TestSettingWatcher(t *testing.T) { s0.ExecutorConfig().(sql.ExecutorConfig).RangeFeedFactory, tc.Stopper()) require.NoError(t, sw.Start(ctx)) + // TestCluster randomizes the value of SeparatedIntentsEnabled, so set it to + // the same as in fakeSettings for the subsequent equality check. + storage.SeparatedIntentsEnabled.Override( + &s0.ClusterSettings().SV, storage.SeparatedIntentsEnabled.Get(&fakeSettings.SV)) require.NoError(t, checkSettingsValuesMatch(s0.ClusterSettings(), fakeSettings)) for k, v := range toSet { tdb.Exec(t, "SET CLUSTER SETTING "+k+" = $1", v[1]) diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index fd2eb27b04f7..f279db7c5aa6 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -132,12 +132,10 @@ func makeTestConfigFromParams(params base.TestServerArgs) Config { st := params.Settings if params.Settings == nil { st = cluster.MakeClusterSettings() - // TODO(sumeer): re-introduce this randomization. - // enabledSeparated := rand.Intn(2) == 0 - // log.Infof(context.Background(), - // "test Config is randomly setting enabledSeparated: %t", - // enabledSeparated) - // storage.SeparatedIntentsEnabled.Override(&st.SV, enabledSeparated) + enabledSeparated := true + log.Infof(context.Background(), + "test Config is randomly setting enabledSeparated: %t", enabledSeparated) + storage.SeparatedIntentsEnabled.Override(&st.SV, enabledSeparated) } st.ExternalIODir = params.ExternalIODir cfg := makeTestConfig(st) diff --git a/pkg/sql/logictest/testdata/logic_test/aggregate b/pkg/sql/logictest/testdata/logic_test/aggregate index 63c1d4cfe00a..cd0c34f249ad 100644 --- a/pkg/sql/logictest/testdata/logic_test/aggregate +++ b/pkg/sql/logictest/testdata/logic_test/aggregate @@ -1,5 +1,11 @@ subtest other +# Reduce the need for ranged intent resolution, since the test environment +# tends to accumulate lots of un-compacted intent tombstones that can slow it +# down. +statement ok +SET CLUSTER SETTING kv.transaction.max_intents_bytes = '1048576' + statement ok CREATE TABLE kv ( k INT PRIMARY KEY, @@ -892,9 +898,15 @@ CREATE TABLE mnop ( p BIGINT ) +# TODO(sumeer): increase this series back to 2e4 once we have more performance +# improvements to ranged intent resolution. This test is somewhat artificial +# in that it accumulates a large number of deletion markers for intents for +# each key, in the LSM memtable, without them being flushed to ssts. This +# slows down ranged intent resolution. With 1e4 keys, the txn tracks each +# intent individually, and intent resolution is fast. statement ok INSERT INTO mnop (m, n) SELECT i, (1e9 + i/2e4)::float FROM - generate_series(1, 2e4) AS i(i) + generate_series(1, 5e3) AS i(i) statement ok UPDATE mnop SET o = n::decimal, p = (n * 10)::bigint @@ -902,22 +914,22 @@ UPDATE mnop SET o = n::decimal, p = (n * 10)::bigint query RRR SELECT round(variance(n), 2), round(variance(n), 2), round(variance(p)) FROM mnop ---- -0.08 0.08 8 +0.01 0.01 1 query RRR SELECT round(var_pop(n), 2), round(var_pop(n), 2), round(var_pop(p)) FROM mnop ---- -0.08 0.08 8 +0.01 0.01 1 query RRR SELECT round(stddev_samp(n), 2), round(stddev(n), 2), round(stddev_samp(p)) FROM mnop ---- -0.29 0.29 3 +0.07 0.07 1 query RRR SELECT round(stddev_pop(n), 2), round(stddev_pop(n), 2), round(stddev_pop(p)) FROM mnop ---- -0.29 0.29 3 +0.07 0.07 1 query RRR SELECT avg(1::int)::float, avg(2::float)::float, avg(3::decimal)::float diff --git a/pkg/sql/logictest/testdata/logic_test/fk b/pkg/sql/logictest/testdata/logic_test/fk index 20785004df5b..dafec94f2d45 100644 --- a/pkg/sql/logictest/testdata/logic_test/fk +++ b/pkg/sql/logictest/testdata/logic_test/fk @@ -1,5 +1,11 @@ # LogicTest: !3node-tenant(49854) +# Reduce the need for ranged intent resolution, since the test environment +# tends to accumulate lots of un-compacted intent tombstones that can slow it +# down. +statement ok +SET CLUSTER SETTING kv.transaction.max_intents_bytes = "1048576" + statement ok SET CLUSTER SETTING sql.cross_db_fks.enabled = TRUE diff --git a/pkg/sql/logictest/testdata/logic_test/tenant b/pkg/sql/logictest/testdata/logic_test/tenant index 6d67ee383196..a341e3f9861c 100644 --- a/pkg/sql/logictest/testdata/logic_test/tenant +++ b/pkg/sql/logictest/testdata/logic_test/tenant @@ -1,4 +1,5 @@ # LogicTest: !3node-tenant + query IBT colnames SELECT * FROM system.tenants ORDER BY id ---- diff --git a/pkg/sql/logictest/testdata/logic_test/upsert b/pkg/sql/logictest/testdata/logic_test/upsert index 4cb4386fcf55..d8dc13b3cb1b 100644 --- a/pkg/sql/logictest/testdata/logic_test/upsert +++ b/pkg/sql/logictest/testdata/logic_test/upsert @@ -1,5 +1,11 @@ subtest strict +# Reduce the need for ranged intent resolution, since the test environment +# tends to accumulate lots of un-compacted intent tombstones that can slow it +# down. +statement ok +SET CLUSTER SETTING kv.transaction.max_intents_bytes = "1048576" + statement ok CREATE TABLE ex( foo INT PRIMARY KEY, diff --git a/pkg/storage/bench_test.go b/pkg/storage/bench_test.go index 188df3cbacd2..88f6f76aa578 100644 --- a/pkg/storage/bench_test.go +++ b/pkg/storage/bench_test.go @@ -141,7 +141,7 @@ func BenchmarkExportToSst(b *testing.B) { const numIntentKeys = 1000 func setupKeysWithIntent( - b *testing.B, eng Engine, numVersions int, numFlushedVersions int, + b *testing.B, eng Engine, numVersions int, numFlushedVersions int, resolveAll bool, ) roachpb.LockUpdate { txnIDCount := 2 * numVersions val := []byte("value") @@ -179,7 +179,7 @@ func setupKeysWithIntent( Txn: txn.TxnMeta, Status: roachpb.COMMITTED, } - if i < numVersions { + if i < numVersions || resolveAll { batch := eng.NewBatch() for j := 0; j < numIntentKeys; j++ { key := makeKey(nil, j) @@ -209,7 +209,7 @@ func BenchmarkIntentScan(b *testing.B) { eng := setupMVCCInMemPebbleWithSettings( b, makeSettingsForSeparatedIntents(false, sep)) numFlushedVersions := (percentFlushed * numVersions) / 100 - setupKeysWithIntent(b, eng, numVersions, numFlushedVersions) + setupKeysWithIntent(b, eng, numVersions, numFlushedVersions, false) lower := makeKey(nil, 0) iter := eng.NewMVCCIterator(MVCCKeyAndIntentsIterKind, IterOptions{ LowerBound: lower, @@ -258,6 +258,101 @@ func BenchmarkIntentScan(b *testing.B) { } } +func BenchmarkScanAllIntentDeleted(b *testing.B) { + skip.UnderShort(b, "setting up unflushed data takes too long") + for _, sep := range []bool{false, true} { + b.Run(fmt.Sprintf("separated=%t", sep), func(b *testing.B) { + for _, numVersions := range []int{200} { + b.Run(fmt.Sprintf("versions=%d", numVersions), func(b *testing.B) { + for _, percentFlushed := range []int{0, 50, 90, 100} { + b.Run(fmt.Sprintf("percent-flushed=%d", percentFlushed), func(b *testing.B) { + eng := setupMVCCInMemPebbleWithSettings( + b, makeSettingsForSeparatedIntents(false, sep)) + numFlushedVersions := (percentFlushed * numVersions) / 100 + setupKeysWithIntent(b, eng, numVersions, numFlushedVersions, true) + lower := makeKey(nil, 0) + iter := eng.NewMVCCIterator(MVCCKeyAndIntentsIterKind, IterOptions{ + LowerBound: lower, + UpperBound: makeKey(nil, numIntentKeys), + }) + iter.SeekGE(MVCCKey{Key: lower}) + var buf []byte + b.ResetTimer() + for i := 0; i < b.N; i++ { + valid, err := iter.Valid() + if err != nil { + b.Fatal(err) + } + if !valid { + iter.SeekGE(MVCCKey{Key: lower}) + } else { + // Read latest version. + k := iter.UnsafeKey() + if !k.IsValue() { + b.Fatalf("expected value %s", k.String()) + } + // Skip to next key. This will do one Next and then Seek. + // iter.NextKey() + buf = append(buf[:0], k.Key...) + buf = roachpb.BytesNext(buf) + iter.SeekGE(MVCCKey{Key: buf}) + } + } + }) + } + }) + } + }) + } +} + +func BenchmarkNextPrevAllIntentDeleted(b *testing.B) { + skip.UnderShort(b, "setting up unflushed data takes too long") + for _, sep := range []bool{false, true} { + b.Run(fmt.Sprintf("separated=%t", sep), func(b *testing.B) { + for _, numVersions := range []int{200} { + b.Run(fmt.Sprintf("versions=%d", numVersions), func(b *testing.B) { + for _, percentFlushed := range []int{0, 50, 90, 100} { + b.Run(fmt.Sprintf("percent-flushed=%d", percentFlushed), func(b *testing.B) { + eng := setupMVCCInMemPebbleWithSettings( + b, makeSettingsForSeparatedIntents(false, sep)) + numFlushedVersions := (percentFlushed * numVersions) / 100 + setupKeysWithIntent(b, eng, numVersions, numFlushedVersions, true) + lower := makeKey(nil, 0) + iter := eng.NewMVCCIterator(MVCCKeyAndIntentsIterKind, IterOptions{ + LowerBound: lower, + UpperBound: makeKey(nil, numIntentKeys), + }) + iter.SeekGE(MVCCKey{Key: lower}) + iter.NextKey() + iter.NextKey() + b.ResetTimer() + for i := 0; i < b.N; i++ { + iter.Next() + valid, err := iter.Valid() + if err != nil { + b.Fatal(err) + } + if !valid { + b.Fatal("should be valid") + } + iter.Prev() + valid, err = iter.Valid() + if err != nil { + b.Fatal(err) + } + if !valid { + b.Fatal("should be valid") + } + } + }) + } + }) + } + }) + } +} + func BenchmarkIntentResolution(b *testing.B) { skip.UnderShort(b, "setting up unflushed data takes too long") for _, sep := range []bool{false, true} { @@ -269,7 +364,7 @@ func BenchmarkIntentResolution(b *testing.B) { eng := setupMVCCInMemPebbleWithSettings( b, makeSettingsForSeparatedIntents(false, sep)) numFlushedVersions := (percentFlushed * numVersions) / 100 - lockUpdate := setupKeysWithIntent(b, eng, numVersions, numFlushedVersions) + lockUpdate := setupKeysWithIntent(b, eng, numVersions, numFlushedVersions, false) keys := make([]roachpb.Key, numIntentKeys) for i := range keys { keys[i] = makeKey(nil, i) @@ -309,7 +404,7 @@ func BenchmarkIntentRangeResolution(b *testing.B) { eng := setupMVCCInMemPebbleWithSettings( b, makeSettingsForSeparatedIntents(false, sep)) numFlushedVersions := (percentFlushed * numVersions) / 100 - lockUpdate := setupKeysWithIntent(b, eng, numVersions, numFlushedVersions) + lockUpdate := setupKeysWithIntent(b, eng, numVersions, numFlushedVersions, false) keys := make([]roachpb.Key, numIntentKeys+1) for i := range keys { keys[i] = makeKey(nil, i) diff --git a/pkg/storage/in_mem.go b/pkg/storage/in_mem.go index aea049d7d6ae..2f46ac6f021c 100644 --- a/pkg/storage/in_mem.go +++ b/pkg/storage/in_mem.go @@ -14,10 +14,13 @@ import ( "context" "math/rand" + "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/pebble" + "github.com/cockroachdb/pebble/vfs" ) // NewInMem allocates and returns a new, opened in-memory engine. The caller @@ -30,6 +33,36 @@ func NewInMem( return newPebbleInMem(ctx, attrs, cacheSize, settings) } +// NewInMemWithCache allocated and returns a new, opened in-memory engine, and +// uses the provided cache. The caller must call the engine's Close method +// when the engine is no longer needed. +func NewInMemWithCache( + ctx context.Context, attrs roachpb.Attributes, cache *pebble.Cache, settings *cluster.Settings, +) Engine { + log.Infof(ctx, "NewInMemWithCache") + opts := DefaultPebbleOptions() + opts.Cache = cache + + opts.FS = vfs.NewMem() + db, err := NewPebble( + ctx, + PebbleConfig{ + StorageConfig: base.StorageConfig{ + Attrs: attrs, + // TODO(bdarnell): The hard-coded 512 MiB is wrong; see + // https://github.com/cockroachdb/cockroach/issues/16750 + MaxSize: 512 << 20, /* 512 MiB */ + Settings: settings, + }, + Opts: opts, + }) + if err != nil { + panic(err) + } + return db + +} + // The ForTesting functions randomize the settings for separated intents. This // is a bit peculiar for tests outside the storage package, since they usually // have higher level cluster constructs, including creating a cluster.Settings diff --git a/pkg/storage/intent_reader_writer.go b/pkg/storage/intent_reader_writer.go index 92d34f46f7ab..9bd6e8cb72be 100644 --- a/pkg/storage/intent_reader_writer.go +++ b/pkg/storage/intent_reader_writer.go @@ -46,7 +46,7 @@ var SeparatedIntentsEnabled = settings.RegisterBoolSetting( "storage.transaction.separated_intents.enabled", "if enabled, intents will be written to a separate lock table, instead of being "+ "interleaved with MVCC values", - false, + true, ) // This file defines wrappers for Reader and Writer, and functions to do the diff --git a/pkg/storage/pebble.go b/pkg/storage/pebble.go index 358313330404..8ba6a5742141 100644 --- a/pkg/storage/pebble.go +++ b/pkg/storage/pebble.go @@ -565,6 +565,7 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (*Pebble, error) { func newPebbleInMem( ctx context.Context, attrs roachpb.Attributes, cacheSize int64, settings *cluster.Settings, ) *Pebble { + log.Infof(ctx, "newPebbleInMem: cacheSize %d", cacheSize) opts := DefaultPebbleOptions() opts.Cache = pebble.NewCache(cacheSize) defer opts.Cache.Unref()