Merge #105591 #106630

105591: tenantcostclient: support changing ru settings r=JeffSwenson a=JeffSwenson

Previously, there was no way to change the RU cost settings for a stand alone sql server. The setting values are stored in the kv server and initialized by the settings watcher in the sql server. The cost client only respected the values of settings at the time of creating the cost client, which happened as part of creating the kv client, which is required by the settings watcher to read the settings from the kv layer. So there is no way to initialize the cost client with the actual settings value.

Now, the tenantcostclient will update the model whenever the value of settings change, including when the settings are first read during sql server startup.

Part of: https://cockroachlabs.atlassian.net/browse/CC-7140
Release Note: None

106630: sql/sem/builtins: deflake TestSerialNormalizationWithUniqueUnorderedID r=rafiss a=andyyang890

This patch increases the sample count for a probabilistic test that
tests for uniform distribution so that it is less likely to flake.

Fixes #106580

Release note: None

Co-authored-by: Jeff <swenson@cockroachlabs.com>
Co-authored-by: Andy Yang <yang@cockroachlabs.com>

pkg/ccl/multitenantccl/tenantcostclient/BUILD.bazel

@@ -71,6 +71,7 @@ go_test(
         "//pkg/security/securitytest",
         "//pkg/security/username",
         "//pkg/server",
+        "//pkg/settings",
         "//pkg/settings/cluster",
         "//pkg/sql",
         "//pkg/sql/catalog/systemschema",

pkg/ccl/multitenantccl/tenantcostclient/tenant_side.go

@@ -10,6 +10,7 @@ package tenantcostclient
 
 import (
 	"context"
+	"sync/atomic"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
@@ -180,7 +181,13 @@ func newTenantSideCostController(
 		NewRate:   initialRate,
 	})
 
-	c.costCfg = tenantcostmodel.ConfigFromSettings(&st.SV)
+	tenantcostmodel.SetOnChange(&st.SV, func(ctx context.Context) {
+		config := tenantcostmodel.ConfigFromSettings(&st.SV)
+		c.costCfg.Swap(&config)
+	})
+	initialConfig := tenantcostmodel.ConfigFromSettings(&st.SV)
+	c.costCfg.CompareAndSwap(nil, &initialConfig)
+
 	c.modeMu.externalIORUAccountingMode = externalIORUAccountingModeFromString(ExternalIORUAccountingMode.Get(&st.SV))
 	ExternalIORUAccountingMode.SetOnChange(&st.SV, func(context.Context) {
 		c.modeMu.Lock()
@@ -243,7 +250,7 @@ type tenantSideCostController struct {
 	timeSource           timeutil.TimeSource
 	testInstr            TestInstrumentation
 	settings             *cluster.Settings
-	costCfg              tenantcostmodel.Config
+	costCfg              atomic.Pointer[tenantcostmodel.Config]
 	tenantID             roachpb.TenantID
 	provider             kvtenant.TokenBucketProvider
 	limiter              limiter
@@ -428,12 +435,13 @@ func (c *tenantSideCostController) onTick(ctx context.Context, newTime time.Time
 		deltaCPU = 0
 	}
 
-	ru := c.costCfg.PodCPUCost(deltaCPU)
+	costCfg := c.costCfg.Load()
+	ru := costCfg.PodCPUCost(deltaCPU)
 
 	var deltaPGWireEgressBytes uint64
 	if newExternalUsage.PGWireEgressBytes > c.run.externalUsage.PGWireEgressBytes {
 		deltaPGWireEgressBytes = newExternalUsage.PGWireEgressBytes - c.run.externalUsage.PGWireEgressBytes
-		ru += c.costCfg.PGWireEgressCost(int64(deltaPGWireEgressBytes))
+		ru += costCfg.PGWireEgressCost(int64(deltaPGWireEgressBytes))
 	}
 
 	// KV RUs are not included here, these metrics correspond only to the SQL pod.
@@ -779,8 +787,9 @@ func (c *tenantSideCostController) OnResponseWait(
 	}
 
 	// Account for the cost of write requests and read responses.
-	writeRU := c.costCfg.RequestCost(req)
-	readRU := c.costCfg.ResponseCost(resp)
+	costCfg := c.costCfg.Load()
+	writeRU := costCfg.RequestCost(req)
+	readRU := costCfg.ResponseCost(resp)
 	totalRU := writeRU + readRU
 
 	// TODO(andyk): Consider breaking up huge acquisition requests into chunks
@@ -859,8 +868,9 @@ func (c *tenantSideCostController) onExternalIO(
 		return nil
 	}
 
-	totalRU := c.costCfg.ExternalIOIngressCost(usage.IngressBytes) +
-		c.costCfg.ExternalIOEgressCost(usage.EgressBytes)
+	costCfg := c.costCfg.Load()
+	totalRU := costCfg.ExternalIOIngressCost(usage.IngressBytes) +
+		costCfg.ExternalIOEgressCost(usage.EgressBytes)
 
 	if wait {
 		if err := c.limiter.Wait(ctx, totalRU); err != nil {
@@ -891,5 +901,5 @@ func (c *tenantSideCostController) GetCPUMovingAvg() float64 {
 
 // GetCostConfig is part of the multitenant.TenantSideCostController interface.
 func (c *tenantSideCostController) GetCostConfig() *tenantcostmodel.Config {
-	return &c.costCfg
+	return c.costCfg.Load()
 }

pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go

@@ -39,6 +39,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcostmodel"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
@@ -1404,3 +1405,135 @@ func BenchmarkExternalIOAccounting(b *testing.B) {
 		})
 	}
 }
+
+func TestRUSettingsChanged(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+
+	params := base.TestServerArgs{
+		DefaultTestTenant: base.TestTenantDisabled,
+	}
+
+	s, mainDB, _ := serverutils.StartServer(t, params)
+	defer s.Stopper().Stop(ctx)
+	sysDB := sqlutils.MakeSQLRunner(mainDB)
+
+	tenantID := serverutils.TestTenantID()
+	tenant1, tenantDB1 := serverutils.StartTenant(t, s, base.TestTenantArgs{
+		TenantID: tenantID,
+	})
+	defer tenant1.Stopper().Stop(ctx)
+	defer tenantDB1.Close()
+
+	costClient, err := tenantcostclient.NewTenantSideCostController(tenant1.ClusterSettings(), tenantID, nil)
+	require.NoError(t, err)
+
+	initialModel := costClient.GetCostConfig()
+
+	// Increase the RU cost of everything by 100x
+	settings := []*settings.FloatSetting{
+		tenantcostmodel.ReadBatchCost,
+		tenantcostmodel.ReadRequestCost,
+		tenantcostmodel.ReadPayloadCostPerMiB,
+		tenantcostmodel.WriteBatchCost,
+		tenantcostmodel.WriteRequestCost,
+		tenantcostmodel.WritePayloadCostPerMiB,
+		tenantcostmodel.SQLCPUSecondCost,
+		tenantcostmodel.PgwireEgressCostPerMiB,
+		tenantcostmodel.ExternalIOEgressCostPerMiB,
+		tenantcostmodel.ExternalIOIngressCostPerMiB,
+	}
+	for _, setting := range settings {
+		sysDB.Exec(t, fmt.Sprintf("ALTER TENANT ALL SET CLUSTER SETTING %s = $1", setting.Key()), setting.Default()*100)
+	}
+
+	// Check to make sure the cost of the query increased. Use SucceedsSoon
+	// because the settings propogation is async.
+	testutils.SucceedsSoon(t, func() error {
+		currentModel := costClient.GetCostConfig()
+
+		expect100x := func(name string, getter func(model *tenantcostmodel.Config) tenantcostmodel.RU) error {
+			before := getter(initialModel)
+			after := getter(currentModel)
+			expect := before * 100
+			if after != expect {
+				return errors.Newf("expected %s to be %f found %f", name, expect, after)
+			}
+			return nil
+		}
+
+		err = expect100x("KVReadBatch", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.KVReadBatch
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("KVReadRequest", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.KVReadRequest
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("KVReadByte", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.KVReadByte
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("KVWriteBatch", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.KVWriteBatch
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("KVWriteRequest", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.KVWriteRequest
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("KVWriteByte", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.KVWriteByte
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("PodCPUSecond", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.PodCPUSecond
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("PGWireEgressByte", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.PGWireEgressByte
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("ExternalIOEgressByte", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.ExternalIOEgressByte
+		})
+		if err != nil {
+			return err
+		}
+
+		err = expect100x("ExternalIOIngressByte", func(m *tenantcostmodel.Config) tenantcostmodel.RU {
+			return m.ExternalIOIngressByte
+		})
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+}

pkg/sql/sem/builtins/builtins_test.go

@@ -124,7 +124,7 @@ CREATE TABLE t (
   j INT
 )`)
 
-		numberOfRows := 10000
+		numberOfRows := 100000
 
 		// Insert rows.
 		tdb.Exec(t, fmt.Sprintf(`