Merge #99502

99502: sql: add support for re-using lease cache for audit logging r=fqazi a=fqazi Previously, when a query needed to resolve table names for audit we would always take the hit of a KV call. This could be problematic since we could incur extra round trips on multi-region clusters. To address this, this patch will add support for using the lease cache for these looks up and eliminate any extra round trips. Fixes: #99475 Release note (performance improvement): audit logging could incur extra latency when resolving table/view/sequence names. Co-authored-by: Faizan Qazi <[email protected]>
cockroachdb · Mar 25, 2023 · 9a0d493 · 9a0d493
2 parents 3d90d97 + c4a90d6
commit 9a0d493
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 2 deletions.
diff --git a/pkg/bench/rttanalysis/BUILD.bazel b/pkg/bench/rttanalysis/BUILD.bazel
@@ -36,6 +36,7 @@ go_test(
     size = "large",
     srcs = [
         "alter_table_bench_test.go",
+        "audit_bench_test.go",
         "bench_test.go",
         "create_alter_role_bench_test.go",
         "discard_bench_test.go",

diff --git a/pkg/bench/rttanalysis/audit_bench_test.go b/pkg/bench/rttanalysis/audit_bench_test.go
@@ -0,0 +1,25 @@
+// Copyright 2020 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package rttanalysis
+
+import "testing"
+
+func BenchmarkAudit(b *testing.B) { reg.Run(b) }
+func init() {
+	reg.Register("Audit", []RoundTripBenchTestCase{
+		{
+			Name: "select from an audit table",
+			Setup: `CREATE TABLE audit_table(a INT);
+							ALTER TABLE audit_table EXPERIMENTAL_AUDIT SET READ WRITE;`,
+			Stmt: "SELECT * from audit_table",
+		},
+	})
+}
diff --git a/pkg/bench/rttanalysis/testdata/benchmark_expectations b/pkg/bench/rttanalysis/testdata/benchmark_expectations
@@ -27,6 +27,7 @@ exp,benchmark
 7,AlterTableUnsplit/alter_table_unsplit_at_1_value
 9,AlterTableUnsplit/alter_table_unsplit_at_2_values
 11,AlterTableUnsplit/alter_table_unsplit_at_3_values
+5,Audit/select_from_an_audit_table
 20,CreateRole/create_role_with_1_option
 23,CreateRole/create_role_with_2_options
 26,CreateRole/create_role_with_3_options

diff --git a/pkg/sql/schema_resolver.go b/pkg/sql/schema_resolver.go
@@ -223,7 +223,10 @@ func (sr *schemaResolver) GetQualifiedTableNameByID(
 func (sr *schemaResolver) getQualifiedTableName(
 	ctx context.Context, desc catalog.TableDescriptor,
 ) (*tree.TableName, error) {
-	dbDesc, err := sr.descCollection.ByID(sr.txn).Get().Database(ctx, desc.GetParentID())
+	// When getting the fully qualified name allow use of leased descriptors,
+	// since these will not involve any round trip.
+	descGetter := sr.descCollection.ByIDWithLeased(sr.txn)
+	dbDesc, err := descGetter.Get().Database(ctx, desc.GetParentID())
 	if err != nil {
 		return nil, err
 	}
@@ -237,7 +240,7 @@ func (sr *schemaResolver) getQualifiedTableName(
 	// information from the namespace table.
 	var schemaName tree.Name
 	schemaID := desc.GetParentSchemaID()
-	scDesc, err := sr.descCollection.ByID(sr.txn).Get().Schema(ctx, schemaID)
+	scDesc, err := descGetter.Get().Schema(ctx, schemaID)
 	switch {
 	case scDesc != nil:
 		schemaName = tree.Name(scDesc.GetName())