Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rttanalysis: don't count leasing the database desc #93153

Merged
merged 2 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/bench/rttanalysis/rtt_analysis_bench.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,15 @@ func executeRoundTripTest(b testingB, tc RoundTripBenchTestCase, cc ClusterConst
// 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, "CREATE DATABASE bench")
// Make sure the database descriptor is leased, so that tests don't count
// the leasing.
sql.Exec(b, "USE bench")
// Also force a lease on the "public" schema too.
sql.Exec(b, "CREATE TABLE bench.public.__dummy__()")
sql.Exec(b, "SELECT 1 FROM bench.public.__dummy__")
sql.Exec(b, "DROP TABLE bench.public.__dummy__")

sql.Exec(b, tc.Setup)
for _, statement := range statements {
cluster.clearStatementTrace(statement.SQL)
Expand Down
76 changes: 38 additions & 38 deletions pkg/bench/rttanalysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ 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
13,CreateRole/create_role_with_1_option
16,CreateRole/create_role_with_2_options
19,CreateRole/create_role_with_3_options
14,CreateRole/create_role_with_no_options
12,CreateRole/create_role_with_1_option
15,CreateRole/create_role_with_2_options
18,CreateRole/create_role_with_3_options
13,CreateRole/create_role_with_no_options
10,DropDatabase/drop_database_0_tables
11,DropDatabase/drop_database_1_table
11,DropDatabase/drop_database_2_tables
Expand All @@ -47,46 +47,46 @@ exp,benchmark
12,DropView/drop_1_view
13,DropView/drop_2_views
13,DropView/drop_3_views
10,Grant/grant_all_on_1_table
10,Grant/grant_all_on_2_tables
10,Grant/grant_all_on_3_tables
9,Grant/grant_all_on_1_table
9,Grant/grant_all_on_2_tables
9,Grant/grant_all_on_3_tables
11,GrantRole/grant_1_role
15,GrantRole/grant_2_roles
4,ORMQueries/activerecord_type_introspection_query
6,ORMQueries/django_table_introspection_1_table
6,ORMQueries/django_table_introspection_4_tables
6,ORMQueries/django_table_introspection_8_tables
2,ORMQueries/has_column_privilege_using_attnum
2,ORMQueries/has_column_privilege_using_column_name
1,ORMQueries/has_schema_privilege_1
1,ORMQueries/has_schema_privilege_3
1,ORMQueries/has_schema_privilege_5
2,ORMQueries/has_sequence_privilege_1
4,ORMQueries/has_sequence_privilege_3
6,ORMQueries/has_sequence_privilege_5
2,ORMQueries/has_table_privilege_1
4,ORMQueries/has_table_privilege_3
6,ORMQueries/has_table_privilege_5
3,ORMQueries/activerecord_type_introspection_query
5,ORMQueries/django_table_introspection_1_table
5,ORMQueries/django_table_introspection_4_tables
5,ORMQueries/django_table_introspection_8_tables
1,ORMQueries/has_column_privilege_using_attnum
1,ORMQueries/has_column_privilege_using_column_name
0,ORMQueries/has_schema_privilege_1
0,ORMQueries/has_schema_privilege_3
0,ORMQueries/has_schema_privilege_5
1,ORMQueries/has_sequence_privilege_1
3,ORMQueries/has_sequence_privilege_3
5,ORMQueries/has_sequence_privilege_5
1,ORMQueries/has_table_privilege_1
3,ORMQueries/has_table_privilege_3
5,ORMQueries/has_table_privilege_5
85,ORMQueries/hasura_column_descriptions
85,ORMQueries/hasura_column_descriptions_8_tables
6,ORMQueries/hasura_column_descriptions_modified
6,ORMQueries/information_schema._pg_index_position
5,ORMQueries/pg_attribute
5,ORMQueries/pg_class
7,ORMQueries/pg_is_other_temp_schema
7,ORMQueries/pg_is_other_temp_schema_multiple_times
4,ORMQueries/pg_my_temp_schema
4,ORMQueries/pg_my_temp_schema_multiple_times
4,ORMQueries/pg_namespace
5,ORMQueries/pg_type
10,Revoke/revoke_all_on_1_table
10,Revoke/revoke_all_on_2_tables
10,Revoke/revoke_all_on_3_tables
5,ORMQueries/hasura_column_descriptions_modified
5,ORMQueries/information_schema._pg_index_position
4,ORMQueries/pg_attribute
4,ORMQueries/pg_class
6,ORMQueries/pg_is_other_temp_schema
6,ORMQueries/pg_is_other_temp_schema_multiple_times
3,ORMQueries/pg_my_temp_schema
3,ORMQueries/pg_my_temp_schema_multiple_times
3,ORMQueries/pg_namespace
4,ORMQueries/pg_type
9,Revoke/revoke_all_on_1_table
9,Revoke/revoke_all_on_2_tables
9,Revoke/revoke_all_on_3_tables
9,RevokeRole/revoke_1_role
11,RevokeRole/revoke_2_roles
1,SystemDatabaseQueries/select_system.users_with_empty_database_Name
1,SystemDatabaseQueries/select_system.users_with_schema_Name
2,SystemDatabaseQueries/select_system.users_without_schema_Name
1,SystemDatabaseQueries/select_system.users_without_schema_Name
12,Truncate/truncate_1_column_0_rows
12,Truncate/truncate_1_column_1_row
12,Truncate/truncate_1_column_2_rows
Expand All @@ -95,5 +95,5 @@ exp,benchmark
12,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
4,VirtualTableQueries/virtual_table_cache_with_point_lookups
8,VirtualTableQueries/virtual_table_cache_with_schema_change
3,VirtualTableQueries/virtual_table_cache_with_point_lookups
7,VirtualTableQueries/virtual_table_cache_with_schema_change
18 changes: 14 additions & 4 deletions pkg/bench/rttanalysis/validate_benchmark_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ var (
rewriteIterations = flag.Int("rewrite-iterations", 50,
"if re-writing, the number of times to execute each benchmark to "+
"determine the range of possible values")
allowOffByOne = flag.Bool("allow-off-by-one", true,
"if set, expectations that are not a range get a ±1 tolerance")
)

// RunBenchmarkExpectationTests runs tests to validate or rewrite the contents
Expand Down Expand Up @@ -319,10 +321,18 @@ func (b benchmarkExpectations) find(name string) (benchmarkExpectation, bool) {
}

func (e benchmarkExpectation) matches(roundTrips int) bool {
// Either the value falls within the expected range, or
return (e.min <= roundTrips && roundTrips <= e.max) ||
// the expectation isn't a range, so give it a leeway of one.
e.min == e.max && (roundTrips == e.min-1 || roundTrips == e.min+1)
// Does the value fall in the range?
if e.min <= roundTrips && roundTrips <= e.max {
return true
}

// If the expectation isn't a range, it gets a leeway of one because we got
// tired of small indeterminism.
if (e.min == e.max) && *allowOffByOne && (roundTrips == e.min-1 || roundTrips == e.min+1) {
return true
}

return false
}

func (e benchmarkExpectation) String() string {
Expand Down