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

lease: TestLeaseAcquireAndReleaseConcurrencly is skipped #51798

Closed
jordanlewis opened this issue Jul 22, 2020 · 3 comments · Fixed by #92679
Closed

lease: TestLeaseAcquireAndReleaseConcurrencly is skipped #51798

jordanlewis opened this issue Jul 22, 2020 · 3 comments · Fixed by #92679
Assignees
@ajwerner
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@jordanlewis
Copy link
Member

jordanlewis commented Jul 22, 2020 •
edited by cockroach-jira-scripts
Loading

As of 12/1/17 (374413f):

	t.Skip("fails in the presence of migrations requiring backfill, but cannot import sqlmigrations")

Jira issue: CRDB-4011
@jordanlewis jordanlewis added the C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. label Jul 22, 2020
@jlinder jlinder added the T-sql-schema-deprecated Use T-sql-foundations instead label Jun 16, 2021
@andreimatei
Copy link
Contributor

Why was this closed? The test is still skipped and references this issue:

cockroach/pkg/sql/catalog/lease/lease_internal_test.go

Line 836 in 2675c7c

skip.WithIssue(t, 51798, "fails in the presence of migrations requiring backfill, "+

The code says:

	skip.WithIssue(t, 51798, "fails in the presence of migrations requiring backfill, "+
		"but cannot import startupmigrations")

Migrations requiring backfill in the way that this test means are gone. The startupmigrations package is also about to be gone. But the test crashes if unskipped, so it must have rotted somehow.

@andreimatei andreimatei reopened this Nov 15, 2022
@andreimatei
Copy link
Contributor

cc @ajwerner

@postamar postamar removed their assignment Nov 22, 2022
@postamar
Copy link
Contributor

I honestly have no clue why I closed this.

@ajwerner ajwerner mentioned this issue Nov 29, 2022
Merged
craig bot pushed a commit that referenced this issue Nov 30, 2022
@ajwerner @andreimatei
Merge #92679 #92681
d764ec1 
92679: lease: revive TestLeaseAcquireAndReleaseConcurrencly r=ajwerner a=ajwerner

This test relied on an old placement of a testing knob (it's been moved back), and it relied on the leasing subsystem interacting with the lease table itself in interesting ways. We sidestep this latter bit by making the test use a new table. Lastly, we allow other concurrent leasing activity to not disturb the test.

Fixes #51798

Release note: None

92681: server: minor cleanup r=andreimatei a=andreimatei

Get the tenant's tracer more directly.

Release note: None
Epic: None

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
@craig craig bot closed this as completed in 2bd6fd2 Nov 30, 2022
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ajwerner ajwerner

Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

lease: revive TestLeaseAcquireAndReleaseConcurrencly ajwerner/cockroach
6 participants
@jordanlewis @jlinder @andreimatei @ajwerner @postamar @tbg