Merge cockroachdb#105592 cockroachdb#105944 cockroachdb#106931 cockroachdb#107284

105592: tenantcostmodel: charge for cross region network traffic r=JeffSwenson a=JeffSwenson

Previously the 'tenant_cost_model.region_multiplier_table' allowed configuring the cost of multi-region operations by applying a multiplier to the entire operation cost. There were three problems with this approach that prevented deploying it to production:

1. There was a flat multiplier for reads and writes. Because the fixed overhead of a read is much lower, the cost impact of cross region network traffic is much larger when compared to writes. The break even read multiplier was an order of magnitude higher than the break even write multiplier.
2. The correct multiplier depends heavily on how many bytes are in an operation. Large operations require a larger multiplier to break even than smaller operations. Again the break even multiplier for large operations needed to be an order of magnitude larger than the break even for small operations.
3. The original setting assumed the cost of network traffic was symmetric between two different regions. That assumption is true for some cloud providers, but is not true for all cloud providers.

Now, the 'tenant_cost_model.cross_region_network_cost' setting allows configuring the RU/byte cost of network traffic between region pairs. The cost is computed by the dist sender because computing the network cost requires locality information for the local node, the target replica, and all replica's in the range.

The network cost only applies to logical byte traffic, it does not apply to RPC overhead. This is believed to be okay, because the fixed RU cost of each request and batch is high enough to cover the network cost of the operation overhead.

This only accounts for network traffic incurred by reads or writes between the sql server or the kv server. It does not account for network traffic generated by zone configuration changes, replica rebalancing, or distsql.
- Currently distsql is disabled in production Serverless and will only be reenabled once distsql properly accounts for network traffic. Distsql should use the same byte per RU cost table as sql server <-> kv traffic.
- Zone configuration is expected to generate a small enough amount of the overall network traffic that we do not need to charge for it. We can revise this decision in the future if production data shows that is not true.
- Replica rebalancing is something controlled by CRDB. If rebalancing cost ends up being a problem, we could look into reducing the amount of cross-region rebalancing traffic or we could start charging for rebalancing traffic.

This change is backwards incompatible and will be back ported to 23.1. This is safe because Serverless is the only user of the setting.

Release Note: none

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

105944: sql: populate information_schema.routines and parameters r=rafiss a=rafiss

fixes cockroachdb#104083

---

### builtins: add nameconcatoid function 

This adds the nameconcatoid in order to match PostgreSQL. It is used by
the information_schema to create unique names.

Release note (sql change): Added the nameconcatoid builtin function,
which concatenates a name with an OID.

---

### sql: populate pg_catalog.pg_language

Release note (sql change): The pg_catalog.pg_language table is now
populated with data about languages used to define functions.

--- 

### sql: implement information_schema.routines view

Release note (sql change): The information_schema.routines view is now
populated with information about functions.

---

### builtins: add pg_get_function_arg_default

Add an implementation for pg_get_function_arg_default to match
PostgreSQL. Since we don't support function defaults, this always
returns NULL.

Release note: None

---

### sql: populate information_schema.parameters

Release note (sql change): The information_schema.parameters table is
now populated with information about function parameters.

106931: server: ensure SQL statement diagnostic requests work with secondary tenants r=yuzefovich a=knz

Fixes cockroachdb#107243.

Release note (bug fix): A bug with the "SQL statement diagnostic
request" HTTP API that would affect e.g. CC Serverless was fixed. This
bug had existed since v22.x.

Epic: CRDB-26687
Informs: CRDB-18499
Informs cockroachdb#76378.


107284: backupccl: fix URI map keys r=dt a=dt

Protos as keys aren't useful if they have pointers, but the string repr should be equal.

Release note: none.
Epic: none.

Co-authored-by: Jeff <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: David Taylor <[email protected]>

Author: 5 people

docs/generated/sql/functions.md

@@ -3587,6 +3587,8 @@ table. Returns an error if validation fails.</p>
 </span></td><td>Immutable</td></tr>
 <tr><td><a name="information_schema._pg_numeric_scale"></a><code>information_schema._pg_numeric_scale(typid: oid, typmod: int4) &rarr; <a href="int.html">int</a></code></td><td><span class="funcdesc"><p>Returns the scale of the given type with type modifier</p>
 </span></td><td>Immutable</td></tr>
