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

*: de-flake TestPaginatedBackupTenant and remove data race #74926

Merged
merged 3 commits into from
Jan 18, 2022

Conversation

irfansharif
Copy link
Contributor

@irfansharif irfansharif commented Jan 18, 2022

See individual commits; this PR squashes a data race in KV among other things.


This test readily fails under stress with span configs enabled (#73876).
Now that we split within the tenant keyspace for tenant tables, the
export requests generated for each BACKUP is liable to be retried if
concurrent with range splits. The splits causes KV to error out with
RangeKeyMismatchErrors, at which point the request is internally
retried. The test, which wants to assert on what ExportSpan requests are
intercepted, previously did not need to consider these retries/need to
de-dup -- it now does.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@irfansharif irfansharif requested a review from a team as a code owner January 18, 2022 04:17
@irfansharif irfansharif removed request for a team January 18, 2022 04:18
@irfansharif irfansharif linked an issue Jan 18, 2022 that may be closed by this pull request
This test readily fails under stress with span configs enabled (cockroachdb#73876).
Now that we split within the tenant keyspace for tenant tables, the
export requests generated for each BACKUP is liable to be retried if
concurrent with range splits. The splits causes KV to error out with
RangeKeyMismatchErrors, at which point the request is internally
retried. The test, which wants to assert on what ExportSpan requests are
intercepted, previously did not need to consider these retries/need to
de-dup -- it now does.

Release note: None
This test got a lot slower under the span configs infrastructure,
something we're investigating as part of cockroachdb#74919. Until then, prevent
this test from spoiling builds.

Release note: None
Fixes cockroachdb#74932; we were reading from the replicas map without having
acquired the prerequisite lock.

Release note: None
@irfansharif irfansharif force-pushed the 220117.deflake-spanconfigs branch from 3aaf50b to 3218353 Compare January 18, 2022 12:37
Copy link
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

The "sqush race" commit needs to be backed out.

@irfansharif
Copy link
Contributor Author

bors r+

@irfansharif irfansharif changed the title backupccl: de-flake TestPaginatedBackupTenant backupccl: de-flake TestPaginatedBackupTenant + remove data race Jan 18, 2022
@irfansharif irfansharif changed the title backupccl: de-flake TestPaginatedBackupTenant + remove data race *: de-flake TestPaginatedBackupTenant and remove data race Jan 18, 2022
@craig
Copy link
Contributor

craig bot commented Jan 18, 2022

Build succeeded:

Copy link
Collaborator

@arulajmani arulajmani left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r4, 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @adityamaru, @ajwerner, @irfansharif, @nvanbenschoten, and @tbg)

@craig craig bot merged commit 922e498 into cockroachdb:master Jan 18, 2022
@irfansharif irfansharif deleted the 220117.deflake-spanconfigs branch January 18, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

importccl: TestCSVImportCanBeResumed failed under testrace
4 participants