From b886a14cfc46052081a2d119e8dad488167595de Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Mon, 6 May 2013 09:53:53 +0200 Subject: [PATCH 1/5] Remove race conditions from UncertaintyGroup --- utility/uncertaintygroup.go | 46 +++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/utility/uncertaintygroup.go b/utility/uncertaintygroup.go index b1c909744..2c00833a0 100644 --- a/utility/uncertaintygroup.go +++ b/utility/uncertaintygroup.go @@ -15,6 +15,7 @@ package utility import ( "fmt" + "sync" ) type state int @@ -62,18 +63,19 @@ type uncertaintyGroup struct { successes uint results chan error anomalies []error + sync.Mutex } -func (g uncertaintyGroup) Succeed() { - if g.state == finished { +func (g *uncertaintyGroup) Succeed() { + if g.isFinished() { panic("cannot remark when done") } g.results <- nil } -func (g uncertaintyGroup) Fail(err error) { - if g.state == finished { +func (g *uncertaintyGroup) Fail(err error) { + if g.isFinished() { panic("cannot remark when done") } @@ -84,22 +86,42 @@ func (g uncertaintyGroup) Fail(err error) { g.results <- err } -func (g uncertaintyGroup) MayFail(err error) { - if g.state == finished { +func (g *uncertaintyGroup) MayFail(err error) { + if g.isFinished() { panic("cannot remark when done") } g.results <- err } -func (g *uncertaintyGroup) Wait() bool { +func (g *uncertaintyGroup) isFinished() bool { + g.Lock() + defer g.Unlock() + + return g.state == finished +} + +func (g *uncertaintyGroup) finish() { + g.Lock() + defer g.Unlock() + + g.state = finished +} + +func (g *uncertaintyGroup) start() { + g.Lock() + defer g.Unlock() + if g.state != unstarted { panic("cannot restart") } - defer close(g.results) - g.state = started +} + +func (g *uncertaintyGroup) Wait() bool { + defer close(g.results) + g.start() for g.remaining > 0 { result := <-g.results @@ -113,12 +135,12 @@ func (g *uncertaintyGroup) Wait() bool { g.remaining-- } - g.state = finished + g.finish() return len(g.anomalies) == 0 } -func (g uncertaintyGroup) Errors() []error { +func (g *uncertaintyGroup) Errors() []error { if g.state != finished { panic("cannot provide errors until finished") } @@ -126,7 +148,7 @@ func (g uncertaintyGroup) Errors() []error { return g.anomalies } -func (g uncertaintyGroup) String() string { +func (g *uncertaintyGroup) String() string { return fmt.Sprintf("UncertaintyGroup %s with %s failures", g.state, g.anomalies) } From 9183302b1fcf89d1032b3b2c06c4a3295c78d718 Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Mon, 6 May 2013 09:56:32 +0200 Subject: [PATCH 2/5] Web handler returns 404 for favicon requests --- web/web.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/web.go b/web/web.go index aa5c19ec7..863b671a5 100644 --- a/web/web.go +++ b/web/web.go @@ -41,6 +41,10 @@ type WebService struct { func (w WebService) ServeForever() error { gorest.RegisterService(w.MetricsHandler) + exp.Handle("/favicon.ico", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "", 404) + })) + // TODO(julius): This will need to be rewritten once the exp package provides // the coarse mux behaviors via a wrapper function. exp.Handle("/debug/pprof/", http.HandlerFunc(pprof.Index)) From 77401676540b6858a07e3684f5902599420b7610 Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Mon, 6 May 2013 09:59:47 +0200 Subject: [PATCH 3/5] Add comments about potential race conditions --- web/status.go | 1 + 1 file changed, 1 insertion(+) diff --git a/web/status.go b/web/status.go index 7b5a5f9f4..5e70227d0 100644 --- a/web/status.go +++ b/web/status.go @@ -52,6 +52,7 @@ func (h *StatusHandler) ServeRequestsForever() { Config: h.Config.String(), Flags: flags, Rules: "TODO: list rules here", + // BUG: race condition, concurrent map access TargetPools: h.TargetManager.Pools(), } From cdde766f39f204f05564275be10f275bb8c833f4 Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Mon, 6 May 2013 10:39:13 +0200 Subject: [PATCH 4/5] Embed mutex on web status handler --- web/status.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/status.go b/web/status.go index 5e70227d0..fc2501c9f 100644 --- a/web/status.go +++ b/web/status.go @@ -48,10 +48,10 @@ func (h *StatusHandler) ServeRequestsForever() { }) h.PrometheusStatus = &PrometheusStatus{ - BuildInfo: h.BuildInfo, - Config: h.Config.String(), - Flags: flags, - Rules: "TODO: list rules here", + BuildInfo: h.BuildInfo, + Config: h.Config.String(), + Flags: flags, + Rules: "TODO: list rules here", // BUG: race condition, concurrent map access TargetPools: h.TargetManager.Pools(), } From a07b89d41176fcfec18d78fc1fce7cd4fc5bbc05 Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Mon, 6 May 2013 10:54:27 +0200 Subject: [PATCH 5/5] Document go-1.1 race detector with prometheus --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index 726315c90..1bcafbbb7 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,22 @@ architecture and release identification remarks for us. $ make test +### Race Detector + +Go 1.1 includes a [race +detector](http://tip.golang.org/doc/articles/race_detector.html) which can be +enabled at build time. Here's how to use it with prometheus (assumes that +you've already run a successful build). + +To run the tests with race detection: + + $ GORACE="log_path=/tmp/foo" go test -race ./... + +To run the server with race detection: + + $ go build -race . + $ GORACE="log_path=/tmp/foo" ./prometheus + [![Build Status](https://travis-ci.org/prometheus/prometheus.png)](https://travis-ci.org/prometheus/prometheus) ## Contributing