From 40b65fff964a488ade3a6818b330c18d14331984 Mon Sep 17 00:00:00 2001
From: sumeerbhola <sumeer@cockroachlabs.com>
Date: Thu, 18 Feb 2021 09:12:17 -0500
Subject: [PATCH] storage,server: enable separated intents

Separated intents are enabled by default, and the setting
is randomized on the testing path for testserver.go.

The intent resolution performance, which caused these
to be disabled, has been improved due to
https://github.com/cockroachdb/cockroach/pull/60622 and
https://github.com/cockroachdb/cockroach/pull/60698

TestLogic/aggregate is tweaked to reduce the number of
intents created, as explained in the TODO added where
the change was made.

Informs #41720

Release note (ops change): The default value of the
storage.transaction.separated_intents.enabled cluster
setting is changed to true.
---
 pkg/server/testserver.go                        | 11 +++++------
 pkg/sql/logictest/testdata/logic_test/aggregate | 16 +++++++++++-----
 pkg/storage/intent_reader_writer.go             |  2 +-
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
index fd2eb27b04f7..acb7845a9144 100644
--- a/pkg/server/testserver.go
+++ b/pkg/server/testserver.go
@@ -15,6 +15,7 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/http/cookiejar"
@@ -132,12 +133,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 := rand.Intn(2) == 0
+		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..a20d52a68ff9 100644
--- a/pkg/sql/logictest/testdata/logic_test/aggregate
+++ b/pkg/sql/logictest/testdata/logic_test/aggregate
@@ -892,9 +892,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 +908,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/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