diff --git a/pkg/bench/rttanalysis/BUILD.bazel b/pkg/bench/rttanalysis/BUILD.bazel index 0dacf556980e..49ce84966124 100644 --- a/pkg/bench/rttanalysis/BUILD.bazel +++ b/pkg/bench/rttanalysis/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//pkg/base", "//pkg/kv/kvclient/kvcoord", "//pkg/sql", + "//pkg/sql/parser", "//pkg/testutils", "//pkg/testutils/skip", "//pkg/testutils/sqlutils", diff --git a/pkg/bench/rttanalysis/rtt_analysis_bench.go b/pkg/bench/rttanalysis/rtt_analysis_bench.go index e9f333c84be8..65e4b012d89b 100644 --- a/pkg/bench/rttanalysis/rtt_analysis_bench.go +++ b/pkg/bench/rttanalysis/rtt_analysis_bench.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord" + "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -117,32 +118,51 @@ func executeRoundTripTest(b testingB, tc RoundTripBenchTestCase, cc ClusterConst b.StopTimer() var r tracingpb.Recording + // The statement trace records individual statements, but we may want to + // execute multiple SQL statements. Note that multi-statement traces won't + // count round trips correctly if there are duplicate statements. + statements, err := parser.Parse(tc.Stmt) + if err != nil { + panic(err) + } + // Do an extra iteration and don't record it in order to deal with effects of // running it the first time. for i := 0; i < b.N()+1; i++ { sql.Exec(b, "CREATE DATABASE bench;") sql.Exec(b, tc.Setup) - cluster.clearStatementTrace(tc.Stmt) + for _, statement := range statements { + cluster.clearStatementTrace(statement.SQL) + } b.StartTimer() sql.Exec(b, tc.Stmt) b.StopTimer() var ok bool - r, ok = cluster.getStatementTrace(tc.Stmt) - if !ok { - b.Fatalf( - "could not find number of round trips for statement: %s", - tc.Stmt, - ) - } - // If there's a retry error then we're just going to throw away this - // run. - rt, hasRetry := countKvBatchRequestsInRecording(r) - if hasRetry { - i-- - } else if i > 0 { // skip the initial iteration - roundTrips += rt + total := 0 + for _, statement := range statements { + r, ok = cluster.getStatementTrace(statement.SQL) + if !ok { + b.Fatalf( + "could not find number of round trips for statement: %s", + statement.SQL, + ) + } + + // If there's a retry error then we're just going to throw away this + // run. + rt, hasRetry := countKvBatchRequestsInRecording(r) + if hasRetry { + i-- + ok = false + break + } else if i > 0 { // skip the initial iteration + total += rt + } + } + if ok { + roundTrips += total } sql.Exec(b, "DROP DATABASE bench;") diff --git a/pkg/bench/rttanalysis/testdata/benchmark_expectations b/pkg/bench/rttanalysis/testdata/benchmark_expectations index ad40f000e801..4f75451bfc89 100644 --- a/pkg/bench/rttanalysis/testdata/benchmark_expectations +++ b/pkg/bench/rttanalysis/testdata/benchmark_expectations @@ -92,3 +92,5 @@ exp,benchmark 18,Truncate/truncate_2_column_2_rows 1,VirtualTableQueries/select_crdb_internal.invalid_objects_with_1_fk 1,VirtualTableQueries/select_crdb_internal.tables_with_1_fk +6,VirtualTableQueries/virtual_table_cache_with_point_lookups +21,VirtualTableQueries/virtual_table_cache_with_schema_change diff --git a/pkg/bench/rttanalysis/virtual_table_bench_test.go b/pkg/bench/rttanalysis/virtual_table_bench_test.go index 5ab442a63273..8b0860ab8b50 100644 --- a/pkg/bench/rttanalysis/virtual_table_bench_test.go +++ b/pkg/bench/rttanalysis/virtual_table_bench_test.go @@ -35,5 +35,32 @@ CREATE TABLE t2 (i INT PRIMARY KEY, j INT REFERENCES t1(i)); `, Stmt: `SELECT * FROM "".crdb_internal.invalid_objects`, }, + // This checks that descriptors are still cached after they are written. We + // expect the second and third selects not to go to KV because the + // descriptors were cached after the first lookup. + { + Name: "virtual table cache with schema change", + Setup: ` +CREATE TABLE t1 (i INT PRIMARY KEY); +CREATE TABLE t2 (i INT PRIMARY KEY, j INT);`, + Stmt: ` +SELECT * FROM crdb_internal.tables; +ALTER TABLE t1 ADD COLUMN j INT; +SELECT * FROM crdb_internal.table_columns; +CREATE INDEX idx ON t2 (j); +SELECT * FROM crdb_internal.index_columns;`, + }, + // This checks that catalog point lookups following a virtual table scan + // access cached descriptors. + { + Name: "virtual table cache with point lookups", + Setup: ` +CREATE TABLE t1 (i INT PRIMARY KEY); +CREATE TABLE t2 (i INT PRIMARY KEY, j INT);`, + Stmt: ` +SELECT * FROM crdb_internal.tables; +SELECT * FROM t1; +SELECT * FROM t2;`, + }, }) }