sql/logictest: disable stats collection for system tables earlier#99155
Conversation
yuzefovich
left a comment
There was a problem hiding this comment.
Maybe also revert #97775 then?
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @msirek)
msirek
left a comment
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rytaft)
pkg/sql/logictest/logic.go line 1460 at r1 (raw file):
stats.AutomaticStatisticsOnSystemTables.Override( context.Background(), ¶ms.ServerArgs.TempStorageConfig.Settings.SV, false, )
Should we do this for sql.stats.automatic_collection.enabled as well, which is also using SET CLUSTER SETTING below?
a160c61 to
7f7fc3e
Compare
rytaft
left a comment
There was a problem hiding this comment.
TFTRs!
Maybe also revert #97775 then?
Done.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @msirek and @yuzefovich)
pkg/sql/logictest/logic.go line 1460 at r1 (raw file):
Previously, msirek (Mark Sirek) wrote…
Should we do this for
sql.stats.automatic_collection.enabledas well, which is also usingSET CLUSTER SETTINGbelow?
It's not really needed for that one, since we won't have created any non-system tables by that point, so there's no race condition. I think I'll leave it there, if you don't mind, since there's a big comment and a bunch of other stats settings getting updated in the same place.
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 cockroachdb#99118 Release note: None
7f7fc3e to
9c6fcd1
Compare
rytaft
left a comment
There was a problem hiding this comment.
bors r+
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @msirek and @yuzefovich)
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
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
EXPLAINoutput for queries over system tables.Fixes #99118
Release note: None