Skip to content

sql: fix insight integration test for contention#108165

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
koorosh:sql-fix-insights-integration-test-for-contention
Aug 5, 2023
Merged

sql: fix insight integration test for contention#108165
craig[bot] merged 1 commit intocockroachdb:masterfrom
koorosh:sql-fix-insights-integration-test-for-contention

Conversation

@koorosh
Copy link
Copy Markdown
Contributor

@koorosh koorosh commented Aug 4, 2023

This change lowers the latency threshold for which statement is considered slow to make sure it detects contention.
Previous value was set to 100ms which is default. New value is set to 30ms to be the same as overridden
value in other tests.

Release note: None

Resolves: #108020

This change lowers the latency threshold for which
statement is considered slow to make sure it detects
contention.
Previous value was set to 100ms which is default. New
value is set to 30ms to be the same as overridden
value in other tests.

Release note: None
@koorosh koorosh added the backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only label Aug 4, 2023
@koorosh koorosh requested review from a team, THardy98, gtr, j82w, maryliag, xinhaoz and zachlite August 4, 2023 13:25
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Aug 4, 2023

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

@blathers-crl blathers-crl bot added the O-community Originated from the community label Aug 4, 2023
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @gtr, @j82w, @THardy98, @xinhaoz, and @zachlite)

Copy link
Copy Markdown
Contributor

@j82w j82w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @gtr, @THardy98, @xinhaoz, and @zachlite)

@koorosh
Copy link
Copy Markdown
Contributor Author

koorosh commented Aug 4, 2023

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 4, 2023

Build failed:

@maryliag
Copy link
Copy Markdown
Contributor

maryliag commented Aug 4, 2023

bors retry

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 4, 2023

This PR was included in a batch that timed out, it will be automatically retried

@koorosh
Copy link
Copy Markdown
Contributor Author

koorosh commented Aug 4, 2023

bors retry

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 4, 2023

Already running a review

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 4, 2023

Build failed (retrying...):

craig bot pushed a commit that referenced this pull request Aug 4, 2023
108115: roachtest: don't reuse clusters that call dmsetup r=erikgrinaker,srosenberg a=andrewbaptist

Certain tests need to modify the blockdevice and they are prone to failures during setup that the device is still busy. Ideally we would figure out what is still holding onto the dish handle, but it is safer to simply not reuse clusters that perform this by adding `spec.ReuseNone()`

Fixes: #107865
Epic: none

Release note: None

108165: sql: fix insight integration test for contention r=koorosh a=koorosh

This change lowers the latency threshold for which statement is considered slow to make sure it detects contention.
Previous value was set to 100ms which is default. New value is set to 30ms to be the same as overridden
value in other tests.

Release note: None

Resolves: #108020

108192: ccl/sqlproxyccl: deflake TestConnectionMigration r=darinpp a=jaylim-crl

Fixes #106885.

This test flake seems extremely rare, and it's unclear why it occurred in the
first place. The past 1000 runs (all of what TC has) have been successful.
Regardless, this commit attempts at deflaking TestConnectionMigration. Given
that some subtests transfer connections through `transferConnWithRetries`, it
is possible that the transfer was retried, causing the metric to be
incremented.

Release note: None

Epic: none

Co-authored-by: Andrew Baptist <baptist@cockroachlabs.com>
Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>
Co-authored-by: Jay <jay@cockroachlabs.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 4, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 4, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 5, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 5, 2023

Build succeeded:

@craig craig bot merged commit 5cfe9fe into cockroachdb:master Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-23.1.x PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pkg/sql/sqlstats/insights/integration/integration_test: TestInsightsIntegrationForContention failed

4 participants