silences: fix concurrent cache writes (#561)

This fixes #559 by removing concurrent map writes to the matcher cache.
The cache was guarded by the Silence's main lock, which only used a
read-lock on queries.
The cache's get methods lazily loads data into the cache and thus
causing concurrent writes.

We just change the main lock to always write-lock, as we don't expect
high lock contention at this point and would have it in a dedicated
cache lock anyway.
This commit is contained in:
Fabian Reinartz 2016-11-21 11:09:49 +01:00 committed by GitHub
parent 406aacf141
commit b6851a5421
4 changed files with 54 additions and 51 deletions

View File

@ -32,7 +32,7 @@ var (
NotifierConfig: NotifierConfig{
VSendResolved: false,
},
HTML: `{{ template "email.default.html" . }}`,
HTML: `{{ template "email.default.html" . }}`,
}
// DefaultEmailSubject defines the default Subject header of an Email.

View File

@ -96,16 +96,16 @@ type Silences struct {
now func() time.Time
retention time.Duration
mc matcherCache
gossip mesh.Gossip // gossip channel for sharing silences
// We store silences in a map of IDs for now. Currently, the memory
// state is equivalent to the mesh.GossipData representation.
// In the future we'll want support for efficient queries by time
// range and affected labels.
mtx sync.RWMutex
// Mutex also guards the matcherCache, which always need write lock access.
mtx sync.Mutex
st gossipData
mc matcherCache
}
type metrics struct {
@ -632,7 +632,9 @@ func (s *Silences) query(q *query, now *timestamp.Timestamp) ([]*pb.Silence, err
// the trivial solution for now.
var res []*pb.Silence
s.mtx.RLock()
s.mtx.Lock()
defer s.mtx.Unlock()
if q.ids != nil {
for _, id := range q.ids {
if s, ok := s.st[string(id)]; ok {
@ -662,7 +664,6 @@ func (s *Silences) query(q *query, now *timestamp.Timestamp) ([]*pb.Silence, err
resf = append(resf, sil)
}
}
s.mtx.RUnlock()
return resf, nil
}
@ -672,6 +673,9 @@ func (s *Silences) query(q *query, now *timestamp.Timestamp) ([]*pb.Silence, err
func (s *Silences) loadSnapshot(r io.Reader) error {
st := gossipData{}
s.mtx.Lock()
defer s.mtx.Unlock()
for {
var sil pb.MeshSilence
if _, err := pbutil.ReadDelimited(r, &sil); err != nil {
@ -686,9 +690,8 @@ func (s *Silences) loadSnapshot(r io.Reader) error {
return err
}
}
s.mtx.Lock()
s.st = st
s.mtx.Unlock()
return nil
}
@ -699,8 +702,8 @@ func (s *Silences) Snapshot(w io.Writer) (int, error) {
start := time.Now()
defer func() { s.metrics.snapshotDuration.Observe(time.Since(start).Seconds()) }()
s.mtx.RLock()
defer s.mtx.RUnlock()
s.mtx.Lock()
defer s.mtx.Unlock()
var n int
for _, s := range s.st {
@ -719,8 +722,8 @@ type gossiper struct {
// Gossip implements the mesh.Gossiper interface.
func (g gossiper) Gossip() mesh.GossipData {
g.mtx.RLock()
defer g.mtx.RUnlock()
g.mtx.Lock()
defer g.mtx.Unlock()
return g.st.clone()
}

View File

@ -182,6 +182,7 @@ type bintree struct {
Func func() (*asset, error)
Children map[string]*bintree
}
var _bintree = &bintree{nil, map[string]*bintree{
"template": &bintree{nil, map[string]*bintree{
"default.tmpl": &bintree{templateDefaultTmpl, map[string]*bintree{}},
@ -234,4 +235,3 @@ func _filePath(dir, name string) string {
cannonicalName := strings.Replace(name, "\\", "/", -1)
return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...)
}

View File

@ -602,29 +602,29 @@ func AssetNames() []string {
// _bindata is a table, holding each asset generator, mapped to its name.
var _bindata = map[string]func() (*asset, error){
"ui/app/css/main.css": uiAppCssMainCss,
"ui/app/index.html": uiAppIndexHtml,
"ui/app/js/app.js": uiAppJsAppJs,
"ui/app/partials/alert.html": uiAppPartialsAlertHtml,
"ui/app/partials/alerts.html": uiAppPartialsAlertsHtml,
"ui/app/partials/route.html": uiAppPartialsRouteHtml,
"ui/app/css/main.css": uiAppCssMainCss,
"ui/app/index.html": uiAppIndexHtml,
"ui/app/js/app.js": uiAppJsAppJs,
"ui/app/partials/alert.html": uiAppPartialsAlertHtml,
"ui/app/partials/alerts.html": uiAppPartialsAlertsHtml,
"ui/app/partials/route.html": uiAppPartialsRouteHtml,
"ui/app/partials/silence-form.html": uiAppPartialsSilenceFormHtml,
"ui/app/partials/silence.html": uiAppPartialsSilenceHtml,
"ui/app/partials/silences.html": uiAppPartialsSilencesHtml,
"ui/app/partials/status.html": uiAppPartialsStatusHtml,
"ui/bindata.go": uiBindataGo,
"ui/lib/angular-moment.min.js": uiLibAngularMomentMinJs,
"ui/lib/angular-resource.min.js": uiLibAngularResourceMinJs,
"ui/lib/angular-route.min.js": uiLibAngularRouteMinJs,
"ui/lib/angular-sanitize.min.js": uiLibAngularSanitizeMinJs,
"ui/lib/angular.min.js": uiLibAngularMinJs,
"ui/lib/d3.v3.min.js": uiLibD3V3MinJs,
"ui/lib/jquery.min.js": uiLibJqueryMinJs,
"ui/lib/js-yaml.min.js": uiLibJsYamlMinJs,
"ui/lib/kube.min.css": uiLibKubeMinCss,
"ui/lib/moment.min.js": uiLibMomentMinJs,
"ui/lib/routing-tree.js": uiLibRoutingTreeJs,
"ui/web.go": uiWebGo,
"ui/app/partials/silence.html": uiAppPartialsSilenceHtml,
"ui/app/partials/silences.html": uiAppPartialsSilencesHtml,
"ui/app/partials/status.html": uiAppPartialsStatusHtml,
"ui/bindata.go": uiBindataGo,
"ui/lib/angular-moment.min.js": uiLibAngularMomentMinJs,
"ui/lib/angular-resource.min.js": uiLibAngularResourceMinJs,
"ui/lib/angular-route.min.js": uiLibAngularRouteMinJs,
"ui/lib/angular-sanitize.min.js": uiLibAngularSanitizeMinJs,
"ui/lib/angular.min.js": uiLibAngularMinJs,
"ui/lib/d3.v3.min.js": uiLibD3V3MinJs,
"ui/lib/jquery.min.js": uiLibJqueryMinJs,
"ui/lib/js-yaml.min.js": uiLibJsYamlMinJs,
"ui/lib/kube.min.css": uiLibKubeMinCss,
"ui/lib/moment.min.js": uiLibMomentMinJs,
"ui/lib/routing-tree.js": uiLibRoutingTreeJs,
"ui/web.go": uiWebGo,
}
// AssetDir returns the file names below a certain
@ -666,6 +666,7 @@ type bintree struct {
Func func() (*asset, error)
Children map[string]*bintree
}
var _bintree = &bintree{nil, map[string]*bintree{
"ui": &bintree{nil, map[string]*bintree{
"app": &bintree{nil, map[string]*bintree{
@ -677,28 +678,28 @@ var _bintree = &bintree{nil, map[string]*bintree{
"app.js": &bintree{uiAppJsAppJs, map[string]*bintree{}},
}},
"partials": &bintree{nil, map[string]*bintree{
"alert.html": &bintree{uiAppPartialsAlertHtml, map[string]*bintree{}},
"alerts.html": &bintree{uiAppPartialsAlertsHtml, map[string]*bintree{}},
"route.html": &bintree{uiAppPartialsRouteHtml, map[string]*bintree{}},
"alert.html": &bintree{uiAppPartialsAlertHtml, map[string]*bintree{}},
"alerts.html": &bintree{uiAppPartialsAlertsHtml, map[string]*bintree{}},
"route.html": &bintree{uiAppPartialsRouteHtml, map[string]*bintree{}},
"silence-form.html": &bintree{uiAppPartialsSilenceFormHtml, map[string]*bintree{}},
"silence.html": &bintree{uiAppPartialsSilenceHtml, map[string]*bintree{}},
"silences.html": &bintree{uiAppPartialsSilencesHtml, map[string]*bintree{}},
"status.html": &bintree{uiAppPartialsStatusHtml, map[string]*bintree{}},
"silence.html": &bintree{uiAppPartialsSilenceHtml, map[string]*bintree{}},
"silences.html": &bintree{uiAppPartialsSilencesHtml, map[string]*bintree{}},
"status.html": &bintree{uiAppPartialsStatusHtml, map[string]*bintree{}},
}},
}},
"bindata.go": &bintree{uiBindataGo, map[string]*bintree{}},
"lib": &bintree{nil, map[string]*bintree{
"angular-moment.min.js": &bintree{uiLibAngularMomentMinJs, map[string]*bintree{}},
"angular-moment.min.js": &bintree{uiLibAngularMomentMinJs, map[string]*bintree{}},
"angular-resource.min.js": &bintree{uiLibAngularResourceMinJs, map[string]*bintree{}},
"angular-route.min.js": &bintree{uiLibAngularRouteMinJs, map[string]*bintree{}},
"angular-route.min.js": &bintree{uiLibAngularRouteMinJs, map[string]*bintree{}},
"angular-sanitize.min.js": &bintree{uiLibAngularSanitizeMinJs, map[string]*bintree{}},
"angular.min.js": &bintree{uiLibAngularMinJs, map[string]*bintree{}},
"d3.v3.min.js": &bintree{uiLibD3V3MinJs, map[string]*bintree{}},
"jquery.min.js": &bintree{uiLibJqueryMinJs, map[string]*bintree{}},
"js-yaml.min.js": &bintree{uiLibJsYamlMinJs, map[string]*bintree{}},
"kube.min.css": &bintree{uiLibKubeMinCss, map[string]*bintree{}},
"moment.min.js": &bintree{uiLibMomentMinJs, map[string]*bintree{}},
"routing-tree.js": &bintree{uiLibRoutingTreeJs, map[string]*bintree{}},
"angular.min.js": &bintree{uiLibAngularMinJs, map[string]*bintree{}},
"d3.v3.min.js": &bintree{uiLibD3V3MinJs, map[string]*bintree{}},
"jquery.min.js": &bintree{uiLibJqueryMinJs, map[string]*bintree{}},
"js-yaml.min.js": &bintree{uiLibJsYamlMinJs, map[string]*bintree{}},
"kube.min.css": &bintree{uiLibKubeMinCss, map[string]*bintree{}},
"moment.min.js": &bintree{uiLibMomentMinJs, map[string]*bintree{}},
"routing-tree.js": &bintree{uiLibRoutingTreeJs, map[string]*bintree{}},
}},
"web.go": &bintree{uiWebGo, map[string]*bintree{}},
}},
@ -750,4 +751,3 @@ func _filePath(dir, name string) string {
cannonicalName := strings.Replace(name, "\\", "/", -1)
return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...)
}