Merge #148290 #148596

148290: sql: default to node-level cache for sequences r=bghal a=bghal

The session-level cache was implemented first and shared node-level cache was added in #118546.
The better behaving node-level cache is not the default option.
Changes the default for `CACHE` option to set node-level cache. A new option to allows the session-level cache to be set.

Epic: CRDB-42587
Fixes: #131531

Release note (sql change): Changed the basic sequence cache option to set cache at the node-level. The PER SESSION NODE sequence option (syntax is  is [ PER SESSION CACHE # ]) provides the previous behavior to set session-level cache.

148596: roachtest: remove error assertion from panic node mutator r=stevendann,DarrylWong a=herkolategan

The panic node mutator error assertion is causing flakes, as the error is not always the specific TCP error that is expected even if the panic is working as it should.

Given that we are not testing the panic implementation here, asserting on the node being down should be enough.

Fixes: #148586

Epic: None
Release note: None

Co-authored-by: Brendan Gerrity <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>

Author: 3 people

docs/generated/sql/bnf/alter_sequence.bnf

@@ -1,5 +1,5 @@
 alter_sequence_stmt ::=
 	( 'ALTER' 'SEQUENCE' sequence_name 'RENAME' 'TO' sequence_name | 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name 'RENAME' 'TO' sequence_name )
-	| ( 'ALTER' 'SEQUENCE' sequence_name ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) | 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) )
+	| ( 'ALTER' 'SEQUENCE' sequence_name ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) | 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) )
 	| ( 'ALTER' 'SEQUENCE' sequence_name 'SET' 'SCHEMA' schema_name | 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name 'SET' 'SCHEMA' schema_name )
 	| ( 'ALTER' 'SEQUENCE' sequence_name 'OWNER' 'TO' role_spec | 'ALTER' 'SEQUENCE' 'IF' 'EXISTS' sequence_name 'OWNER' 'TO' role_spec )

docs/generated/sql/bnf/create_sequence_stmt.bnf

@@ -1,3 +1,3 @@
 create_sequence_stmt ::=
-	'CREATE' opt_temp 'SEQUENCE' sequence_name ( ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) |  )
-	| 'CREATE' opt_temp 'SEQUENCE' 'IF' 'NOT' 'EXISTS' sequence_name ( ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) |  )
+	'CREATE' opt_temp 'SEQUENCE' sequence_name ( ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) |  )
+	| 'CREATE' opt_temp 'SEQUENCE' 'IF' 'NOT' 'EXISTS' sequence_name ( ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) ( ( ( 'AS' typename | 'NO' 'CYCLE' | 'OWNED' 'BY' 'NONE' | 'OWNED' 'BY' column_name | 'CACHE' integer | 'PER' 'NODE' 'CACHE' integer | 'PER' 'SESSION' 'CACHE' integer | 'INCREMENT' integer | 'INCREMENT' 'BY' integer | 'MINVALUE' integer | 'NO' 'MINVALUE' | 'MAXVALUE' integer | 'NO' 'MAXVALUE' | 'START' integer | 'START' 'WITH' integer | 'RESTART' | 'RESTART' integer | 'RESTART' 'WITH' integer | 'VIRTUAL' ) ) )* ) |  )

docs/generated/sql/bnf/stmt_block.bnf

@@ -3743,6 +3743,7 @@ sequence_option_elem ::=
 	| 'OWNED' 'BY' column_path
 	| 'CACHE' signed_iconst64
 	| 'PER' 'NODE' 'CACHE' signed_iconst64
+	| 'PER' 'SESSION' 'CACHE' signed_iconst64
 	| 'INCREMENT' signed_iconst64
 	| 'INCREMENT' 'BY' signed_iconst64
 	| 'MINVALUE' signed_iconst64
@@ -4939,6 +4940,8 @@ col_qual_list ::=
 identity_option_elem ::=
 	'SET' 'NO' 'CYCLE'
 	| 'SET' 'CACHE' signed_iconst64
+	| 'SET' 'PER' 'NODE' 'CACHE' signed_iconst64
+	| 'SET' 'PER' 'SESSION' 'CACHE' signed_iconst64
 	| 'SET' 'INCREMENT' signed_iconst64
 	| 'SET' 'INCREMENT' 'BY' signed_iconst64
 	| 'SET' 'MINVALUE' signed_iconst64

pkg/cmd/roachtest/roachtestutil/mixedversion/steps.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
-	"regexp"
 	"strings"
 	"time"
 
@@ -804,8 +803,6 @@ func (s panicNodeStep) Description() string {
 	return fmt.Sprintf("panicking system interface on node %d", s.targetNode[0])
 }
 