+<tr><td><a name="nameconcatoid"></a><code>nameconcatoid(name: <a href="string.html">string</a>, oid: oid) &rarr; name</code></td><td><span class="funcdesc"><p>Used in the information_schema to produce specific_name columns, which are supposed to be unique per schema. The result is the same as ($1::text || ‘_’ || $2::text)::name except that, if it would not fit in 63 characters, we make it do so by truncating the name input (not the oid).</p>
+</span></td><td>Immutable</td></tr>
 <tr><td><a name="obj_description"></a><code>obj_description(object_oid: oid) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the comment for a database object specified by its OID alone. This is deprecated since there is no guarantee that OIDs are unique across different system catalogs; therefore, the wrong comment might be returned.</p>
 </span></td><td>Stable</td></tr>
 <tr><td><a name="obj_description"></a><code>obj_description(object_oid: oid, catalog_name: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the comment for a database object specified by its OID and the name of the containing system catalog. For example, obj_description(123456, ‘pg_class’) would retrieve the comment for the table with OID 123456.</p>
@@ -3601,6 +3603,8 @@ table. Returns an error if validation fails.</p>
 </span></td><td>Immutable</td></tr>
 <tr><td><a name="pg_function_is_visible"></a><code>pg_function_is_visible(oid: oid) &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Returns whether the function with the given OID belongs to one of the schemas on the search path.</p>
 </span></td><td>Stable</td></tr>
+<tr><td><a name="pg_get_function_arg_default"></a><code>pg_get_function_arg_default(func_oid: oid, arg_num: int4) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Get textual representation of a function argument’s default value. The second argument of this function is the argument number among all arguments (i.e. proallargtypes, <em>not</em> proargtypes), starting with 1, because that’s how information_schema.sql uses it. Currently, this always returns NULL, since CockroachDB does not support default values.</p>
+</span></td><td>Stable</td></tr>
 <tr><td><a name="pg_get_function_arguments"></a><code>pg_get_function_arguments(func_oid: oid) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the argument list (with defaults) necessary to identify a function, in the form it would need to appear in within CREATE FUNCTION.</p>
 </span></td><td>Stable</td></tr>
 <tr><td><a name="pg_get_function_identity_arguments"></a><code>pg_get_function_identity_arguments(func_oid: oid) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the argument list (without defaults) necessary to identify a function, in the form it would need to appear in within ALTER FUNCTION, for instance.</p>

pkg/ccl/backupccl/restore_job.go

@@ -3203,21 +3203,21 @@ func sendAddRemoteSSTs(
 	// We can then look in this map using the proto attached to each file to find
 	// the URI for that file.
 	// TODO(dt/butler): should we plumb the original string instead?
-	urisForDirs := make(map[cloudpb.ExternalStorage]string)
+	urisForDirs := make(map[string]string)
 	for _, u := range uris {
 		dir, err := cloud.ExternalStorageConfFromURI(u, username.SQLUsername{})
 		if err != nil {
 			return err
 		}
-		urisForDirs[dir] = u
+		urisForDirs[dir.String()] = u
 	}
 	for _, loc := range backupLocalityInfo {
 		for _, u := range loc.URIsByOriginalLocalityKV {
 			dir, err := cloud.ExternalStorageConfFromURI(u, username.SQLUsername{})
 			if err != nil {
 				return err
 			}
-			urisForDirs[dir] = u
+			urisForDirs[dir.String()] = u
 		}
 	}
 
@@ -3268,7 +3268,7 @@ func sendAddRemoteSSTs(
 				}
 				file.BackingFileSize = uint64(sz)
 			}
-			uri, ok := urisForDirs[file.Dir]
+			uri, ok := urisForDirs[file.Dir.String()]
 			if !ok {
 				return errors.AssertionFailedf("URI not found for %s", file.Dir.String())
 			}

pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go

@@ -165,12 +165,12 @@ func (ts *testState) stop() {
 }
 
 type cmdArgs struct {
-	count        int64
-	bytes        int64
-	repeat       int64
-	label        string
-	wait         bool
-	ruMultiplier float64
+	count       int64
+	bytes       int64
+	repeat      int64
+	label       string
+	wait        bool
+	networkCost float64
 }
 
 func parseBytesVal(arg datadriven.CmdArg) (int64, error) {
@@ -234,15 +234,17 @@ func parseArgs(t *testing.T, d *datadriven.TestData) cmdArgs {
 				d.Fatalf(t, "invalid wait value")
 			}
 
-		case "ruMultiplier":
+		case "networkCost":
 			if len(args.Vals) != 1 {
-				d.Fatalf(t, "expected one value for ruMultiplier")
+				d.Fatalf(t, "expected one value for networkCost")
 			}
 			val, err := strconv.ParseFloat(args.Vals[0], 64)
 			if err != nil {
-				d.Fatalf(t, "invalid ruMultiplier value")
+				d.Fatalf(t, "invalid networkCost value")
 			}
-			res.ruMultiplier = val
+			res.networkCost = val
+		default:
+			d.Fatalf(t, "uknown command: '%s'", args.Key)
 		}
 	}
 	return res
