From 06c9dcd3bb47c57c122d8eb00af802fcbbbdff06 Mon Sep 17 00:00:00 2001
From: Rohit Jnagal <jnagal@google.com>
Date: Thu, 7 May 2015 16:44:22 +0000
Subject: [PATCH] Fix handling of time ranges in timed store.

The current logic assumes that entries are added to the store in
monotonically increasing order for time. This is not true when
we add creation events for existing containers.
---
 events/handler_test.go |  6 +++---
 utils/timed_store.go   | 48 +++++++++++++++++++++++++++---------------
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/events/handler_test.go b/events/handler_test.go
index 65b38897..abda4cbc 100644
--- a/events/handler_test.go
+++ b/events/handler_test.go
@@ -170,8 +170,8 @@ func TestGetEventsForOneEvent(t *testing.T) {
 
 func TestGetEventsForTimePeriod(t *testing.T) {
 	myEventHolder, myRequest, fakeEvent, fakeEvent2 := initializeScenario(t)
-	myRequest.StartTime = createOldTime(t).Add(-1 * time.Second * 10)
-	myRequest.EndTime = createOldTime(t).Add(time.Second * 10)
+	myRequest.StartTime = time.Now().Add(-1 * time.Second * 10)
+	myRequest.EndTime = time.Now().Add(time.Second * 10)
 	myRequest.EventType[info.EventOom] = true
 
 	myEventHolder.AddEvent(fakeEvent)
@@ -181,7 +181,7 @@ func TestGetEventsForTimePeriod(t *testing.T) {
 	assert.Nil(t, err)
 
 	checkNumberOfEvents(t, 1, len(receivedEvents))
-	ensureProperEventReturned(t, fakeEvent, receivedEvents[0])
+	ensureProperEventReturned(t, fakeEvent2, receivedEvents[0])
 }
 
 func TestGetEventsForNoTypeRequested(t *testing.T) {
diff --git a/utils/timed_store.go b/utils/timed_store.go
index cfd49236..70c310d3 100644
--- a/utils/timed_store.go
+++ b/utils/timed_store.go
@@ -19,10 +19,24 @@ import (
 	"time"
 )
 
+type timedStoreDataSlice []timedStoreData
+
+func (t timedStoreDataSlice) Less(i, j int) bool {
+	return t[i].timestamp.Before(t[j].timestamp)
+}
+
+func (t timedStoreDataSlice) Len() int {
+	return len(t)
+}
+
+func (t timedStoreDataSlice) Swap(i, j int) {
+	t[i], t[j] = t[j], t[i]
+}
+
 // A time-based buffer for ContainerStats.
 // Holds information for a specific time period and/or a max number of items.
 type TimedStore struct {
-	buffer   []timedStoreData
+	buffer   timedStoreDataSlice
 	age      time.Duration
 	maxItems int
 }
@@ -36,7 +50,7 @@ type timedStoreData struct {
 // A maxItems value of -1 means no limit.
 func NewTimedStore(age time.Duration, maxItems int) *TimedStore {
 	return &TimedStore{
-		buffer:   make([]timedStoreData, 0),
+		buffer:   make(timedStoreDataSlice, 0),
 		age:      age,
 		maxItems: maxItems,
 	}
@@ -44,7 +58,21 @@ func NewTimedStore(age time.Duration, maxItems int) *TimedStore {
 
 // Adds an element to the start of the buffer (removing one from the end if necessary).
 func (self *TimedStore) Add(timestamp time.Time, item interface{}) {
-	// Remove any elements before the eviction time.
+	// Remove any elements if over our max size.
+	if self.maxItems >= 0 && (len(self.buffer)+1) > self.maxItems {
+		startIndex := len(self.buffer) + 1 - self.maxItems
+		self.buffer = self.buffer[startIndex:]
+	}
+	// Add the new element first and sort. We can then remove an expired element, if required.
+	copied := item
+	self.buffer = append(self.buffer, timedStoreData{
+		timestamp: timestamp,
+		data:      copied,
+	})
+
+	sort.Sort(self.buffer)
+	// Remove any elements before eviction time.
+	// TODO(rjnagal): This is assuming that the added entry has timestamp close to now.
 	evictTime := timestamp.Add(-self.age)
 	index := sort.Search(len(self.buffer), func(index int) bool {
 		return self.buffer[index].timestamp.After(evictTime)
@@ -53,17 +81,6 @@ func (self *TimedStore) Add(timestamp time.Time, item interface{}) {
 		self.buffer = self.buffer[index:]
 	}
 
-	// Remove any elements if over our max size.
-	if self.maxItems >= 0 && (len(self.buffer)+1) > self.maxItems {
-		startIndex := len(self.buffer) + 1 - self.maxItems
-		self.buffer = self.buffer[startIndex:]
-	}
-
-	copied := item
-	self.buffer = append(self.buffer, timedStoreData{
-		timestamp: timestamp,
-		data:      copied,
-	})
 }
 
 // Returns up to maxResult elements in the specified time period (inclusive).
@@ -80,9 +97,6 @@ func (self *TimedStore) InTimeRange(start, end time.Time, maxResults int) []inte
 		maxResults = -1
 	}
 
-	// NOTE: Since we store the elments in descending timestamp order "start" will
-	// be a higher index than "end".
-
 	var startIndex int
 	if start.IsZero() {
 		// None specified, start at the beginning.