Browse Source

etcdserver: fix data race in retrieving self stats

Jonathan Boulle 11 years ago
parent
commit
c30b82b596
3 changed files with 27 additions and 21 deletions
  1. 3 9
      etcdserver/etcdhttp/http.go
  2. 5 4
      etcdserver/etcdhttp/http_test.go
  3. 19 8
      etcdserver/server.go

+ 3 - 9
etcdserver/etcdhttp/http.go

@@ -42,6 +42,7 @@ func NewClientHandler(server *etcdserver.EtcdServer) http.Handler {
 		server:       server,
 		clusterStore: server.ClusterStore,
 		stats:        server,
+		storestats:   server,
 		timer:        server,
 		timeout:      defaultServerTimeout,
 	}
@@ -175,22 +176,15 @@ func (h serverHandler) serveStoreStats(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(h.storestats.JSON())
+	w.Write(h.storestats.StoreStatsJSON())
 }
 
 func (h serverHandler) serveSelfStats(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
 	}
-	s := h.stats.SelfStats()
-	b, err := json.Marshal(s)
-	if err != nil {
-		log.Printf("error marshalling stats: %v\n", err)
-		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
-		return
-	}
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(b)
+	w.Write(h.stats.SelfStatsJSON())
 }
 
 func (h serverHandler) serveLeaderStats(w http.ResponseWriter, r *http.Request) {

+ 5 - 4
etcdserver/etcdhttp/http_test.go

@@ -639,11 +639,12 @@ func TestServeMachines(t *testing.T) {
 }
 
 type dummyServerStats struct {
-	ss *stats.ServerStats
+	js []byte
 	ls *stats.LeaderStats
 }
 
-func (dss *dummyServerStats) SelfStats() *stats.ServerStats   { return dss.ss }
+func (dss *dummyServerStats) SelfStatsJSON() []byte           { return dss.js }
+func (dss *dummyServerStats) SelfStats() *stats.ServerStats   { return nil }
 func (dss *dummyServerStats) LeaderStats() *stats.LeaderStats { return dss.ls }
 
 func TestServeSelfStats(t *testing.T) {
@@ -657,7 +658,7 @@ func TestServeSelfStats(t *testing.T) {
 	}
 	sh := &serverHandler{
 		stats: &dummyServerStats{
-			ss: ss,
+			js: w,
 		},
 	}
 	rw := httptest.NewRecorder()
@@ -737,7 +738,7 @@ type dummyStoreStats struct {
 	data []byte
 }
 
-func (dss *dummyStoreStats) JSON() []byte { return dss.data }
+func (dss *dummyStoreStats) StoreStatsJSON() []byte { return dss.data }
 
 func TestServeStoreStats(t *testing.T) {
 	w := "foobarbaz"

+ 19 - 8
etcdserver/server.go

@@ -91,17 +91,18 @@ type Server interface {
 }
 
 type ServerStats interface {
-	// SelfStats returns the statistics of this server
+	// SelfStats returns the struct representing statistics of this server
 	SelfStats() *stats.ServerStats
+	// SelfStats returns the statistics of this server in JSON form
+	SelfStatsJSON() []byte
 	// LeaderStats returns the statistics of all followers in the cluster
 	// if this server is leader. Otherwise, nil is returned.
 	LeaderStats() *stats.LeaderStats
 }
 
 type StoreStats interface {
-	// JSON returns statistics of the underlying Store used by the
-	// EtcdServer, in JSON format
-	JSON() []byte
+	// StoreStatsJSON returns statistics of the store in JSON format
+	StoreStatsJSON() []byte
 }
 
 type RaftTimer interface {
@@ -364,18 +365,28 @@ func (s *EtcdServer) Do(ctx context.Context, r pb.Request) (Response, error) {
 }
 
 func (s *EtcdServer) SelfStats() *stats.ServerStats {
-	s.stats.LeaderInfo.Uptime = time.Now().Sub(s.stats.LeaderInfo.StartTime).String()
-	s.stats.SendingPkgRate, s.stats.SendingBandwidthRate = s.stats.SendRates()
-	s.stats.RecvingPkgRate, s.stats.RecvingBandwidthRate = s.stats.RecvRates()
 	return s.stats
 }
 
+func (s *EtcdServer) SelfStatsJSON() []byte {
+	stats := *s.stats
+	stats.LeaderInfo.Uptime = time.Now().Sub(stats.LeaderInfo.StartTime).String()
+	stats.SendingPkgRate, stats.SendingBandwidthRate = stats.SendRates()
+	stats.RecvingPkgRate, stats.RecvingBandwidthRate = stats.RecvRates()
+	b, err := json.Marshal(s.stats)
+	// TODO(jonboulle): appropriate error handling?
+	if err != nil {
+		log.Printf("error marshalling self stats: %v", err)
+	}
+	return b
+}
+
 func (s *EtcdServer) LeaderStats() *stats.LeaderStats {
 	// TODO(jonboulle): need to lock access to lstats, set it to nil when not leader, ...
 	return s.lstats
 }
 
-func (s *EtcdServer) StoreStats() []byte {
+func (s *EtcdServer) StoreStatsJSON() []byte {
 	return s.store.JsonStats()
 }