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

Update Raft Vendoring #4539

Merged
merged 1 commit into from
Sep 6, 2018
Merged

Update Raft Vendoring #4539

merged 1 commit into from
Sep 6, 2018

Conversation

mkeeler
Copy link
Member

@mkeeler mkeeler commented Aug 17, 2018

Pulls in a fix for a potential memory leak regarding consistent reads that invoke VerifyLeader.

This is the comparison for our current version to the update (useful if you want the commit message context)
hashicorp/raft@6d14f0c...master

Pulls in a fix for a potential memory leak regarding consistent reads that invoke VerifyLeader.
@mkeeler mkeeler requested review from banks and kyhavlov August 17, 2018 18:28
@mkeeler
Copy link
Member Author

mkeeler commented Aug 17, 2018

Before these changes and after letting a cluster run for a while with constant kv write + consistent read:

(pprof) top5
Showing nodes accounting for 10920.40kB, 95.52% of 11432.45kB total
Showing top 5 nodes out of 51
      flat  flat%   sum%        cum   cum%
 5120.43kB 44.79% 44.79%  5120.43kB 44.79%  github.com/hashicorp/consul/vendor/github.com/hashicorp/raft.(*Raft).VerifyLeader /go/src/github.com/hashicorp/consul/vendor/github.com/hashicorp/raft/future.go
 4096.25kB 35.83% 80.62%  4096.25kB 35.83%  github.com/hashicorp/consul/vendor/github.com/hashicorp/raft.(*Raft).VerifyLeader /go/src/github.com/hashicorp/consul/vendor/github.com/hashicorp/raft/api.go
  678.95kB  5.94% 86.56%   678.95kB  5.94%  github.com/hashicorp/consul/vendor/github.com/hashicorp/raft.(*Raft).verifyLeader /go/src/github.com/hashicorp/consul/vendor/github.com/hashicorp/raft/raft.go
  512.69kB  4.48% 91.04%   512.69kB  4.48%  crypto/x509.parseCertificate /usr/local/go/src/crypto/x509/x509.go
  512.08kB  4.48% 95.52%   512.08kB  4.48%  net/http.(*Server).Serve /usr/local/go/src/net/http/server.go

After the changes:

(pprof) top5
Showing nodes accounting for 2703.62kB, 56.89% of 4752.21kB total
Showing top 5 nodes out of 67
      flat  flat%   sum%        cum   cum%
  591.75kB 12.45% 12.45%   591.75kB 12.45%  crypto/elliptic.initTable /usr/local/go/src/crypto/elliptic/p256_amd64.go
  544.67kB 11.46% 23.91%   544.67kB 11.46%  github.com/hashicorp/consul/vendor/github.com/hashicorp/memberlist.(*NetTransport).udpListen /go/src/github.com/hashicorp/consul/vendor/github.com/hashicorp/memberlist/net_transport.go
  540.51kB 11.37% 35.29%   540.51kB 11.37%  github.com/hashicorp/consul/agent.New /go/src/github.com/hashicorp/consul/agent/agent.go
     514kB 10.82% 46.10%      514kB 10.82%  github.com/hashicorp/consul/vendor/github.com/hashicorp/serf/serf.Create /go/src/github.com/hashicorp/consul/vendor/github.com/hashicorp/serf/serf/serf.go
  512.69kB 10.79% 56.89%  1024.97kB 21.57%  crypto/x509.parseCertificate /usr/local/go/src/crypto/x509/x509.go

One other difference is that during the first run I was executing against the HTTP API on the leader server whereas in the second run I happened to be using an other client. That accounts for why the crypto/x509.parseCertificate and net/http Serve are not in the second pprof output.

Copy link
Member

@banks banks left a comment

Choose a reason for hiding this comment

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

Cool!

@mkeeler mkeeler merged commit d1e52e5 into master Sep 6, 2018
@mkeeler mkeeler deleted the vendor-raft branch September 6, 2018 19:09
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.

2 participants