Skip to content

Commit

Permalink
sql: add support for re-using lease cache for audit logging
Browse files Browse the repository at this point in the history
Previously, when a query needed to resolve table names for
audit we would always take the hit of reading the descriptor KV.
This was problematic and incur round trips, which are expensive
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.
  • Loading branch information
fqazi committed Mar 24, 2023
1 parent 319c5ce commit c4a90d6
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
1 change: 1 addition & 0 deletions pkg/bench/rttanalysis/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
25 changes: 25 additions & 0 deletions pkg/bench/rttanalysis/audit_bench_test.go
Original file line number Diff line number Diff line change
@@ -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",
},
})
}
1 change: 1 addition & 0 deletions pkg/bench/rttanalysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions pkg/sql/schema_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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())
Expand Down

0 comments on commit c4a90d6

Please sign in to comment.