Merge #147996 #149757 #149767 #149848

147996: sql: add security_invoker feature flag and grammar support r=Dedej-Bergin a=Dedej-Bergin

This change adds the `enable_view_security_invoker` feature flag. It also adds grammar support for the following two statements:
```
CREATE VIEW <view_name> WITH ( security_invoker = true ) AS SELECT * FROM <table_name>;
ALTER VIEW <view_name> SET ( security_invoker = false );
```

Informs: #138918
Epic: CRDB-48807
Release note: None

149757: asim,roachpb: follow-up to #149699 r=wenyihu6 a=tbg

- **tests: add missing RaftCPUPerWrite**
- **roachpb: rm a test**
- **roachpb: improve a comment**
- **asim: include WriteBytesPerSecond in StoreMetrics**
- **asim: fix argument passing in BasicLoad.Generate**

Epic: CRDB-25222


149767: util: add envAlwaysKeepTestLogsEnabled r=rickystewart a=wenyihu6

This commit adds support for a new environment variable,
COCKROACH_ALWAYS_KEEP_TEST_LOGS, which ensures that crdb and test logs are kept
even when tests fail. Example usage:

```
./dev test .... -- --test_env=COCKROACH_ALWAYS_KEEP_TEST_LOGS=true
```

Epic: none
Release note: none



149848: sql: default sql.stats.error_on_concurrent_create_stats.enabled to false r=mw5h a=mw5h

'sql.stats.error_on_concurrent_create_stats.enabled' was introduced with a default value of true to ease backporting. Going forward, we want this to default to false so that customers are not alarmed by error counters increasing.

Informs: #148413
Release note (ops change): The value of
sql.stats.error_on_concurrent_create_stats.enabled now defaults to false, supressing error counters for auto stats jobs that fail due to concurrent stats jobs in progress.

Co-authored-by: Bergin Dedej <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: wenyihu6 <[email protected]>
Co-authored-by: Matt White <[email protected]>

Author: 5 people

docs/generated/settings/settings-for-tenants.txt

@@ -356,7 +356,7 @@ sql.stats.automatic_partial_collection.fraction_stale_rows	float	0.05	target fra
 sql.stats.automatic_partial_collection.min_stale_rows	integer	100	target minimum number of stale rows per table that will trigger a partial statistics refresh	application
 sql.stats.cleanup.recurrence	string	@hourly	cron-tab recurrence for SQL Stats cleanup job	application
 sql.stats.detailed_latency_metrics.enabled	boolean	false	label latency metrics with the statement fingerprint. Workloads with tens of thousands of distinct query fingerprints should leave this setting false. (experimental, affects performance for workloads with high fingerprint cardinality)	application
-sql.stats.error_on_concurrent_create_stats.enabled	boolean	true	set to true to error on concurrent CREATE STATISTICS jobs, instead of skipping them	application
+sql.stats.error_on_concurrent_create_stats.enabled	boolean	false	set to true to error on concurrent CREATE STATISTICS jobs, instead of skipping them	application
 sql.stats.flush.enabled	boolean	true	if set, SQL execution statistics are periodically flushed to disk	application
 sql.stats.flush.interval	duration	10m0s	the interval at which SQL execution statistics are flushed to disk, this value must be less than or equal to 1 hour	application
 sql.stats.forecasts.enabled	boolean	true	when true, enables generation of statistics forecasts by default for all tables	application

docs/generated/settings/settings.html

@@ -311,7 +311,7 @@
 <tr><td><div id="setting-sql-stats-automatic-partial-collection-min-stale-rows" class="anchored"><code>sql.stats.automatic_partial_collection.min_stale_rows</code></div></td><td>integer</td><td><code>100</code></td><td>target minimum number of stale rows per table that will trigger a partial statistics refresh</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-cleanup-recurrence" class="anchored"><code>sql.stats.cleanup.recurrence</code></div></td><td>string</td><td><code>@hourly</code></td><td>cron-tab recurrence for SQL Stats cleanup job</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-detailed-latency-metrics-enabled" class="anchored"><code>sql.stats.detailed_latency_metrics.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>label latency metrics with the statement fingerprint. Workloads with tens of thousands of distinct query fingerprints should leave this setting false. (experimental, affects performance for workloads with high fingerprint cardinality)</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-sql-stats-error-on-concurrent-create-stats-enabled" class="anchored"><code>sql.stats.error_on_concurrent_create_stats.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>set to true to error on concurrent CREATE STATISTICS jobs, instead of skipping them</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-sql-stats-error-on-concurrent-create-stats-enabled" class="anchored"><code>sql.stats.error_on_concurrent_create_stats.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to error on concurrent CREATE STATISTICS jobs, instead of skipping them</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-flush-enabled" class="anchored"><code>sql.stats.flush.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, SQL execution statistics are periodically flushed to disk</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-flush-interval" class="anchored"><code>sql.stats.flush.interval</code></div></td><td>duration</td><td><code>10m0s</code></td><td>the interval at which SQL execution statistics are flushed to disk, this value must be less than or equal to 1 hour</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-sql-stats-forecasts-enabled" class="anchored"><code>sql.stats.forecasts.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>when true, enables generation of statistics forecasts by default for all tables</td><td>Serverless/Dedicated/Self-Hosted</td></tr>