@@ -313,24 +315,18 @@ func (ts *testState) request(
 	}
 
 	var writeCount, readCount, writeBytes, readBytes int64
-	var writeRUMultiplier, readRUMultiplier tenantcostmodel.RUMultiplier
+	var writeNetworkCost, readNetworkCost tenantcostmodel.NetworkCost
 	if isWrite {
 		writeCount = args.count
 		writeBytes = args.bytes
-		writeRUMultiplier = tenantcostmodel.RUMultiplier(args.ruMultiplier)
-		if writeRUMultiplier == 0 {
-			writeRUMultiplier = 1
-		}
+		writeNetworkCost = tenantcostmodel.NetworkCost(args.networkCost)
 	} else {
 		readCount = args.count
 		readBytes = args.bytes
-		readRUMultiplier = tenantcostmodel.RUMultiplier(args.ruMultiplier)
-		if readRUMultiplier == 0 {
-			readRUMultiplier = 1
-		}
+		readNetworkCost = tenantcostmodel.NetworkCost(args.networkCost)
 	}
-	reqInfo := tenantcostmodel.TestingRequestInfo(1, writeCount, writeBytes, writeRUMultiplier)
-	respInfo := tenantcostmodel.TestingResponseInfo(!isWrite, readCount, readBytes, readRUMultiplier)
+	reqInfo := tenantcostmodel.TestingRequestInfo(1, writeCount, writeBytes, writeNetworkCost)
+	respInfo := tenantcostmodel.TestingResponseInfo(!isWrite, readCount, readBytes, readNetworkCost)
 
 	for ; repeat > 0; repeat-- {
 		ts.runOperation(t, d, args.label, func() {
@@ -788,7 +784,7 @@ func TestWaitingRU(t *testing.T) {
 
 	// Immediately consume the initial 10K RUs.
 	require.NoError(t, ctrl.OnResponseWait(ctx,
-		tenantcostmodel.TestingRequestInfo(1, 1, 10237952, 1), tenantcostmodel.ResponseInfo{}))
+		tenantcostmodel.TestingRequestInfo(1, 1, 10237952, 0), tenantcostmodel.ResponseInfo{}))
 
 	stopper := stop.NewStopper()
 	defer stopper.Stop(ctx)
@@ -806,7 +802,7 @@ func TestWaitingRU(t *testing.T) {
 	// Send 20 KV requests for 1K RU each.
 	const count = 20
 	const fillRate = 100
-	req := tenantcostmodel.TestingRequestInfo(1, 1, 1021952, 1)
+	req := tenantcostmodel.TestingRequestInfo(1, 1, 1021952, 0)
 	resp := tenantcostmodel.TestingResponseInfo(false, 0, 0, 0)
 
 	testutils.SucceedsWithin(t, func() error {

pkg/ccl/multitenantccl/tenantcostclient/testdata/consumption

@@ -364,9 +364,9 @@ PGWire egress:  12345 bytes
 ExternalIO egress: 1026000 bytes
 ExternalIO ingress: 1034000 bytes
 
-# Read the same amount of bytes as the first subtest. Increase in RUs should be
-# multiplied by 2 of the ones in the first subtest.
-read bytes=1048576 ruMultiplier=2.0
+# Read the same amount of bytes as the first subtest. Should have an increase
+# of ~10.5 RUs compared to the first test.
+read bytes=1048576 networkCost=0.00001
 ----
 
 advance
@@ -380,18 +380,17 @@ token-bucket-response
 
 usage
 ----
-RU:  2357.64
-KVRU:  72.25
+RU:  2351.50
+KVRU:  66.11
 Reads:  5 requests in 4 batches (2228224 bytes)
 Writes:  5 requests in 4 batches (10240 bytes)
 SQL Pods CPU seconds:  3.82
 PGWire egress:  12345 bytes
 ExternalIO egress: 1026000 bytes
 ExternalIO ingress: 1034000 bytes
 
-# Test write operation consumption with RU multiplier of 3.5 on the RUs
-# increased in the second subtest.
-write bytes=1024 ruMultiplier=3.5
+# This write is expected to consume an extra 5.12 RUs from network cost usage.
+write bytes=1024 networkCost=0.005
 ----
 
 advance
@@ -405,8 +404,8 @@ token-bucket-response
 
 usage
 ----
-RU:  2368.14
-KVRU:  82.75
+RU:  2359.62
+KVRU:  74.23
 Reads:  5 requests in 4 batches (2228224 bytes)
 Writes:  6 requests in 5 batches (11264 bytes)
 SQL Pods CPU seconds:  3.82
@@ -415,10 +414,10 @@ ExternalIO egress: 1026000 bytes
 ExternalIO ingress: 1034000 bytes
 
 # Test multiple requests in the same batch with RU multiplier of 1.
-write count=2 bytes=1024 ruMultiplier=1.0
+write count=2 bytes=1024 networkCost=0
 ----
 
-read count=2 bytes=65536 ruMultiplier=1.0
+read count=2 bytes=65536 networkCost=0
 ----
 
 advance
@@ -432,8 +431,8 @@ token-bucket-response
 
 usage
 ----
-RU:  2373.89
-KVRU:  88.50
+RU:  2365.37
+KVRU:  79.98
 Reads:  7 requests in 5 batches (2293760 bytes)
 Writes:  8 requests in 6 batches (12288 bytes)
 SQL Pods CPU seconds:  3.82

pkg/cli/clisqlshell/testdata/describe

@@ -585,13 +585,13 @@ https://www.postgresql.org/docs/9.5/view-pg-indexes.html"
 pg_catalog,pg_inherits,table,admin,NULL,permanent,prefix,"table inheritance hierarchy (empty - feature does not exist)
 https://www.postgresql.org/docs/9.5/catalog-pg-inherits.html"
 pg_catalog,pg_init_privs,table,admin,NULL,permanent,prefix,pg_init_privs was created for compatibility and is currently unimplemented
-pg_catalog,pg_language,table,admin,NULL,permanent,prefix,"available languages (empty - feature does not exist)
+pg_catalog,pg_language,table,admin,NULL,permanent,prefix,"available languages
 https://www.postgresql.org/docs/9.5/catalog-pg-language.html"
 pg_catalog,pg_largeobject,table,admin,NULL,permanent,prefix,pg_largeobject was created for compatibility and is currently unimplemented
 pg_catalog,pg_largeobject_metadata,table,admin,NULL,permanent,prefix,pg_largeobject_metadata was created for compatibility and is currently unimplemented
 pg_catalog,pg_locks,table,admin,NULL,permanent,prefix,"locks held by active processes (empty - feature does not exist)
 https://www.postgresql.org/docs/9.6/view-pg-locks.html"
-pg_catalog,pg_matviews,table,admin,NULL,permanent,prefix,"available materialized views (empty - feature does not exist)
+pg_catalog,pg_matviews,table,admin,NULL,permanent,prefix,"available materialized views
 https://www.postgresql.org/docs/9.6/view-pg-matviews.html"
 pg_catalog,pg_namespace,table,admin,NULL,permanent,prefix,"available namespaces
 https://www.postgresql.org/docs/9.5/catalog-pg-namespace.html"