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

etcdserver/stats: fix stats data race. #9562

Merged
merged 2 commits into from
Apr 12, 2018
Merged

etcdserver/stats: fix stats data race. #9562

merged 2 commits into from
Apr 12, 2018

Conversation

disksing
Copy link
Contributor

Hi, the data race is recently reported from our CI. We are using etcd v3.2.14 as embed library.

WARNING: DATA RACE
Write at 0x00c425a78640 by goroutine 13:
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats.(*statsQueue).Insert()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats/queue.go:72 +0x3e3
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats.(*ServerStats).SendAppendReq()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats/server.go:120 +0x153
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/rafthttp.(*Transport).Send()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/rafthttp/transport.go:174 +0x343
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver.(*raftNode).start.func1()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/raft.go:214 +0xf4b
Previous read at 0x00c425a78640 by goroutine 99:
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats.(*statsQueue).Rate()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats/queue.go:43 +0x184
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats.(*ServerStats).JSON()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/stats/server.go:79 +0x122
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver.(*EtcdServer).SelfStats()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/server.go:1015 +0x58
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http.(*statsHandler).serveSelf()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http/client.go:301 +0x13b
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http.(*statsHandler).(github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http.serveSelf)-fm()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http/client.go:95 +0x5f
  net/http.HandlerFunc.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:1918 +0x51
  net/http.(*ServeMux).ServeHTTP()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:2254 +0xa2
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http.requestLogger.func1()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/api/v2http/http.go:72 +0x20f
  net/http.HandlerFunc.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:1918 +0x51
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/pkg/cors.(*CORSHandler).ServeHTTP()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/pkg/cors/cors.go:89 +0x16d
  net/http.(*ServeMux).ServeHTTP()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:2254 +0xa2
  net/http.serverHandler.ServeHTTP()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:2619 +0xbc
  net/http.(*conn).serve()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:1801 +0x83b
Goroutine 13 (running) created at:
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver.(*raftNode).start()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/raft.go:157 +0x5f
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver.(*EtcdServer).run()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/etcdserver/server.go:678 +0x58b
Goroutine 99 (finished) created at:
  net/http.(*Server).Serve()
      /home/travis/.gimme/versions/go1.9.4.linux.amd64/src/net/http/server.go:2720 +0x37c
  github.com/pingcap/pd/vendor/github.com/coreos/etcd/embed.(*serveCtx).serve.func2()
      /home/travis/gopath/src/github.com/pingcap/pd/vendor/github.com/coreos/etcd/embed/serve.go:116 +0x4c

@codecov-io
Copy link

Codecov Report

Merging #9562 into master will increase coverage by 0.1%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #9562     +/-   ##
=========================================
+ Coverage   72.23%   72.33%   +0.1%     
=========================================
  Files         368      368             
  Lines       31341    31341             
=========================================
+ Hits        22638    22670     +32     
+ Misses       7074     7031     -43     
- Partials     1629     1640     +11
Impacted Files Coverage Δ
etcdserver/stats/server.go 81.03% <0%> (ø) ⬆️
etcdctl/ctlv3/command/get_command.go 46.59% <0%> (-30.69%) ⬇️
compactor/periodic.go 63.63% <0%> (-25.76%) ⬇️
proxy/grpcproxy/watcher.go 85.71% <0%> (-8.17%) ⬇️
etcdctl/ctlv3/command/lease_command.go 65.34% <0%> (-5.95%) ⬇️
pkg/transport/listener.go 60.54% <0%> (-3.25%) ⬇️
proxy/grpcproxy/register.go 80.55% <0%> (-2.78%) ⬇️
etcdserver/v3_server.go 78.37% <0%> (-2.55%) ⬇️
auth/simple_token.go 93.51% <0%> (-1.86%) ⬇️
pkg/adt/interval_tree.go 88.58% <0%> (-1.81%) ⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f46368c...ba59bb2. Read the comment docs.

stats.SendingPkgRate, stats.SendingBandwidthRate = stats.sendRateQueue.Rate()
stats.RecvingPkgRate, stats.RecvingBandwidthRate = stats.recvRateQueue.Rate()
ss.Unlock()
stats.LeaderInfo.Uptime = time.Since(stats.LeaderInfo.StartTime).String()
Copy link
Contributor

Choose a reason for hiding this comment

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

switch line 79 and 80?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gyuho Fixed.

@gyuho
Copy link
Contributor

gyuho commented Apr 12, 2018

lgtm thanks!

@gyuho gyuho merged commit 836fffa into etcd-io:master Apr 12, 2018
@disksing disksing deleted the data-race branch April 12, 2018 05:40
@gyuho gyuho mentioned this pull request Apr 19, 2018
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants