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

server: deflake TestRapidRestarts #29230

Merged

Conversation

petermattis
Copy link
Collaborator

In go1.10 and earlier, it was not safe to call http.ServeMux.ServeHTTP
concurrently with http.ServeMux.Handle. (This is fixed in go1.11). In
the interim, provide our own safeServeMux wrapper that provides proper
locking.

Fixes #29227

Release note: None

@petermattis petermattis requested a review from a team August 29, 2018 02:09
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@petermattis petermattis requested a review from tbg August 29, 2018 02:09
@petermattis
Copy link
Collaborator Author

TODO: This deserves a more dedicate test as I haven't been able to get TestRapidRestarts to fail reliably.

@petermattis petermattis force-pushed the pmattis/deflake-test-rapid-restarts branch from 4650cd2 to ac5957b Compare August 29, 2018 12:33
@petermattis
Copy link
Collaborator Author

Added a dedicated test which fails reliably with http.ServeMux and passes with safeServeMux.

bors r=benesch

In go1.10 and earlier, it was not safe to call `http.ServeMux.ServeHTTP`
concurrently with `http.ServeMux.Handle`. (This is fixed in go1.11). In
the interim, provide our own safeServeMux wrapper that provides proper
locking.

Fixes cockroachdb#29227

Release note: None
@petermattis petermattis force-pushed the pmattis/deflake-test-rapid-restarts branch from ac5957b to f932530 Compare August 29, 2018 12:59
@craig
Copy link
Contributor

craig bot commented Aug 29, 2018

Canceled

@petermattis
Copy link
Collaborator Author

bors r=benesch

craig bot pushed a commit that referenced this pull request Aug 29, 2018
29230: server: deflake TestRapidRestarts r=benesch a=petermattis

In go1.10 and earlier, it was not safe to call `http.ServeMux.ServeHTTP`
concurrently with `http.ServeMux.Handle`. (This is fixed in go1.11). In
the interim, provide our own safeServeMux wrapper that provides proper
locking.

Fixes #29227

Release note: None

29236: Revert "storage: enable the merge queue by default" r=tschottdorf a=benesch

This reverts commit 98ca1d0. The merge
queue will be reenabled once flaky tests are fixed.

To reviewers: I'd much prefer to merge #29235. But if that gets stuck in code review or the flakiness reaches a breaking point, feel free to merge this instead.

Co-authored-by: Peter Mattis <[email protected]>
Co-authored-by: Nikhil Benesch <[email protected]>
@craig
Copy link
Contributor

craig bot commented Aug 29, 2018

Build succeeded

@craig craig bot merged commit f932530 into cockroachdb:master Aug 29, 2018
@petermattis petermattis deleted the pmattis/deflake-test-rapid-restarts branch August 29, 2018 13:27
craig bot pushed a commit that referenced this pull request Aug 29, 2018
29259: release-2.1: server: deflake TestRapidRestarts r=benesch a=petermattis

Backport 1/1 commits from #29230.

/cc @cockroachdb/release

---

In go1.10 and earlier, it was not safe to call `http.ServeMux.ServeHTTP`
concurrently with `http.ServeMux.Handle`. (This is fixed in go1.11). In
the interim, provide our own safeServeMux wrapper that provides proper
locking.

Fixes #29227

Release note: None


Co-authored-by: Peter Mattis <[email protected]>
aayushshah15 added a commit to aayushshah15/cockroach that referenced this pull request Jul 23, 2020
`safeServeMux` was a wrapper to provide proper locking around
`http.ServeMux` because in go1.10 and below, calls to `Handle` and
`ServeHTTP` could race with each other.

We don't need it anymore, see cockroachdb#29230.

Release note: None
aayushshah15 added a commit to aayushshah15/cockroach that referenced this pull request Jul 24, 2020
`safeServeMux` was a wrapper to provide proper locking around
`http.ServeMux` because in go1.10 and below, calls to `Handle` and
`ServeHTTP` could race with each other.

We don't need it anymore, see cockroachdb#29230.

Release note: None
aayushshah15 added a commit to aayushshah15/cockroach that referenced this pull request Aug 13, 2020
`safeServeMux` was a wrapper to provide proper locking around
`http.ServeMux` because in go1.10 and below, calls to `Handle` and
`ServeHTTP` could race with each other.

We don't need it anymore, see cockroachdb#29230.

Release note: None
aayushshah15 added a commit to aayushshah15/cockroach that referenced this pull request Aug 26, 2020
`safeServeMux` was a wrapper to provide proper locking around
`http.ServeMux` because in go1.10 and below, calls to `Handle` and
`ServeHTTP` could race with each other.

We don't need it anymore, see cockroachdb#29230.

Release note: None
dhartunian pushed a commit to dhartunian/cockroach that referenced this pull request Sep 22, 2020
`safeServeMux` was a wrapper to provide proper locking around
`http.ServeMux` because in go1.10 and below, calls to `Handle` and
`ServeHTTP` could race with each other.

We don't need it anymore, see cockroachdb#29230.

Release note: None
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.

acceptance: TestRapidRestarts CI failures
3 participants