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.

Sign up for GitHub

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

Jump to bottom

Update go-etcd client library version and add ugorji codec to dependencies #9390

Merged
merged 1 commit into from
Jun 8, 2015
Merged

Update go-etcd client library version and add ugorji codec to dependencies #9390

merged 1 commit into from
Jun 8, 2015

Conversation

fgrzadkowski
Copy link
Contributor

This improves performance of reading responses from etcd server.

Latency metrics in 100 node cluster with 8-core master during density test (30 pods per node):

INFO: Top latency metric: {Verb:LIST Resource:pods Quantile:0.99 Latency:1.013133s}
INFO: Top latency metric: {Verb:LIST Resource:pods Quantile:0.9 Latency:1.013133s}
INFO: Top latency metric: {Verb:POST Resource:services Quantile:0.99 Latency:981.668ms}
INFO: Top latency metric: {Verb:POST Resource:limitranges Quantile:0.99 Latency:564.016ms}
INFO: Top latency metric: {Verb:POST Resource:services Quantile:0.9 Latency:512.61ms}

Ref #4521
@wojtek-t @davidopp

@k8s-bot
Copy link

k8s-bot commented Jun 8, 2015

EXPERIMENTAL JENKINS PR BUILDER: e2e build succeeded.

@xiang90
Copy link
Contributor

xiang90 commented Jun 8, 2015

@fgrzadkowski Any data about the latency before the change?

@fgrzadkowski
Copy link
Contributor Author

I'm running it now.

@fgrzadkowski
Copy link
Contributor Author

Results from one run without this change (current HEAD):

INFO: Top latency metric: {Verb:LIST Resource:pods Quantile:0.99 Latency:1.307695s}
INFO: Top latency metric: {Verb:LIST Resource:pods Quantile:0.9 Latency:1.089075s}
INFO: Top latency metric: {Verb:LIST Resource:pods Quantile:0.5 Latency:576.638ms}
INFO: Top latency metric: {Verb:POST Resource:pods Quantile:0.99 Latency:427.68ms}
INFO: Top latency metric: {Verb:POST Resource:bindings Quantile:0.99 Latency:395.389ms}

Which gives around 25% improvement.

Both results are after a single run, so in reality difference might be slightly different.

@xiang90
Copy link
Contributor

xiang90 commented Jun 8, 2015

@fgrzadkowski Got it. Thank you!

@lavalamp
Copy link
Member

lavalamp commented Jun 8, 2015

LGTM

@lavalamp lavalamp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2015
krousey added a commit that referenced this pull request Jun 8, 2015
@krousey
Merge pull request #9390 from fgrzadkowski/bump_ectd_client
ab1bc9f 
Update go-etcd client library version and add ugorji codec to dependencies
@krousey krousey merged commit ab1bc9f into kubernetes:master Jun 8, 2015
@ghost
Copy link

ghost commented Jun 9, 2015

This PR seems to have introduced a large number of e2e test failures.
Most of them manifest as:

"cannot allocate resources of type serviceipallocation at this time"

from:

https://github.com/GoogleCloudPlatform/kubernetes/blob/51750a4680cbb161b068b7748c8d00cacb8a893a/pkg/registry/service/allocator/etcd/etcd.go#L147

We're considering rolling this PR back as a result.

@krousey krousey mentioned this pull request Jun 9, 2015
Closed
@ghost
Copy link

ghost commented Jun 9, 2015

Any major objections @fgrzadkowski ?

@ghost
Copy link

ghost commented Jun 9, 2015

@lavalamp @davidopp FYI

@krousey
Copy link
Contributor

krousey commented Jun 9, 2015

Sorry @fgrzadkowski, we're going to roll this back and hope it improves the stability.

@krousey krousey mentioned this pull request Jun 9, 2015
Merged
@fgrzadkowski
Copy link
Contributor Author

Thanks, I'll look into this.

@justinsb justinsb mentioned this pull request Jun 15, 2015
Closed
@mnemotiv mnemotiv mentioned this pull request Jun 16, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@fgrzadkowski @k8s-bot @xiang90 @lavalamp @krousey @googlebot