docs/generated/sql/bnf/stmt_block.bnf

@@ -1459,6 +1459,7 @@ unreserved_keyword ::=
 	| 'SEARCH'
 	| 'SECOND'
 	| 'SECURITY'
+	| 'SECURITY_INVOKER'
 	| 'SECONDARY'
 	| 'SERIALIZABLE'
 	| 'SEQUENCE'
@@ -4385,6 +4386,7 @@ bare_label_keywords ::=
 	| 'SEARCH'
 	| 'SECONDARY'
 	| 'SECURITY'
+	| 'SECURITY_INVOKER'
 	| 'SELECT'
 	| 'SEQUENCE'
 	| 'SEQUENCES'

pkg/kv/kvserver/asim/gen/generator.go

@@ -165,8 +165,8 @@ func (bl BasicLoad) Generate(seed int64, settings *config.SimulationSettings) []
 			bl.RWRatio,
 			bl.MaxBlockSize,
 			bl.MinBlockSize,
-			bl.RaftCPUPerWrite,
 			bl.RequestCPUPerAccess,
+			bl.RaftCPUPerWrite,
 		),
 	}
 }

pkg/kv/kvserver/asim/metrics/tracker.go

@@ -141,22 +141,23 @@ func (mt *Tracker) Tick(ctx context.Context, tick time.Time, s state.State) {
 		desc := store.Descriptor()
 
 		sm := StoreMetrics{
-			Tick:               tick,
-			StoreID:            int64(storeID),
-			QPS:                int64(desc.Capacity.QueriesPerSecond),
-			CPU:                int64(desc.Capacity.CPUPerSecond),
-			WriteKeys:          u.WriteKeys,
-			WriteBytes:         u.WriteBytes,
-			ReadKeys:           u.ReadKeys,
-			ReadBytes:          u.ReadBytes,
-			Replicas:           int64(desc.Capacity.RangeCount),
-			Leases:             int64(desc.Capacity.LeaseCount),
-			LeaseTransfers:     u.LeaseTransfers,
-			Rebalances:         u.Rebalances,
-			RebalanceSentBytes: u.RebalanceSentBytes,
-			RebalanceRcvdBytes: u.RebalanceRcvdBytes,
-			RangeSplits:        u.RangeSplits,
-			DiskFractionUsed:   desc.Capacity.FractionUsed(),
+			Tick:                tick,
+			StoreID:             int64(storeID),
+			QPS:                 int64(desc.Capacity.QueriesPerSecond),
+			CPU:                 int64(desc.Capacity.CPUPerSecond),
+			WriteKeys:           u.WriteKeys,
+			WriteBytes:          u.WriteBytes,
+			WriteBytesPerSecond: int64(desc.Capacity.WriteBytesPerSecond),
+			ReadKeys:            u.ReadKeys,
+			ReadBytes:           u.ReadBytes,
+			Replicas:            int64(desc.Capacity.RangeCount),
+			Leases:              int64(desc.Capacity.LeaseCount),
+			LeaseTransfers:      u.LeaseTransfers,
+			Rebalances:          u.Rebalances,
+			RebalanceSentBytes:  u.RebalanceSentBytes,
+			RebalanceRcvdBytes:  u.RebalanceRcvdBytes,
+			RangeSplits:         u.RangeSplits,
+			DiskFractionUsed:    desc.Capacity.FractionUsed(),
 		}
 		sms = append(sms, sm)
 	}

pkg/kv/kvserver/asim/tests/default_settings.go

@@ -115,6 +115,7 @@ func (f randTestingFramework) defaultLoadGen() gen.BasicLoad {
 		MinKey:              f.defaultStaticSettings.minKey,
 		MaxKey:              f.defaultStaticSettings.maxKey,
 		RequestCPUPerAccess: f.defaultStaticSettings.requestCPUPerAccess,
+		RaftCPUPerWrite:     f.defaultStaticSettings.raftCPUPerWrite,
 	}
 }
 

pkg/roachpb/metadata.proto

