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

sql: TestCancelQueryPermissions flaking #63547

Closed
adityamaru opened this issue Apr 13, 2021 · 6 comments · Fixed by #70951
Closed

sql: TestCancelQueryPermissions flaking #63547

adityamaru opened this issue Apr 13, 2021 · 6 comments · Fixed by #70951
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@adityamaru
Copy link
Contributor

The TestCancelQueryPermissions flaked with:

=== RUN   TestCancelQueryPermissions/unpermissioned_users_cannot_cancel_other_users
panic: Fail in goroutine after TestCancelQueryPermissions/non-admins_with_CANCELQUERY_can_cancel_non-admins has completed

goroutine 365437 [running]:
testing.(*common).Fail(0xc00757e780)
	/usr/local/go/src/testing/testing.go:688 +0x125
testing.(*common).FailNow(0xc00757e780)
	/usr/local/go/src/testing/testing.go:710 +0x2b
testing.(*common).Fatalf(0xc00757e780, 0x47c9e4c, 0x1c, 0xc00ba7a4a0, 0x2, 0x2)
	/usr/local/go/src/testing/testing.go:806 +0x93
github.com/cockroachdb/cockroach/pkg/testutils/sqlutils.(*SQLRunner).ExpectErr(0xc002060fa8, 0x560fca0, 0xc00757e780, 0x47b8f45, 0x18, 0x47b6bed, 0x18, 0x0, 0x0, 0x0)
	/go/src/github.com/cockroachdb/cockroach/pkg/testutils/sqlutils/sql_runner.go:97 +0x1c5
github.com/cockroachdb/cockroach/pkg/sql_test.TestCancelQueryPermissions.func2.1(0xc0136594f0, 0xc002586340, 0xc00757e780)
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/run_control_test.go:327 +0xae
created by github.com/cockroachdb/cockroach/pkg/sql_test.TestCancelQueryPermissions.func2
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/run_control_test.go:318 +0x13d

Flake can be found at - https://teamcity.cockroachdb.com/viewLog.html?buildId=2876992&buildTypeId=Cockroach_UnitTests

Not skipping for now, as we have only seen one instance of it.

@adityamaru adityamaru added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Apr 13, 2021
@rafiss
Copy link
Collaborator

rafiss commented Apr 14, 2021

Interesting.. I unskipped this a month ago since I couldn't reproduce the flake under stress: #62132

@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 12, 2021
@tbg
Copy link
Member

tbg commented Jul 26, 2021

This has gotten much more prevalent, skipping this test.

The problem is that the test is doing illegal things. It leaks a bunch of goroutines that outlive the subtest and may end up accessing t.

@tbg
Copy link
Member

tbg commented Jul 26, 2021

(It uses t when there are test failures, so not saying the problem is just with the test. The problem is that the test explodes when it actually finds a problem).

tbg added a commit to tbg/cockroach that referenced this issue Jul 26, 2021
Refs: cockroachdb#63547

Reason: https://cockroachlabs.slack.com/archives/C016CAD2HQ8/p1627290710022700

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None
@tbg
Copy link
Member

tbg commented Jul 26, 2021

I should also mention, I tried to fix this test, but it's not easy since ctx cancellation doesn't work (lib/pq)? There isn't a good way to force the pg_sleep to terminate when the subtest is done.

@tbg
Copy link
Member

tbg commented Jul 26, 2021

(One way to do it is to set up a new testserver for each subtest)

craig bot pushed a commit that referenced this issue Jul 26, 2021
68038: build: remove pointless rebuilds of cluster-ui r=jordanlewis a=jordanlewis

Since 60916fb, any change to any file in any subdirectory of pkg/ui
would cause a full rebuild of pkg/ui/cluster-ui, which would then cause
a full rebuild of pkg/ui as well.

Now, pkg/ui/cluster-ui's source files are tracked separately, so changes
to pkg/ui/ that don't touch cluster-ui won't force a rebuild of
cluster-ui.

This should save a lot of time, hopefully. Touching a single .tsx file
in pkg/ui/src used to cause 30+ seconds of build time due to this issue,
now it is significantly faster since the various webpack caches can
actually be used again.

Release note: None

cc @cockroachdb/cluster-ui-prs @cockroachdb/sql-observability 

68059: sql: skip TestCancelQueryPermissions r=irfansharif a=tbg

Refs: #63547

Reason: https://cockroachlabs.slack.com/archives/C016CAD2HQ8/p1627290710022700

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None

Co-authored-by: Jordan Lewis <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
@craig craig bot closed this as completed in aa7b70b Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants