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

perf: upgrade from go1.10 to go1.11 #29827

Closed
nvanbenschoten opened this issue Sep 7, 2018 · 8 comments
Closed

perf: upgrade from go1.10 to go1.11 #29827

nvanbenschoten opened this issue Sep 7, 2018 · 8 comments
Assignees
Labels
C-performance Perf of queries or internals. Solution not expected to change functional behavior.

Comments

@nvanbenschoten
Copy link
Member

go1.11 was recently released: https://golang.org/doc/go1.11. I ran our standard benchmarks against it to get an idea of what kind of performance we can expect when we upgrade. This will may influence our decision of whether the upgrade before or after our 2.1 release.

All benchmarks were run on GCE n1-standard-4 linux machines with the nobarrier SSD mount option. I ran 3 benchmarks, kv0, kv95, and tpcc1-nowait. Each benchmark was run on a single node cluster and on a three node cluster. Each configuration was run 5 times with go1.10 and 5 times with go1.11. Each trial was run for 3 minutes, with a cluster wipe in-between.

name               old ops/sec  new ops/sec  delta
kv0-1node           5.78k ± 4%   6.41k ± 3%  +10.93%  (p=0.008 n=5+5)
kv0-3node           7.11k ± 2%   7.24k ± 1%   +1.81%  (p=0.056 n=5+5)
kv95-1node          9.63k ± 3%  10.58k ± 2%   +9.89%  (p=0.008 n=5+5)
kv95-3node          16.7k ± 1%   16.8k ± 4%   +0.52%  (p=0.151 n=5+5)
tpcc-nowait-1node    75.6 ± 4%    78.2 ± 3%   +3.49%  (p=0.056 n=5+5)
tpcc-nowait-3node    49.8 ± 2%    49.4 ± 5%     ~     (p=1.000 n=5+5)

name               old p50(ms)  new p50(ms)  delta
kv0-1node            10.5 ± 0%     9.4 ± 0%  -10.48%  (p=0.029 n=4+4)
kv0-3node            7.90 ± 0%    7.78 ± 2%     ~     (p=0.333 n=4+5)
kv95-1node           6.30 ± 0%    5.80 ± 0%   -7.94%  (p=0.079 n=4+5)
kv95-3node           3.44 ± 2%    3.40 ± 0%     ~     (p=0.444 n=5+5)
tpcc-nowait-1node    88.1 ± 5%    86.4 ± 3%     ~     (p=0.643 n=5+5)
tpcc-nowait-3node     176 ± 5%     178 ± 6%     ~     (p=0.881 n=5+5)

name               old p99(ms)  new p99(ms)  delta
kv0-1node            29.2 ± 4%    26.4 ± 5%   -9.33%  (p=0.008 n=5+5)
kv0-3node            28.3 ± 0%    28.3 ± 0%     ~     (all equal)
kv95-1node           19.9 ± 0%    17.8 ± 0%  -10.55%  (p=0.079 n=4+5)
kv95-3node           16.3 ± 0%    15.9 ± 2%   -2.21%  (p=0.095 n=4+5)
tpcc-nowait-1node     631 ± 6%     617 ± 3%     ~     (p=0.683 n=5+5)
tpcc-nowait-3node     772 ± 0%     765 ± 5%     ~     (p=0.968 n=4+5)

cc. @petermattis @a-robinson @benesch

@nvanbenschoten nvanbenschoten added the C-performance Perf of queries or internals. Solution not expected to change functional behavior. label Sep 7, 2018
@nvanbenschoten nvanbenschoten added this to the 2.1 milestone Sep 7, 2018
@nvanbenschoten nvanbenschoten self-assigned this Sep 7, 2018
@a-robinson
Copy link
Contributor

I'd go for it, but that would have been my answer before seeing the numbers as well. It's too bad it only makes a real difference on single-node clusters.

@nvanbenschoten
Copy link
Member Author

I inventoried the changes that will be included in go1.11.1. I found three that I think we should be aware of:

I'm not able to find any estimate of when go1.11.1 will be released, but previous releases have typically gone about 6 weeks between the minor release and the first patch release. This would put the release of go1.10.1 somewhere in early October.

My suggestion would be to upgrade the betas to go1.11 now and update to go1.11.1 as soon as it is available. Thoughts @bdarnell @petermattis?

@benesch
Copy link
Contributor

benesch commented Sep 10, 2018

At the very least I think we should bump master to go1.11. I'm updating the builder now.

@nvanbenschoten
Copy link
Member Author

Thanks @benesch.

@petermattis
Copy link
Collaborator

@nvanbenschoten Those 3 changes are all a bit disconcerting, though it seems extremely likely that go1.11.1 will be released before we release 2.1.

@nvanbenschoten
Copy link
Member Author

Yes, that was my thought as well. I think we should move to go1.11 for the next beta release so that we can begin testing with it as soon as possible.

benesch added a commit to benesch/cockroach that referenced this issue Sep 17, 2018
Go 1.11 brings us some nice performance improvements on single-node
clusters. See cockroachdb#29827 for benchmark results.

Fix cockroachdb#29827.

Release note: None
craig bot pushed a commit that referenced this issue Sep 17, 2018
30035: builder: upgrade to go1.11 r=bdarnell,raduberinde,nvanbenschoten a=benesch

Go 1.11 brings us some nice performance improvements on single-node
clusters. See #29827 for benchmark results.

Fix #29827.

Release note: None

30285: gossip: Don't include gossip connectivity in periodic status logs r=a-robinson a=a-robinson

As discussed on #30088. It can be absolutely massive on large clusters,
and isn't needed because we still log the connectivity info whenever it
changes.

Release note: None

I still think that logging on every change in connectivity is going to be a mess in large clusters during rolling restarts even if gossip is otherwise perfectly stable, but this is useful regardless of whether we do anything about that.

Co-authored-by: Nikhil Benesch <[email protected]>
Co-authored-by: Alex Robinson <[email protected]>
@craig craig bot closed this as completed in #30035 Sep 17, 2018
benesch added a commit to benesch/cockroach that referenced this issue Sep 17, 2018
Go 1.11 brings us some nice performance improvements on single-node
clusters. See cockroachdb#29827 for benchmark results.

Fix cockroachdb#29827.

Release note: None
@benesch benesch reopened this Sep 18, 2018
@benesch
Copy link
Contributor

benesch commented Sep 18, 2018

The upgrade was rolled back due to golang/go#27660.

@nvanbenschoten
Copy link
Member Author

It doesn't look like we're going to release v2.1 with go1.11, so moving to 2.2.

@nvanbenschoten nvanbenschoten modified the milestones: 2.1, 2.2 Oct 1, 2018
@petermattis petermattis removed this from the 2.2 milestone Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Development

No branches or pull requests

4 participants