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

kvstreamer: extend a test to assert that the streamer is used #82671

Merged
merged 1 commit into from
Jun 14, 2022

Conversation

yuzefovich
Copy link
Member

This commit extends an existing test which examines the trace of
different queries to assert that the streamer is used in all cases when
we expect. It also simplifies some of the queries as well as adjusts the
workmem limit which got out of sync with the index join memory limiting
behavior which was changed recently.

Fixes: #82646.

Release note: None

@yuzefovich yuzefovich requested review from rharding6373, michae2 and a team June 9, 2022 15:07
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@rharding6373 rharding6373 left a comment

Choose a reason for hiding this comment

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

Great, thank you! :lgtm:

Reviewed all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @michae2)

@yuzefovich
Copy link
Member Author

TFTR!

bors r+

@yuzefovich
Copy link
Member Author

Looks like the test might be flaky.

bors r-

@craig
Copy link
Contributor

craig bot commented Jun 9, 2022

Canceled.

@yuzefovich
Copy link
Member Author

The adjusted test started being flaky in rare circumstances due to join reader not behaving under memory pressure for the lookup join with ordering test case. Opened up #82693 to fix that and will merge this PR once that PR goes in.

This commit extends an existing test which examines the trace of
different queries to assert that the streamer is used in all cases when
we expect. It also simplifies some of the queries as well as adjusts the
workmem limit which got out of sync with the index join memory limiting
behavior which was changed recently.

Release note: None
@yuzefovich yuzefovich removed the do-not-merge bors won't merge a PR with this label. label Jun 14, 2022
@yuzefovich
Copy link
Member Author

Should be good now.

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 14, 2022

Build succeeded:

@craig craig bot merged commit bd5462a into cockroachdb:master Jun 14, 2022
@yuzefovich yuzefovich deleted the streamer-tests branch June 14, 2022 05:03
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.

kvstreamer: add tests to verify that the streamer is used when we expect it to be
3 participants