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

kvserver: minor test cleanup #93363

Merged

Conversation

andreimatei
Copy link
Contributor

@andreimatei andreimatei commented Dec 10, 2022

See individual commits.

This test was uselessly looking at the cancelation of an uncancellable
context.

Release note: None
Epic: None
@andreimatei andreimatei requested a review from tbg December 10, 2022 00:00
@andreimatei andreimatei requested a review from a team as a code owner December 10, 2022 00:00
@cockroach-teamcity
Copy link
Member

This change is Reviewable

This test was jumping through hoops in order to combine a cluster
stopper, with its own Tracer, with calling into a node, which has a
different Tracer. Normally combining Tracers like that is illegal, but
the test was making sure to force the Stopper to create "sterile" spans
for its tasks, which are exempt from this restriction about combining
them with other Tracers, for arcane reasons. This patch switches the
test to a more straight-forward hoop: don't use a context created by the
stopper when calling into the node. Besides being simpler, this is
needed cause the exception for sterile spans is going away.

Release note: None
Epic: None
Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

friendly ping

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

@andreimatei
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 14, 2022

Build succeeded:

@craig craig bot merged commit dc9cfae into cockroachdb:master Dec 14, 2022
@andreimatei andreimatei deleted the small.consistency-queue-test-cancel branch December 14, 2022 22:10
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.

3 participants