-var connectionRefusedRegex = regexp.MustCompile(`dial tcp .*: connect: connection refused`)
-
 func (s panicNodeStep) Run(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *Helper) error {
 	nodeVersion, err := h.System.NodeVersion(s.targetNode[0])
 	if err != nil {
@@ -831,14 +828,6 @@ func (s panicNodeStep) Run(ctx context.Context, l *logger.Logger, rng *rand.Rand
 		return errors.Errorf("expected panic statement to fail, but it succeeded on %s", s.targetNode)
 	}
 
-	isTCPError := connectionRefusedRegex.MatchString(err.Error())
-
-	// The expected behavior is that the panic statement will fail with a TCP connection error,
-	// so any other error is unexpected and should cause the test to fail.
-	if !isTCPError {
-		return errors.Wrapf(err, "unexpected error when executing panic statement on %s", s.targetNode)
-	}
-
 	startCtx, cancel := context.WithTimeout(ctx, startTimeout)
 	defer cancel()
 

pkg/sql/alter_table.go

@@ -1252,8 +1252,11 @@ func applyColumnMutation(
 
 		opts := seqDesc.GetSequenceOpts()
 		optsNode := tree.SequenceOptions{}
-		if opts.CacheSize > 1 {
-			optsNode = append(optsNode, tree.SequenceOption{Name: tree.SeqOptCache, IntVal: &opts.CacheSize})
+		if opts.SessionCacheSize > 1 {
+			optsNode = append(optsNode, tree.SequenceOption{Name: tree.SeqOptCacheSession, IntVal: &opts.SessionCacheSize})
+		}
+		if opts.NodeCacheSize > 1 {
+			optsNode = append(optsNode, tree.SequenceOption{Name: tree.SeqOptCacheNode, IntVal: &opts.NodeCacheSize})
 		}
 		optsNode = append(optsNode, tree.SequenceOption{Name: tree.SeqOptMinValue, IntVal: &opts.MinValue})
 		optsNode = append(optsNode, tree.SequenceOption{Name: tree.SeqOptMaxValue, IntVal: &opts.MaxValue})

pkg/sql/catalog/descpb/structured.go

@@ -302,7 +302,7 @@ func (opts *TableDescriptor_SequenceOpts) HasOwner() bool {
 //
 // Note: An unset cache size is considered disabled.
 func (opts *TableDescriptor_SequenceOpts) EffectiveCacheSize() int64 {
-	switch sessionCacheSize := opts.CacheSize; sessionCacheSize {
+	switch sessionCacheSize := opts.SessionCacheSize; sessionCacheSize {
 	case 0, 1:
 	default:
 		return sessionCacheSize

pkg/sql/catalog/descpb/structured.proto

@@ -1194,8 +1194,8 @@ message TableDescriptor {
     optional SequenceOwner sequence_owner = 6 [(gogoproto.nullable) = false];
 
     // The number of values (which have already been created in KV)
-    // that a node can cache locally.
-    optional int64 cache_size = 7 [(gogoproto.nullable) = false];
+    // that a session can cache locally.
+    optional int64 session_cache_size = 7 [(gogoproto.nullable) = false];
     // AS option value for CREATE SEQUENCE, which specifies the default
     // min and max values a sequence can take on.
     optional string as_integer_type = 8 [(gogoproto.nullable) = false];

pkg/sql/catalog/descpb/structured_test.go

@@ -132,8 +132,8 @@ func TestEffectiveCacheSize(t *testing.T) {
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
 			opts := TableDescriptor_SequenceOpts{
-				CacheSize:     tt.session,
-				NodeCacheSize: tt.node,
+				SessionCacheSize: tt.session,
+				NodeCacheSize:    tt.node,
 			}
 			result := opts.EffectiveCacheSize()
 			if result != tt.expectedResult {

pkg/sql/catalog/schemaexpr/sequence_options.go

@@ -111,7 +111,7 @@ func AssignSequenceOptions(
 			opts.MaxValue = -1
 			opts.Start = opts.MaxValue
 		}
-		opts.CacheSize = 1
+		opts.SessionCacheSize = 1
 	}
 
 	// Set Minvalue and Maxvalue to new types bounds if at current bounds.
@@ -148,19 +148,19 @@ func AssignSequenceOptions(
 				"CYCLE option is not supported")
 		case tree.SeqOptNoCycle:
 			// Do nothing; this is the default.
-		case tree.SeqOptCache:
+		case tree.SeqOptCacheNode:
 			if v := *option.IntVal; v >= 1 {
-				opts.CacheSize = v
+				opts.NodeCacheSize = v
 			} else {
 				return errors.Newf(
-					"CACHE (%d) must be greater than zero", v)
+					"PER NODE CACHE (%d) must be greater than zero", v)
 			}
-		case tree.SeqOptCacheNode:
+		case tree.SeqOptCacheSession:
 			if v := *option.IntVal; v >= 1 {
-				opts.NodeCacheSize = v
+				opts.SessionCacheSize = v
 			} else {
 				return errors.Newf(
-					"PER NODE CACHE (%d) must be greater than zero", v)
+					"PER SESSION CACHE (%d) must be greater than zero", v)
 			}
 		case tree.SeqOptIncrement:
 			// Do nothing; this has already been set.

pkg/sql/catalog/serial_helper.go

@@ -61,7 +61,7 @@ func SequenceOptionsFromNormalizationMode(
 	case sessiondatapb.SerialUsesCachedSQLSequences:
 		cacheValue := sqlclustersettings.CachedSequencesCacheSizeSetting.Get(&st.SV)
 		seqOpts = append(seqOpts, tree.SequenceOption{
-			Name:   tree.SeqOptCache,
+			Name:   tree.SeqOptCacheSession,
 			IntVal: &cacheValue,
 		})
 	case sessiondatapb.SerialUsesCachedNodeSQLSequences: