From 9c6fcd19a0892592b11bc803408b6daad0c780dc Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Tue, 21 Mar 2023 15:57:46 -0500 Subject: [PATCH] sql/logictest: disable stats collection for system tables earlier This commit updates the logictests to disable stats collection for system tables before the test cluster is started. This avoids a race condition where stats might be collected on system tables before collection is disabled with SET CLUSTER SETTING. This should prevent flakes for tests that show EXPLAIN output for queries over system tables. Fixes #99118 Release note: None --- pkg/sql/logictest/logic.go | 13 ++++++++----- .../opt/exec/execbuilder/testdata/explain_analyze | 8 ++------ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index e68a31e7bd4e..73732db60a79 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -1452,6 +1452,14 @@ func (t *logicTest) newCluster( stats.DefaultAsOfTime = 10 * time.Millisecond stats.DefaultRefreshInterval = time.Millisecond + // Disable stats collection on system tables before the cluster is started, + // otherwise there is a race condition where stats may be collected before we + // can disable them with `SET CLUSTER SETTING`. We disable stats collection on + // system tables in order to have deterministic tests. + stats.AutomaticStatisticsOnSystemTables.Override( + context.Background(), ¶ms.ServerArgs.TempStorageConfig.Settings.SV, false, + ) + t.cluster = serverutils.StartNewTestCluster(t.rootT, cfg.NumNodes, params) if cfg.UseFakeSpanResolver { fakeResolver := physicalplanutils.FakeResolverForTestCluster(t.cluster) @@ -1640,11 +1648,6 @@ func (t *logicTest) newCluster( ); err != nil { t.Fatal(err) } - if _, err := conn.Exec( - "SET CLUSTER SETTING sql.stats.system_tables_autostats.enabled = false", - ); err != nil { - t.Fatal(err) - } // We also disable stats forecasts to have deterministic tests. See #97003 // for details. diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze b/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze index 1dcaeb7afbe7..d9fa569fbf62 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze +++ b/pkg/sql/opt/exec/execbuilder/testdata/explain_analyze @@ -124,18 +124,14 @@ regions: statement ok GRANT SELECT ON crdb_internal.tables TO root; -# For some reason, even though we explicitly disable the automatic stats -# collection for both user and system tables, rarely we still see that the stats -# on system.privileges were collected, so we filter out a single line from the -# output to make the test deterministic. query T -SELECT * FROM [EXPLAIN (VERBOSE) SELECT * FROM system.privileges WHERE path = 'vtable/crdb_internal/tables'] - WHERE info NOT LIKE '%estimated row count%'; +EXPLAIN (VERBOSE) SELECT * FROM system.privileges WHERE path = 'vtable/crdb_internal/tables' ---- distribution: local vectorized: true · • scan columns: (username, path, privileges, grant_options, user_id) + estimated row count: 10 (missing stats) table: privileges@privileges_path_user_id_key spans: /"vtable/crdb_internal/tables"-/"vtable/crdb_internal/tables"/PrefixEnd