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

execbuilder: fix a rare flake #82642

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

yuzefovich
Copy link
Member

This commit fixes a rare flake in the vectorize_local test. I don't
quite understand why the test flakes tbh, but deduplicating the messages
from the trace makes it non-flaky while still ensuring that the test
exercises what it is intended to (that two spans are scanned in
parallel).

Fixes: #81636.

Release note: None

This commit fixes a rare flake in the `vectorize_local` test. I don't
quite understand why the test flakes tbh, but deduplicating the messages
from the trace makes it non-flaky while still ensuring that the test
exercises what it is intended to (that two spans are scanned in
parallel).

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@msirek msirek left a comment

Choose a reason for hiding this comment

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

If you think this could be more than a testing artifact, maybe we should open an issue to look into it. I guess it's hard to know, since it seems this is not easily reproducible. Could something be added to tracing to help debug it in the future?
:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)

@yuzefovich
Copy link
Member Author

To be honest, I'm very puzzled by this failure. It is reproducible, but quite rare - like once in several hundreds of runs, sometimes in a thousand. I thought it was related to a range split, so I added a SHOW RANGES query before the traced query, and I still saw a failure with the trace while the range boundaries were as expected.

My guess is still that it has something to do with the testing setup around the zone configs, range boundaries, etc, since the first failure is relatively recent, and we had some changes in that area, but I don't think it's worth investigating this issue further - I spent like an hour and a half on it already, and I highly doubt it's a correctness issue.

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 9, 2022

Build succeeded:

@craig craig bot merged commit 635792c into cockroachdb:master Jun 9, 2022
@yuzefovich yuzefovich deleted the fix-execbuilder-flake branch June 9, 2022 04:49
@yuzefovich
Copy link
Member Author

I did bisect of this failure and got to 77adcaf (which was my hypothesis too). So it looks like we could include the issue into the list of #75639. I still don't think it's worth spending more time on this. cc @irfansharif for visibility.

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.

sql/opt/exec/execbuilder: TestExecBuild failed
3 participants