diff --git a/storage/memory/memory.go b/storage/memory/memory.go index dd353cc4..63c6d552 100644 --- a/storage/memory/memory.go +++ b/storage/memory/memory.go @@ -49,11 +49,18 @@ func (self *containerStorage) updatePrevStats(stats *info.ContainerStats) { } // make a deep copy. self.prevStats.Timestamp = stats.Timestamp + // copy the slice first. + percpuSlice := self.prevStats.Cpu.Usage.PerCpu *self.prevStats.Cpu = *stats.Cpu - self.prevStats.Cpu.Usage.PerCpu = make([]uint64, len(stats.Cpu.Usage.PerCpu)) - for i, perCpu := range stats.Cpu.Usage.PerCpu { - self.prevStats.Cpu.Usage.PerCpu[i] = perCpu + // If the old slice is enough to hold the new data, then don't allocate + // a new slice. + if len(percpuSlice) != len(stats.Cpu.Usage.PerCpu) { + percpuSlice = make([]uint64, len(stats.Cpu.Usage.PerCpu)) } + for i, perCpu := range stats.Cpu.Usage.PerCpu { + percpuSlice[i] = perCpu + } + self.prevStats.Cpu.Usage.PerCpu = percpuSlice *self.prevStats.Memory = *stats.Memory } @@ -75,9 +82,9 @@ func (self *containerStorage) AddStats(stats *info.ContainerStats) error { } } if self.recentStats.Len() >= self.maxNumStats { - self.recentStats.Remove(self.recentStats.Front()) + self.recentStats.Remove(self.recentStats.Back()) } - self.recentStats.PushBack(stats) + self.recentStats.PushFront(stats) self.updatePrevStats(stats) return nil } @@ -88,18 +95,25 @@ func (self *containerStorage) RecentStats(numStats int) ([]*info.ContainerStats, if self.recentStats.Len() < numStats || numStats < 0 { numStats = self.recentStats.Len() } - ret := make([]*info.ContainerStats, 0, numStats) + + // Stats in the recentStats list are stored in reverse chronological + // order, i.e. most recent stats is in the front. + // numStats will always <= recentStats.Len() so that there will be + // always at least numStats available stats to retrieve. We traverse + // the recentStats list from its head and fill the ret slice in + // reverse order so that the returned slice will be in chronological + // order. The order of the returned slice is not specified by the + // StorageDriver interface, so it is not necessary for other storage + // drivers to return the slice in the same order. + ret := make([]*info.ContainerStats, numStats) e := self.recentStats.Front() - for i := 0; i < numStats; i++ { + for i := numStats - 1; i >= 0; i-- { data, ok := e.Value.(*info.ContainerStats) if !ok { return nil, fmt.Errorf("The %vth element is not a ContainerStats", i) } - ret = append(ret, data) + ret[i] = data e = e.Next() - if e == nil { - break - } } return ret, nil } @@ -162,23 +176,34 @@ type InMemoryStorage struct { func (self *InMemoryStorage) AddStats(ref info.ContainerReference, stats *info.ContainerStats) error { var cstore *containerStorage var ok bool - self.lock.Lock() - if cstore, ok = self.containerStorageMap[ref.Name]; !ok { - cstore = newContainerStore(ref, self.maxNumSamples, self.maxNumStats) - self.containerStorageMap[ref.Name] = cstore - } - self.lock.Unlock() + + func() { + self.lock.Lock() + defer self.lock.Unlock() + if cstore, ok = self.containerStorageMap[ref.Name]; !ok { + cstore = newContainerStore(ref, self.maxNumSamples, self.maxNumStats) + self.containerStorageMap[ref.Name] = cstore + } + }() return cstore.AddStats(stats) } func (self *InMemoryStorage) Samples(name string, numSamples int) ([]*info.ContainerStatsSample, error) { var cstore *containerStorage var ok bool - self.lock.RLock() - if cstore, ok = self.containerStorageMap[name]; !ok { - return nil, fmt.Errorf("unable to find data for container %v", name) + + err := func() error { + self.lock.RLock() + defer self.lock.RUnlock() + if cstore, ok = self.containerStorageMap[name]; !ok { + return fmt.Errorf("unable to find data for container %v", name) + } + return nil + }() + + if err != nil { + return nil, err } - self.lock.RUnlock() return cstore.Samples(numSamples) } @@ -186,11 +211,17 @@ func (self *InMemoryStorage) Samples(name string, numSamples int) ([]*info.Conta func (self *InMemoryStorage) RecentStats(name string, numStats int) ([]*info.ContainerStats, error) { var cstore *containerStorage var ok bool - self.lock.RLock() - if cstore, ok = self.containerStorageMap[name]; !ok { - return nil, fmt.Errorf("unable to find data for container %v", name) + err := func() error { + self.lock.RLock() + defer self.lock.RUnlock() + if cstore, ok = self.containerStorageMap[name]; !ok { + return fmt.Errorf("unable to find data for container %v", name) + } + return nil + }() + if err != nil { + return nil, err } - self.lock.RUnlock() return cstore.RecentStats(numStats) } @@ -198,11 +229,17 @@ func (self *InMemoryStorage) RecentStats(name string, numStats int) ([]*info.Con func (self *InMemoryStorage) Percentiles(name string, cpuPercentiles, memPercentiles []int) (*info.ContainerStatsPercentiles, error) { var cstore *containerStorage var ok bool - self.lock.RLock() - if cstore, ok = self.containerStorageMap[name]; !ok { - return nil, fmt.Errorf("unable to find data for container %v", name) + err := func() error { + self.lock.RLock() + defer self.lock.RUnlock() + if cstore, ok = self.containerStorageMap[name]; !ok { + return fmt.Errorf("unable to find data for container %v", name) + } + return nil + }() + if err != nil { + return nil, err } - self.lock.RUnlock() return cstore.Percentiles(cpuPercentiles, memPercentiles) } diff --git a/storage/memory/memory_test.go b/storage/memory/memory_test.go index 98f9a6a8..394e48f6 100644 --- a/storage/memory/memory_test.go +++ b/storage/memory/memory_test.go @@ -44,6 +44,32 @@ func TestSamplesWithoutSample(t *testing.T) { runStorageTest(test.StorageDriverTestSamplesWithoutSample, t) } -func TestPercentilessWithoutSample(t *testing.T) { +func TestPercentilesWithoutSample(t *testing.T) { runStorageTest(test.StorageDriverTestPercentilesWithoutSample, t) } + +func TestPercentiles(t *testing.T) { + N := 100 + driver := New(N, N) + test.StorageDriverTestPercentiles(driver, t) +} + +func TestRetrievePartialRecentStats(t *testing.T) { + runStorageTest(test.StorageDriverTestRetrievePartialRecentStats, t) +} + +func TestRetrieveAllRecentStats(t *testing.T) { + runStorageTest(test.StorageDriverTestRetrieveAllRecentStats, t) +} + +func TestNoRecentStats(t *testing.T) { + runStorageTest(test.StorageDriverTestNoRecentStats, t) +} + +func TestNoSamples(t *testing.T) { + runStorageTest(test.StorageDriverTestNoSamples, t) +} + +func TestPercentilesWithoutStats(t *testing.T) { + runStorageTest(test.StorageDriverTestPercentilesWithoutStats, t) +} diff --git a/storage/test/storagetests.go b/storage/test/storagetests.go index e0f27f76..7a45cc8a 100644 --- a/storage/test/storagetests.go +++ b/storage/test/storagetests.go @@ -16,6 +16,7 @@ package test import ( "math/rand" + "reflect" "testing" "time" @@ -43,6 +44,7 @@ func buildTrace(cpu, mem []uint64, duration time.Duration) []*info.ContainerStat stats.Cpu.Usage.Total = cpuTotalUsage stats.Cpu.Usage.User = stats.Cpu.Usage.Total stats.Cpu.Usage.System = 0 + stats.Cpu.Usage.PerCpu = []uint64{cpuUsage} stats.Memory.Usage = mem[i] @@ -51,10 +53,39 @@ func buildTrace(cpu, mem []uint64, duration time.Duration) []*info.ContainerStat return ret } -// The underlying driver must be able to hold more than 10 samples. +func samplesInTrace(samples []*info.ContainerStatsSample, cpuTrace, memTrace []uint64, samplePeriod time.Duration, t *testing.T) { + for _, sample := range samples { + if sample.Duration != samplePeriod { + t.Errorf("sample duration is %v, not %v", sample.Duration, samplePeriod) + } + cpuUsage := sample.Cpu.Usage + memUsage := sample.Memory.Usage + found := false + for _, u := range cpuTrace { + if u == cpuUsage { + found = true + break + } + } + if !found { + t.Errorf("unable to find cpu usage %v", cpuUsage) + } + found = false + for _, u := range memTrace { + if u == memUsage { + found = true + break + } + } + if !found { + t.Errorf("unable to find mem usage %v", memUsage) + } + } +} + func StorageDriverTestSampleCpuUsage(driver storage.StorageDriver, t *testing.T) { defer driver.Close() - N := 10 + N := 100 cpuTrace := make([]uint64, 0, N) memTrace := make([]uint64, 0, N) @@ -73,28 +104,34 @@ func StorageDriverTestSampleCpuUsage(driver storage.StorageDriver, t *testing.T) trace := buildTrace(cpuTrace, memTrace, samplePeriod) for _, stats := range trace { - driver.AddStats(ref, stats) + err := driver.AddStats(ref, stats) + if err != nil { + t.Fatalf("unable to add stats: %v", err) + } + // set the trace to something else. The stats stored in the + // storage should not be affected. + stats.Cpu.Usage.Total = 0 + stats.Cpu.Usage.System = 0 + stats.Cpu.Usage.User = 0 } samples, err := driver.Samples(ref.Name, N) if err != nil { t.Errorf("unable to sample stats: %v", err) } - for _, sample := range samples { - if sample.Duration != samplePeriod { - t.Errorf("sample duration is %v, not %v", sample.Duration, samplePeriod) - } - cpuUsage := sample.Cpu.Usage - found := false - for _, u := range cpuTrace { - if u == cpuUsage { - found = true - } - } - if !found { - t.Errorf("unable to find cpu usage %v", cpuUsage) - } + samplesInTrace(samples, cpuTrace, memTrace, samplePeriod, t) + + samples, err = driver.Samples(ref.Name, -1) + if err != nil { + t.Errorf("unable to sample stats: %v", err) } + samplesInTrace(samples, cpuTrace, memTrace, samplePeriod, t) + + samples, err = driver.Samples(ref.Name, N-5) + if err != nil { + t.Errorf("unable to sample stats: %v", err) + } + samplesInTrace(samples, cpuTrace, memTrace, samplePeriod, t) } func StorageDriverTestMaxMemoryUsage(driver storage.StorageDriver, t *testing.T) { @@ -114,7 +151,16 @@ func StorageDriverTestMaxMemoryUsage(driver storage.StorageDriver, t *testing.T) trace := buildTrace(cpuTrace, memTrace, 1*time.Second) for _, stats := range trace { - driver.AddStats(ref, stats) + err := driver.AddStats(ref, stats) + if err != nil { + t.Fatalf("unable to add stats: %v", err) + } + // set the trace to something else. The stats stored in the + // storage should not be affected. + stats.Cpu.Usage.Total = 0 + stats.Cpu.Usage.System = 0 + stats.Cpu.Usage.User = 0 + stats.Memory.Usage = 0 } percentiles, err := driver.Percentiles(ref.Name, []int{50}, []int{50}) @@ -168,3 +214,176 @@ func StorageDriverTestPercentilesWithoutSample(driver storage.StorageDriver, t * t.Errorf("There should be no percentiles") } } + +func StorageDriverTestPercentiles(driver storage.StorageDriver, t *testing.T) { + defer driver.Close() + N := 100 + cpuTrace := make([]uint64, N) + memTrace := make([]uint64, N) + for i := 1; i < N+1; i++ { + cpuTrace[i-1] = uint64(i) + memTrace[i-1] = uint64(i) + } + + trace := buildTrace(cpuTrace, memTrace, 1*time.Second) + + ref := info.ContainerReference{ + Name: "container", + } + for _, stats := range trace { + driver.AddStats(ref, stats) + } + percentages := []int{ + 80, + 90, + 50, + } + percentiles, err := driver.Percentiles(ref.Name, percentages, percentages) + if err != nil { + t.Fatal(err) + } + for _, x := range percentiles.CpuUsagePercentiles { + for _, y := range percentiles.CpuUsagePercentiles { + // lower percentage, smaller value + if x.Percentage < y.Percentage && x.Value > y.Value { + t.Errorf("%v percent is %v; while %v percent is %v", + x.Percentage, x.Value, y.Percentage, y.Value) + } + } + } + for _, x := range percentiles.MemoryUsagePercentiles { + for _, y := range percentiles.MemoryUsagePercentiles { + if x.Percentage < y.Percentage && x.Value > y.Value { + t.Errorf("%v percent is %v; while %v percent is %v", + x.Percentage, x.Value, y.Percentage, y.Value) + } + } + } +} + +func StorageDriverTestRetrievePartialRecentStats(driver storage.StorageDriver, t *testing.T) { + defer driver.Close() + N := 100 + memTrace := make([]uint64, N) + cpuTrace := make([]uint64, N) + for i := 0; i < N; i++ { + memTrace[i] = uint64(i + 1) + cpuTrace[i] = uint64(1) + } + + ref := info.ContainerReference{ + Name: "container", + } + + trace := buildTrace(cpuTrace, memTrace, 1*time.Second) + + for _, stats := range trace { + driver.AddStats(ref, stats) + } + + recentStats, err := driver.RecentStats(ref.Name, 10) + if err != nil { + t.Fatal(err) + } + + if len(recentStats) > 10 { + t.Fatalf("returned %v stats, not 10.", len(recentStats)) + } + + actualRecentStats := trace[len(trace)-len(recentStats):] + + for _, r := range recentStats { + found := false + for _, s := range actualRecentStats { + if reflect.DeepEqual(s, r) { + found = true + } + } + if !found { + t.Errorf("unexpected stats %+v with memory usage %v", r, r.Memory.Usage) + } + } +} + +func StorageDriverTestRetrieveAllRecentStats(driver storage.StorageDriver, t *testing.T) { + defer driver.Close() + N := 100 + memTrace := make([]uint64, N) + cpuTrace := make([]uint64, N) + for i := 0; i < N; i++ { + memTrace[i] = uint64(i + 1) + cpuTrace[i] = uint64(1) + } + + ref := info.ContainerReference{ + Name: "container", + } + + trace := buildTrace(cpuTrace, memTrace, 1*time.Second) + + for _, stats := range trace { + driver.AddStats(ref, stats) + } + + recentStats, err := driver.RecentStats(ref.Name, -1) + if err != nil { + t.Fatal(err) + } + if len(recentStats) > N { + t.Fatalf("returned %v stats, not 100.", len(recentStats)) + } + + actualRecentStats := trace[len(trace)-len(recentStats):] + + for _, r := range recentStats { + found := false + for _, s := range actualRecentStats { + if reflect.DeepEqual(s, r) { + found = true + } + } + if !found { + t.Errorf("unexpected stats %+v with memory usage %v", r, r.Memory.Usage) + } + } +} + +func StorageDriverTestNoRecentStats(driver storage.StorageDriver, t *testing.T) { + defer driver.Close() + nonExistContainer := "somerandomecontainer" + stats, _ := driver.RecentStats(nonExistContainer, -1) + if len(stats) > 0 { + t.Errorf("RecentStats() returns %v stats on non exist container", len(stats)) + } +} + +func StorageDriverTestNoSamples(driver storage.StorageDriver, t *testing.T) { + defer driver.Close() + nonExistContainer := "somerandomecontainer" + samples, _ := driver.Samples(nonExistContainer, -1) + if len(samples) > 0 { + t.Errorf("Samples() returns %v samples on non exist container", len(samples)) + } +} + +func StorageDriverTestPercentilesWithoutStats(driver storage.StorageDriver, t *testing.T) { + defer driver.Close() + nonExistContainer := "somerandomecontainer" + percentiles, _ := driver.Percentiles(nonExistContainer, []int{50, 80}, []int{50, 80}) + if percentiles == nil { + return + } + if percentiles.MaxMemoryUsage != 0 { + t.Errorf("Percentiles() reports max memory usage > 0 when there's no stats.") + } + for _, p := range percentiles.CpuUsagePercentiles { + if p.Value != 0 { + t.Errorf("Percentiles() reports cpu usage is %v when there's no stats.", p.Value) + } + } + for _, p := range percentiles.MemoryUsagePercentiles { + if p.Value != 0 { + t.Errorf("Percentiles() reports memory usage is %v when there's no stats.", p.Value) + } + } +}