Skip to content

Conversation

@knz
Copy link
Contributor

@knz knz commented Apr 24, 2019

(will merge after 19.1 is released)

Fixes #37114.

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

Prior to this patch, this test would fail `stressrace` after a few
dozen iterations. The root cause of this was the invalid call to
`t.Parallel()`, which this patch removes.

Additionally, this patch adapts TimeUntilStoreDead for each test case
to avoid flakes, and removes a previous hack obviated by this
simplification.

Release note: None

Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
@knz knz requested review from a team, RaduBerinde and tbg April 24, 2019 13:58
@cockroach-teamcity
Copy link
Member

This change is Reviewable

knz added 3 commits April 24, 2019 16:01
Sub-tests that invoke `t.Parallel()` get to run concurrently with
their parent test, and may be delayed arbitrarily past beyond the end
of the termination of the parent test (including beyond the execution
of its `defer` calls).

This means it's unsafe to call `t.Parallel()` with
e.g. `leaktest.AfterTest()`.

This patch removes `t.Parallel` from the test in
`sql/physical_props_test.go` and modifies
`pkg/rpc/TestGRPCKeepaliveFailureFailsInflightRPCs` to use a wait
group instead.

Release note: None
…ightRPCs

Since this now runs on its own goroutine, it is invalid for it to
access the surrounding `testing.T`. This patch moves the code so that
`t` is clearly out of scope.

Release note: None
This suggests using `sync.WaitGroup` instead.

Release note: None
@knz knz force-pushed the backport19.1-36952-37072 branch from 667e01e to c9b31da Compare April 24, 2019 14:02
@tbg
Copy link
Member

tbg commented Apr 29, 2019

@knz I think this is good to go now, the release shas have been picked. :shipit:

@knz
Copy link
Contributor Author

knz commented Apr 29, 2019

yeah I'm not going to be too trigger happy on this one, I'll merge when the release is actually out

@knz knz merged commit 7675772 into cockroachdb:release-19.1 May 1, 2019
@knz knz deleted the backport19.1-36952-37072 branch May 1, 2019 19:59
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