Fix store GC not remove all expired metrics

google · Jul 12, 2022 · f38c61d · f38c61d
1 parent 13d1e22
commit f38c61d
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 2 deletions.
diff --git a/internal/metrics/metric.go b/internal/metrics/metric.go
@@ -204,10 +204,12 @@ func (m *Metric) RemoveDatum(labelvalues ...string) error {
 	k := buildLabelValueKey(labelvalues)
 	olv, ok := m.labelValuesMap[k]
 	if ok {
-		for i, lv := range m.LabelValues {
+		for i := 0; i < len(m.LabelValues); i++ {
+			lv := m.LabelValues[i]
 			if lv == olv {
 				// remove from the slice
 				m.LabelValues = append(m.LabelValues[:i], m.LabelValues[i+1:]...)
+				i--
 				delete(m.labelValuesMap, k)
 				break
 			}

diff --git a/internal/metrics/store.go b/internal/metrics/store.go
@@ -162,7 +162,8 @@ func (s *Store) Gc() error {
 				m.RemoveOldestDatum()
 			}
 		}
-		for _, lv := range m.LabelValues {
+		for i := 0; i < len(m.LabelValues); i++ {
+			lv := m.LabelValues[i]
 			if lv.Expiry <= 0 {
 				continue
 			}
@@ -171,6 +172,7 @@ func (s *Store) Gc() error {
 				if err != nil {
 					return err
 				}
+				i--
 			}
 		}
 		return nil

diff --git a/internal/metrics/store_test.go b/internal/metrics/store_test.go
@@ -4,6 +4,7 @@
 package metrics
 
 import (
+	"strconv"
 	"testing"
 	"time"
 
@@ -150,3 +151,43 @@ func TestExpireOversizeDatum(t *testing.T) {
 		t.Errorf("found label a which is unexpected: %#v", x)
 	}
 }
+
+func TestExpireManyMetrics(t *testing.T) {
+	s := NewStore()
+	m := NewMetric("foo", "prog", Counter, Int, "id")
+	testutil.FatalIfErr(t, s.Add(m))
+	d, err := m.GetDatum("0")
+	if err != nil {
+		t.Error(err)
+	}
+	datum.SetInt(d, 1, time.Now().Add(-time.Hour))
+	lv := m.FindLabelValueOrNil([]string{"0"})
+	if lv == nil {
+		t.Fatal("couldn't find lv")
+	}
+
+	for i := 1; i < 10; i++ {
+		d, err := m.GetDatum(strconv.Itoa(i))
+		if err != nil {
+			t.Error(err)
+		}
+		datum.SetInt(d, 1, time.Now().Add(-time.Hour))
+		lv := m.FindLabelValueOrNil([]string{strconv.Itoa(i)})
+		if lv == nil {
+			t.Fatal("couldn't find lv")
+		}
+		lv.Expiry = time.Minute
+	}
+
+	testutil.FatalIfErr(t, s.Gc())
+	lv = m.FindLabelValueOrNil([]string{"8"})
+	if lv != nil {
+		t.Errorf("lv not expired: %#v", lv)
+		t.Logf("Store: %#v", s)
+	}
+	lv = m.FindLabelValueOrNil([]string{"0"})
+	if lv == nil {
+		t.Errorf("lv expired")
+		t.Logf("Store: %#v", s)
+	}
+}