From 1a0cecb10ae4541e902be8971e604c723c03b0d7 Mon Sep 17 00:00:00 2001 From: geauxvirtual Date: Tue, 21 Jun 2016 14:12:23 -0700 Subject: [PATCH] Fix #1011: Group dynamic metrics in cache under appropriate keys --- control/strategy/cache.go | 34 +++---- control/strategy/cache_small_test.go | 129 +++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 15 deletions(-) create mode 100644 control/strategy/cache_small_test.go diff --git a/control/strategy/cache.go b/control/strategy/cache.go index 421d3b0c8..917ce4948 100644 --- a/control/strategy/cache.go +++ b/control/strategy/cache.go @@ -153,27 +153,31 @@ type listMetricInfo struct { } func (c *cache) updateCache(mts []core.Metric) { - dc := map[string]listMetricInfo{} + dc := map[string]*listMetricInfo{} for _, mt := range mts { - isDynamic, _ := mt.Namespace().IsDynamic() - if isDynamic == false { - // cache the individual metric - c.put(mt.Namespace().String(), mt.Version(), mt) - } else { - // collect the dynamic query results so we can cache - key := fmt.Sprintf("%v:%v", mt.Namespace().String(), mt.Version()) + isDynamic, idx := mt.Namespace().IsDynamic() + if isDynamic { + // cache dynamic metrics + dynNS := make(core.Namespace, len(mt.Namespace())) + copy(dynNS, mt.Namespace()) + for _, v := range idx { + dynNS[v].Value = "*" + } + key := fmt.Sprintf("%v:%v", dynNS.String(), mt.Version()) if _, ok := dc[key]; !ok { - dc[key] = listMetricInfo{ - metrics: []core.Metric{}, + dc[key] = &listMetricInfo{ + metrics: []core.Metric{}, + namespace: dynNS.String(), + version: mt.Version(), } } - var tmp = dc[key] - tmp.metrics = append(dc[key].metrics, mt) - tmp.namespace = mt.Namespace().String() - tmp.version = mt.Version() - dc[key] = tmp + dc[key].metrics = append(dc[key].metrics, mt) + continue } + // cache the individual metric + c.put(mt.Namespace().String(), mt.Version(), mt) } + // write our dynamic metrics to the cache. for _, v := range dc { c.put(v.namespace, v.version, v.metrics) } diff --git a/control/strategy/cache_small_test.go b/control/strategy/cache_small_test.go new file mode 100644 index 000000000..ad446b6de --- /dev/null +++ b/control/strategy/cache_small_test.go @@ -0,0 +1,129 @@ +// +build small + +/* +http://www.apache.org/licenses/LICENSE-2.0.txt + + +Copyright 2015 Intel Corporation + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package strategy + +import ( + "testing" + "time" + + "github.com/intelsdi-x/snap/control/fixtures" + "github.com/intelsdi-x/snap/core" + . "github.com/smartystreets/goconvey/convey" +) + +func TestNewCache(t *testing.T) { + c := NewCache(1 * time.Second) + Convey("Calling NewCache(1 * time.Second)", t, func() { + Convey("Should return a cache", func() { + So(c, ShouldNotBeNil) + }) + Convey("Cache should have a TTL of 1s", func() { + So(c.ttl, ShouldResemble, time.Duration(1*time.Second)) + }) + }) +} + +// TestUpdateCache places static and dynamic entries into the cache and verify those entries +// are inserted into the cache correctly. +func TestUpdateCache(t *testing.T) { + scache := NewCache(300 * time.Millisecond) + // Static metrics + staticMetrics := []core.Metric{ + fixtures.MockMetricType{ + Namespace_: core.NewNamespace("foo", "bar"), + Ver: 0, + }, + fixtures.MockMetricType{ + Namespace_: core.NewNamespace("foo", "baz"), + Ver: 0, + }, + } + scache.updateCache(staticMetrics) + Convey("Updating cache with two static metrics", t, func() { + Convey("Should result in a cache with two entries", func() { + So(len(scache.table), ShouldEqual, 2) + }) + Convey("Should have an entry for '/foo/bar:0'", func() { + _, ok := scache.table["/foo/bar:0"] + So(ok, ShouldBeTrue) + }) + Convey("Should have an entry for '/foo/baz:0'", func() { + _, ok := scache.table["/foo/baz:0"] + So(ok, ShouldBeTrue) + }) + }) + dcache := NewCache(300 * time.Millisecond) + // Dynamic Metrics + dynNS1 := core.NewNamespace("foo", "bar").AddDynamicElement("host", "Mock host").AddStaticElement("qux") + dynNS2 := core.NewNamespace("foo", "baz").AddDynamicElement("host", "Mock host").AddStaticElement("avg") + dynNS3 := core.NewNamespace("foo", "bar").AddDynamicElement("host", "Mock host").AddDynamicElement("rack", "Mock rack").AddStaticElement("temp") + mockNS1 := make([]core.NamespaceElement, len(dynNS1)) + mockNS2 := make([]core.NamespaceElement, len(dynNS1)) + mockNS3 := make([]core.NamespaceElement, len(dynNS2)) + mockNS4 := make([]core.NamespaceElement, len(dynNS3)) + copy(mockNS1, dynNS1) + copy(mockNS2, dynNS1) + copy(mockNS3, dynNS2) + copy(mockNS4, dynNS3) + mockNS1[2].Value = "hosta" + mockNS2[2].Value = "hostb" + mockNS3[2].Value = "hosta" + mockNS4[2].Value = "hostc" + mockNS4[3].Value = "rack1" + + dynamicMetrics := []core.Metric{ + fixtures.MockMetricType{ + Namespace_: mockNS1, + Ver: 0, + }, + fixtures.MockMetricType{ + Namespace_: mockNS2, + Ver: 0, + }, + fixtures.MockMetricType{ + Namespace_: mockNS3, + Ver: 0, + }, + fixtures.MockMetricType{ + Namespace_: mockNS4, + Ver: 0, + }, + } + dcache.updateCache(dynamicMetrics) + Convey("Updating cache with four metrics on three dynamic namespaces", t, func() { + Convey("Should result in a cache with two entries", func() { + So(len(dcache.table), ShouldEqual, 3) + }) + Convey("Should have an entry for '/foo/bar/*/qux:0'", func() { + _, ok := dcache.table["/foo/bar/*/qux:0"] + So(ok, ShouldBeTrue) + }) + Convey("Should have an entry for '/foo/baz/*/avg:0'", func() { + _, ok := dcache.table["/foo/baz/*/avg:0"] + So(ok, ShouldBeTrue) + }) + Convey("Should have an entry for '/foo/bar/*/*/temp:0'", func() { + _, ok := dcache.table["/foo/bar/*/*/temp:0"] + So(ok, ShouldBeTrue) + }) + }) +}