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.

Sign up for GitHub

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

Jump to bottom

persistedsqlstats: TestSQLStatsPersistedLimitReached flakes under stress and Bazel Essential CI #105846

Closed
zachlite opened this issue Jun 29, 2023 · 4 comments · Fixed by #105917
Closed

persistedsqlstats: TestSQLStatsPersistedLimitReached flakes under stress and Bazel Essential CI #105846

zachlite opened this issue Jun 29, 2023 · 4 comments · Fixed by #105917
Labels
C-test-failure Broken test (automatically or manually discovered). skipped-test

Comments

@zachlite
Copy link
Contributor

zachlite commented Jun 29, 2023 •
edited by cockroach-jira-scripts
Loading

#105825 skips the test explicitly under stress, but we need to de-flake the stress test so this test can always be run.

Jira issue: CRDB-29231
@blathers-crl
Copy link

blathers-crl bot commented Jun 29, 2023

Hi @zachlite, please add a C-ategory label to your issue. Check out the label system docs.

While you're here, please consider adding an A- label to help keep our repository tidy.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@zachlite zachlite added the C-test-failure Broken test (automatically or manually discovered). label Jun 29, 2023
@rafiss
Copy link
Collaborator

rafiss commented Jun 30, 2023

This also fails under Bazel Essential CI, without stress.

    flush_test.go:517:
          Error Trace:  pkg/sql/sqlstats/persistedsqlstats/persistedsqlstats_test_test/pkg/sql/sqlstats/persistedsqlstats/flush_test.go:517
          Error:        "16" is not less than or equal to "15"
          Test:         TestSQLStatsPersistedLimitReached

https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_BazelEssentialCi/10735309?hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true&expandBuildProblemsSection=true&expandBuildTestsSection=true

@rafiss rafiss changed the title persistedsqlstats: TestSQLStatsPersistedLimitReached flakes under stress persistedsqlstats: TestSQLStatsPersistedLimitReached flakes under stress and Bazel Essential CI Jun 30, 2023
@rafiss rafiss mentioned this issue Jun 30, 2023
Merged
@tbg
Copy link
Member

tbg commented Jun 30, 2023

Also happened here https://teamcity.cockroachdb.com/viewLog.html?buildId=10742161&tab=buildResultsDiv&buildTypeId=Cockroach_UnitTests_BazelUnitTests

zachlite added a commit to zachlite/cockroach that referenced this issue Jun 30, 2023
@zachlite
persistedsqlstats: skip TestSQLStatsPersistedLimitReached
0e46af9 
Flakes in Bazel essential CI.

Informs cockroachdb#105846, cockroachdb#97488
Epic: none
Release note: None
@zachlite zachlite mentioned this issue Jun 30, 2023
Merged
craig bot pushed a commit that referenced this issue Jun 30, 2023
@chengxiong-ruan @zachlite
Merge #105758 #105893
947a636 
105758: sql: skip multiregional tests again r=chengxiong-ruan a=chengxiong-ruan

Unfortunately, it's still flaky:
#105346 (comment)

Informs: #98020

Release note: None

105893: persistedsqlstats: skip TestSQLStatsPersistedLimitReached r=zachlite a=zachlite

Flakes in Bazel essential CI.
Reverts #105770

Informs #105846, #97488
Epic: none
Release note: None

Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: zachlite <[email protected]>
@blathers-crl blathers-crl bot mentioned this issue Jun 30, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Jun 30, 2023
@zachlite
persistedsqlstats: skip TestSQLStatsPersistedLimitReached
b9747c9 
Flakes in Bazel essential CI.

Informs #105846, #97488
Epic: none
Release note: None
zachlite added a commit to zachlite/cockroach that referenced this issue Jun 30, 2023
@zachlite
persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached
9ba1daa 
TestSQLStatsPersistedLimitReached would previously flake
because it did not consider background SQL activity happening
in the cluster.

This commit re-writes the test to only make assertions that are
not dependent on background cluster activity.

Resolves cockroachdb#105846, cockroachdb#97488
Epic: none
Release note: None
@zachlite zachlite mentioned this issue Jun 30, 2023
Merged
zachlite added a commit to zachlite/cockroach that referenced this issue Jun 30, 2023
@zachlite
persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached
dc4da0a 
TestSQLStatsPersistedLimitReached would previously flake
because it did not consider background SQL activity happening
in the cluster.

This commit re-writes the test to only make assertions that are
not dependent on background cluster activity.

Resolves cockroachdb#105846, cockroachdb#97488
Epic: none
Release note: None
zachlite added a commit to zachlite/cockroach that referenced this issue Jun 30, 2023
@zachlite
persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached
f93c423 
TestSQLStatsPersistedLimitReached would previously flake
because it did not consider background SQL activity happening
in the cluster.

This commit re-writes the test to only make assertions that are
not dependent on background cluster activity.

Resolves cockroachdb#105846, cockroachdb#97488
Epic: none
Release note: None
craig bot pushed a commit that referenced this issue Jun 30, 2023
@zachlite
Merge #105917
9705c25 
105917: persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached r=zachlite a=zachlite

TestSQLStatsPersistedLimitReached would previously flake because it did not consider background SQL activity happening in the cluster.

This commit re-writes the test to only make assertions that are not dependent on background cluster activity.

Resolves #105846, #97488
Epic: none
Release note: None

Co-authored-by: zachlite <[email protected]>
@craig craig bot closed this as completed in a9a8a57 Jun 30, 2023
blathers-crl bot pushed a commit that referenced this issue Jun 30, 2023
@zachlite
persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached
b1301cd 
TestSQLStatsPersistedLimitReached would previously flake
because it did not consider background SQL activity happening
in the cluster.

This commit re-writes the test to only make assertions that are
not dependent on background cluster activity.

Resolves #105846, #97488
Epic: none
Release note: None
@blathers-crl blathers-crl bot mentioned this issue Jun 30, 2023
Merged
@THardy98
Copy link

THardy98 commented Jul 4, 2023

Fixed here: #105917

THardy98 pushed a commit to THardy98/cockroach that referenced this issue Jul 28, 2023
@zachlite
persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached
2962a31 
TestSQLStatsPersistedLimitReached would previously flake
because it did not consider background SQL activity happening
in the cluster.

This commit re-writes the test to only make assertions that are
not dependent on background cluster activity.

Resolves cockroachdb#105846, cockroachdb#97488
Epic: none
Release note: None
@THardy98 THardy98 mentioned this issue Jul 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-test-failure Broken test (automatically or manually discovered). skipped-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

persistedsqlstats: de-flake TestSQLStatsPersistedLimitReached zachlite/cockroach
4 participants
@rafiss @tbg @zachlite @THardy98