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

sql: fix a non-deterministic test #35519

Merged
merged 1 commit into from
Mar 7, 2019
Merged

Conversation

knz
Copy link
Contributor

@knz knz commented Mar 7, 2019

Release note: None

@knz knz requested review from dt and bobvawter March 7, 2019 20:35
@knz knz requested a review from a team March 7, 2019 20:36
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@bobvawter bobvawter left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@knz
Copy link
Contributor Author

knz commented Mar 7, 2019

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 7, 2019

Merge conflict (retrying...)

craig bot pushed a commit that referenced this pull request Mar 7, 2019
35511: workload: use workload split functionality with tpcc r=ajwerner a=ajwerner

Before this PR the tpcc workloads dealt with splitting ranges on their own.
This splitting logic lacked the sophistication built in to the workload package
split implementation which works to mitigate imbalance.

Fixes #25872.

Release note: None

35512: c-deps: bump rocksdb for bottommost files bug fix r=ajkr a=ajkr

Pick up cockroachdb/rocksdb#26

Release note: None

35519: sql: fix a non-deterministic test r=knz a=knz

Release note: None

35520: build: Fix docker link for alpha/beta builds r=knz a=bdarnell

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Andrew Kryczka <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Ben Darnell <[email protected]>
@craig
Copy link
Contributor

craig bot commented Mar 7, 2019

Build succeeded

@craig craig bot merged commit eefa65e into cockroachdb:master Mar 7, 2019
craig bot pushed a commit that referenced this pull request Mar 8, 2019
35521: sql: reduce non-determinism in the show_trace logic tests r=knz a=knz

Fixes #33720.
First commit from #35519.

The test is really about asserting the KV operations sent on behalf of
SQL statements. The distsender traffic is really irrelevant. Since
that's where most of the test flakes / non-determinism is coming from,
simply remove it.

Release note: None

35548: sqlbase: avoid a race in sqlbase.CancelChecker r=knz a=knz

Fixes  #35539.

Prior to the distsql-ification of planNode execution, all the
execution steps in a local query were interleaved (using
coroutine-style concurrency), so that the calls to
`(*CancelChecker).Check()` were all sequential.

With distsql now it's possible for different planNode `Next()` or
`startExec()` methods to be running in concurrent goroutines on
multiple cores, i.e. truly parallel.

This in turn requires atomic access to the counter in the cancel
checker. Without atomic access, there is a race condition and the
possibility that the cancel checker does not work well (some
increments can be performed two times, which could cause the condition
of the check to occasionally fail).

Found with SQLSmith.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
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.

4 participants