@@ -303,8 +303,9 @@ message NodeCapacity {
 
   // NodeCPURateUsage is the node's actual CPU usage in ns/sec, measured by the
   // runtime load monitor tracking user+system CPU time. This is typically
-  // higher than StoresCPURate gaps in the fine-grained instrumentation used to
-  // compute StoresCPURate.
+  // higher than StoresCPURate due gaps in the fine-grained instrumentation used to
+  // compute StoresCPURate, and due to the fact that there are uses of CPU not
+  // occurring in the context of a Store.
   optional int64 node_cpu_rate_usage = 3 [(gogoproto.nullable) = false, (gogoproto.customname) = "NodeCPURateUsage"];
 
   // NodeCPURateCapacity is the node's total CPU capacity in nanoseconds per

pkg/roachpb/metadata_test.go

@@ -850,68 +850,3 @@ func TestRangeDescriptorsByStartKey(t *testing.T) {
 		}
 	}
 }
-
-// TestNodeCapacity tests the NodeCapacity struct.
-func TestNodeCapacity(t *testing.T) {
-	t.Run("basic", func(t *testing.T) {
-		actual := NodeCapacity{
-			StoresCPURate:       1,
-			NumStores:           1,
-			NodeCPURateUsage:    800000000,
-			NodeCPURateCapacity: 10,
-		}
-		require.Equal(t, int64(1), actual.StoresCPURate)
-		require.Equal(t, int32(1), actual.NumStores)
-		require.Equal(t, int64(800000000), actual.NodeCPURateUsage)
-		require.Equal(t, int64(10), actual.NodeCPURateCapacity)
-	})
-
-	t.Run("zero values", func(t *testing.T) {
-		actual := NodeCapacity{}
-		require.Equal(t, int64(0), actual.StoresCPURate)
-		require.Equal(t, int32(0), actual.NumStores)
-		require.Equal(t, int64(0), actual.NodeCPURateUsage)
-		require.Equal(t, int64(0), actual.NodeCPURateCapacity)
-	})
-
-	t.Run("equality", func(t *testing.T) {
-		nc1 := NodeCapacity{
-			StoresCPURate:       1000000000,
-			NumStores:           3,
-			NodeCPURateUsage:    800000000,
-			NodeCPURateCapacity: 1000000000,
-		}
-		nc2 := nc1
-		require.Equal(t, nc1, nc2)
-		require.Equal(t, int64(1000000000), nc2.StoresCPURate)
-		require.Equal(t, int32(3), nc2.NumStores)
-		require.Equal(t, int64(800000000), nc2.NodeCPURateUsage)
-		require.Equal(t, int64(1000000000), nc2.NodeCPURateCapacity)
-	})
-
-	t.Run("wrap in store descriptor", func(t *testing.T) {
-		nc := NodeCapacity{
-			StoresCPURate:       1000000000,
-			NumStores:           3,
-			NodeCPURateUsage:    800000000,
-			NodeCPURateCapacity: 1000000000,
-		}
-
-		storeDesc := StoreDescriptor{
-			StoreID: 1,
-			Node: NodeDescriptor{
-				NodeID: 1,
-			},
-			Capacity: StoreCapacity{
-				Capacity:   1000000000,
-				Available:  500000000,
-				RangeCount: 100,
-			},
-			NodeCapacity: nc,
-		}
-		require.Equal(t, int64(1000000000), storeDesc.NodeCapacity.StoresCPURate)
-		require.Equal(t, int32(3), storeDesc.NodeCapacity.NumStores)
-		require.Equal(t, int64(800000000), storeDesc.NodeCapacity.NodeCPURateUsage)
-		require.Equal(t, int64(1000000000), storeDesc.NodeCapacity.NodeCPURateCapacity)
-	})
-}

pkg/sql/create_stats.go

@@ -87,7 +87,7 @@ var errorOnConcurrentCreateStats = settings.RegisterBoolSetting(
 	settings.ApplicationLevel,
 	"sql.stats.error_on_concurrent_create_stats.enabled",
 	"set to true to error on concurrent CREATE STATISTICS jobs, instead of skipping them",
-	true,
+	false,
 	settings.WithPublic)
 
 const nonIndexColHistogramBuckets = 2

pkg/sql/create_view.go

@@ -77,6 +77,12 @@ func (n *createViewNode) startExec(params runParams) error {
 		return pgerror.Newf(pgcode.ReadOnlySQLTransaction, "schema changes are not allowed on a reader catalog")
 	}
 	createView := n.createView
+
+	if !params.SessionData().AllowViewWithSecurityInvokerClause && createView.Options != nil && createView.Options.SecurityInvoker {
+		return pgerror.Newf(pgcode.FeatureNotSupported,
+			"security invoker views are not supported")
+	}
+
 	tableType := tree.GetTableType(
 		false /* isSequence */, true /* isView */, createView.Materialized,
 	)