Refactor to use percentiles

coder · ammario · Nov 17, 2022 · Oct 25, 2022 · Nov 7, 2022 · Nov 7, 2022
commit 6f914a41a8f2f509dbcc2bdd4d8dcf62934d06b4
diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go
@@ -3,7 +3,6 @@ package databasefake
 import (
 	"context"
 	"database/sql"
-	"math"
 	"sort"
 	"strings"
 	"sync"
@@ -277,38 +276,18 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab
 		}
 	}
 
-	tryMedian := func(fs []float64) float64 {
+	tryPercentile := func(fs []float64, p float64) float64 {
 		if len(fs) == 0 {
 			return -1
 		}
 		sort.Float64s(fs)
-		return fs[len(fs)/2]
-	}
-
-	tryStddev := func(fs []float64) float64 {
-		if len(fs) == 0 {
-			return -1
-		}
-		var sum float64
-		for _, f := range fs {
-			sum += f
-		}
-		mean := sum / float64(len(fs))
-
-		var sumSqDiff float64
-		for _, f := range fs {
-			sumSqDiff += math.Pow(mean-f, 2)
-		}
-		return sumSqDiff / float64(len(fs))
+		return fs[int(float64(len(fs))*p/100)]
 	}
 
 	var row database.GetTemplateAverageBuildTimeRow
-	row.DeleteMedian = tryMedian(deleteTimes)
-	row.DeleteStddev = tryStddev(deleteTimes)
-	row.StopMedian = tryMedian(stopTimes)
-	row.StopStddev = tryStddev(stopTimes)
-	row.StartMedian = tryMedian(startTimes)
-	row.StartStddev = tryStddev(startTimes)
+	row.Delete50, row.Delete95 = tryPercentile(deleteTimes, 50), tryPercentile(deleteTimes, 95)
+	row.Stop50, row.Stop95 = tryPercentile(stopTimes, 50), tryPercentile(stopTimes, 95)
+	row.Start50, row.Start95 = tryPercentile(startTimes, 50), tryPercentile(startTimes, 95)
 	return row, nil
 }
 

diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql
@@ -142,11 +142,11 @@ ORDER BY
 SELECT
 	-- Postgres offers no clear way to DRY this short of a function or other
 	-- complexities.
-	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_median,
-	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_median,
-	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median,
-	coalesce((stddev(exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_stddev,
-	coalesce((stddev(exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_stddev,
-	coalesce((stddev(exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_stddev
+	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_50,
+	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_50,
+	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_50,
+	coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_95,
+	coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_95,
+	coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_95
 FROM build_times
 ;
diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go
@@ -270,16 +270,16 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS
 
 	return codersdk.TemplateBuildTimeStats{
 		codersdk.WorkspaceTransitionStart: {
-			Median: convertMillis(resp.StartMedian),
-			Stddev: convertMillis(resp.StartStddev),
+			P50: convertMillis(resp.Start50),
+			P95: convertMillis(resp.Start95),
 		},
 		codersdk.WorkspaceTransitionStop: {
-			Median: convertMillis(resp.StopMedian),
-			Stddev: convertMillis(resp.StopStddev),
+			P50: convertMillis(resp.Stop50),
+			P95: convertMillis(resp.Stop95),
 		},
 		codersdk.WorkspaceTransitionDelete: {
-			Median: convertMillis(resp.DeleteMedian),
-			Stddev: convertMillis(resp.DeleteStddev),
+			P50: convertMillis(resp.Delete50),
+			P95: convertMillis(resp.Delete95),
 		},
 	}
 }
diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go
@@ -344,18 +344,21 @@ func TestCache_BuildTime(t *testing.T) {
 				gotStats = cache.TemplateBuildTimeStats(template.ID)
 				for transition, stats := range gotStats {
 					if transition == wantTransition {
-						require.Equal(t, tt.want.buildTimeMs, *stats.Median)
+						require.Equal(t, tt.want.buildTimeMs, *stats.P50)
 					} else {
-						require.Empty(t, stats)
+						require.Empty(
+							t, stats, "%v", transition,
+						)
 					}
 				}
 			} else {
+				var stats codersdk.TemplateBuildTimeStats
 				require.Never(t, func() bool {
-					stats := cache.TemplateBuildTimeStats(template.ID)
+					stats = cache.TemplateBuildTimeStats(template.ID)
 					requireBuildTimeStatsEmpty(t, stats)
 					return t.Failed()
 				}, testutil.WaitShort/2, testutil.IntervalMedium,
-					"BuildTimeStats populated",
+					"BuildTimeStats populated", stats,
 				)
 			}
 		})

diff --git a/coderd/templates_test.go b/coderd/templates_test.go
@@ -607,7 +607,7 @@ func TestTemplateMetrics(t *testing.T) {
 	require.Eventuallyf(t, func() bool {
 		template, err = client.Template(ctx, template.ID)
 		require.NoError(t, err)
-		startMs := template.BuildTimeStats[codersdk.WorkspaceTransitionStart].Median
+		startMs := template.BuildTimeStats[codersdk.WorkspaceTransitionStart].P50
 		return startMs != nil && *startMs > 1
 	},
 		testutil.WaitShort, testutil.IntervalFast,

diff --git a/codersdk/templates.go b/codersdk/templates.go
@@ -34,8 +34,8 @@ type Template struct {
 }
 
 type TransitionStats struct {
-	Median *int64
-	Stddev *int64
+	P50 *int64
+	P95 *int64
 }
 
 type TemplateBuildTimeStats map[WorkspaceTransition]TransitionStats