Skip to content

Commit

Permalink
Return an error if no data is available yet instead of 0.
Browse files Browse the repository at this point in the history
  • Loading branch information
markusthoemmes committed May 9, 2019
1 parent af9471a commit 2268d5b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 7 deletions.
8 changes: 8 additions & 0 deletions pkg/autoscaler/aggregation/bucketing.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ func (t *TimedFloat64Buckets) Record(time time.Time, name string, value float64)
bucket.Record(name, value)
}

// IsEmpty returns whether or not there are no values currently stored.
func (t *TimedFloat64Buckets) IsEmpty() bool {
t.bucketsMutex.RLock()
defer t.bucketsMutex.RUnlock()

return len(t.buckets) == 0
}

// ForEachBucket calls the given Accumulator function for each bucket.
func (t *TimedFloat64Buckets) ForEachBucket(accs ...Accumulator) {
t.bucketsMutex.RLock()
Expand Down
4 changes: 2 additions & 2 deletions pkg/autoscaler/aggregation/bucketing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func TestTimedFloat64Buckets(t *testing.T) {
if !cmp.Equal(tt.want, got) {
t.Errorf("Unexpected values (-want +got): %v", cmp.Diff(tt.want, got))
}
if got := len(buckets.buckets); len(tt.want) == 0 && got != 0 {
t.Errorf("len(buckets) = %d, want 0", got)
if len(tt.want) == 0 && !buckets.IsEmpty() {
t.Error("IsEmpty() = false, want true")
}
})
}
Expand Down
13 changes: 9 additions & 4 deletions pkg/autoscaler/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package autoscaler

import (
"context"
"errors"
"sync"
"time"

Expand Down Expand Up @@ -200,8 +201,7 @@ func (c *MetricCollector) StableAndPanicConcurrency(key string) (float64, float6
return 0, 0, k8serrors.NewNotFound(kpa.Resource("Metrics"), key)
}

stable, panic := collection.stableAndPanicConcurrency(time.Now())
return stable, panic, nil
return collection.stableAndPanicConcurrency(time.Now())
}

// collection represents the collection of metrics for one specific entity.
Expand Down Expand Up @@ -274,18 +274,23 @@ func (c *collection) record(stat Stat) {

// stableAndPanicConcurrency calculates both stable and panic concurrency based on the
// current stats.
func (c *collection) stableAndPanicConcurrency(now time.Time) (float64, float64) {
func (c *collection) stableAndPanicConcurrency(now time.Time) (float64, float64, error) {
spec := c.currentMetric().Spec

c.buckets.RemoveOlderThan(now.Add(-spec.StableWindow))

if c.buckets.IsEmpty() {
return 0, 0, errors.New("no data available")
}

panicAverage := aggregation.Average{}
stableAverage := aggregation.Average{}
c.buckets.ForEachBucket(
aggregation.YoungerThan(now.Add(-spec.PanicWindow), panicAverage.Accumulate),
stableAverage.Accumulate, // No need to add a YoungerThan condition as we already deleted all outdated stats above.
)

return stableAverage.Value(), panicAverage.Value()
return stableAverage.Value(), panicAverage.Value(), nil
}

// close stops collecting metrics, stops the scraper.
Expand Down
8 changes: 7 additions & 1 deletion pkg/autoscaler/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,15 @@ func TestMetricCollectorRecord(t *testing.T) {
factory := scraperFactory(scraper, nil)

coll := NewMetricCollector(factory, logger)

// Freshly created collection does not contain any metrics and should return an error.
coll.Create(ctx, defaultMetric)
coll.Record(metricKey, stat)
if _, _, err := coll.StableAndPanicConcurrency(metricKey); err == nil {
t.Error("StableAndPanicConcurrency() = nil, wanted an error")
}

// After adding a stat the concurrencies are calculated correctly.
coll.Record(metricKey, stat)
if stable, panic, _ := coll.StableAndPanicConcurrency(metricKey); stable != panic && stable != want {
t.Errorf("StableAndPanicConcurrency() = %v, %v; want %v (for both)", stable, panic, want)
}
Expand Down

0 comments on commit 2268d5b

Please sign in to comment.