Skip to content

chore: test metricscache on postgres #16711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 27, 2025
Merged

chore: test metricscache on postgres #16711

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0700680
chore: test metricscache on postgres
DanielleMaywood Feb 26, 2025
e4bb752
chore: appease linter
DanielleMaywood Feb 26, 2025
6b5334e
chore: revert to GetTemplates
DanielleMaywood Feb 26, 2025
9f5dddf
chore: wrap c.clock.Now in dbtime.Time
DanielleMaywood Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
chore: test metricscache on postgres 
metricscache_test has been running tests against dbmem only, instead of
against postgres. Unfortunately the implementations of
GetTemplateAverageBuildTime have diverged between dbmem and postgres.
This change gets the tests working on Postgres and test for the
behaviour postgres provides.
  • Loading branch information
@DanielleMaywood
DanielleMaywood committed Feb 26, 2025
commit 07006806892efbdad7cdc3607cebaa5a4427fd65
1 change: 1 addition & 0 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ func New(options *Options) *API {
metricsCache := metricscache.New(
options.Database,
options.Logger.Named("metrics_cache"),
options.Clock,
metricscache.Intervals{
TemplateBuildTimes: options.MetricsCacheRefreshInterval,
DeploymentStats: options.AgentStatsRefreshInterval,
Expand Down
36 changes: 22 additions & 14 deletions coderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ type data struct {
presetParameters []database.TemplateVersionPresetParameter
}

func tryPercentile(fs []float64, p float64) float64 {
func tryPercentileCont(fs []float64, p float64) float64 {
if len(fs) == 0 {
return -1
}
Expand All @@ -281,6 +281,14 @@ func tryPercentile(fs []float64, p float64) float64 {
return fs[lower] + (fs[upper]-fs[lower])*(pos-float64(lower))
}

func tryPercentileDisc(fs []float64, p float64) float64 {
if len(fs) == 0 {
return -1
}
sort.Float64s(fs)
return fs[max(int(math.Ceil(float64(len(fs))*p/100-1)), 0)]
}

func validateDatabaseTypeWithValid(v reflect.Value) (handled bool, err error) {
if v.Kind() == reflect.Struct {
return false, nil
Expand Down Expand Up @@ -2802,8 +2810,8 @@ func (q *FakeQuerier) GetDeploymentWorkspaceAgentStats(_ context.Context, create
latencies = append(latencies, agentStat.ConnectionMedianLatencyMS)
}

stat.WorkspaceConnectionLatency50 = tryPercentile(latencies, 50)
stat.WorkspaceConnectionLatency95 = tryPercentile(latencies, 95)
stat.WorkspaceConnectionLatency50 = tryPercentileCont(latencies, 50)
stat.WorkspaceConnectionLatency95 = tryPercentileCont(latencies, 95)

return stat, nil
}
Expand Down Expand Up @@ -2851,8 +2859,8 @@ func (q *FakeQuerier) GetDeploymentWorkspaceAgentUsageStats(_ context.Context, c
stat.WorkspaceTxBytes += agentStat.TxBytes
latencies = append(latencies, agentStat.ConnectionMedianLatencyMS)
}
stat.WorkspaceConnectionLatency50 = tryPercentile(latencies, 50)
stat.WorkspaceConnectionLatency95 = tryPercentile(latencies, 95)
stat.WorkspaceConnectionLatency50 = tryPercentileCont(latencies, 50)
stat.WorkspaceConnectionLatency95 = tryPercentileCont(latencies, 95)

for _, agentStat := range sessions {
stat.SessionCountVSCode += agentStat.SessionCountVSCode
Expand Down Expand Up @@ -4989,9 +4997,9 @@ func (q *FakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab
}

var row database.GetTemplateAverageBuildTimeRow
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)
row.Delete50, row.Delete95 = tryPercentileDisc(deleteTimes, 50), tryPercentileDisc(deleteTimes, 95)
row.Stop50, row.Stop95 = tryPercentileDisc(stopTimes, 50), tryPercentileDisc(stopTimes, 95)
row.Start50, row.Start95 = tryPercentileDisc(startTimes, 50), tryPercentileDisc(startTimes, 95)
return row, nil
}

Expand Down Expand Up @@ -6026,8 +6034,8 @@ func (q *FakeQuerier) GetUserLatencyInsights(_ context.Context, arg database.Get
Username: user.Username,
AvatarURL: user.AvatarURL,
TemplateIDs: seenTemplatesByUserID[userID],
WorkspaceConnectionLatency50: tryPercentile(latencies, 50),
WorkspaceConnectionLatency95: tryPercentile(latencies, 95),
WorkspaceConnectionLatency50: tryPercentileCont(latencies, 50),
WorkspaceConnectionLatency95: tryPercentileCont(latencies, 95),
}
rows = append(rows, row)
}
Expand Down Expand Up @@ -6671,8 +6679,8 @@ func (q *FakeQuerier) GetWorkspaceAgentStats(_ context.Context, createdAfter tim
if !ok {
continue
}
stat.WorkspaceConnectionLatency50 = tryPercentile(latencies, 50)
stat.WorkspaceConnectionLatency95 = tryPercentile(latencies, 95)
stat.WorkspaceConnectionLatency50 = tryPercentileCont(latencies, 50)
stat.WorkspaceConnectionLatency95 = tryPercentileCont(latencies, 95)
statByAgent[stat.AgentID] = stat
}

Expand Down Expand Up @@ -6809,8 +6817,8 @@ func (q *FakeQuerier) GetWorkspaceAgentUsageStats(_ context.Context, createdAt t
for key, latencies := range latestAgentLatencies {
val, ok := latestAgentStats[key]
if ok {
val.WorkspaceConnectionLatency50 = tryPercentile(latencies, 50)
val.WorkspaceConnectionLatency95 = tryPercentile(latencies, 95)
val.WorkspaceConnectionLatency50 = tryPercentileCont(latencies, 50)
val.WorkspaceConnectionLatency95 = tryPercentileCont(latencies, 95)
}
latestAgentStats[key] = val
}
Expand Down
12 changes: 5 additions & 7 deletions coderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 5 additions & 7 deletions coderd/database/queries/workspaces.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,11 @@ WHERE
ORDER BY created_at DESC;

-- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many
SELECT
template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum
FROM
workspaces
WHERE
template_id = ANY(@template_ids :: uuid[]) AND deleted = false
GROUP BY template_id;
SELECT templates.id AS template_id, COUNT(DISTINCT workspaces.owner_id) AS unique_owners_sum
FROM templates
LEFT JOIN workspaces ON workspaces.template_id = templates.id AND workspaces.deleted = false
WHERE templates.id = ANY(@template_ids :: uuid[])
GROUP BY templates.id;
Comment on lines +418 to +422
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrote this query as the test-suite expected this query to be able to return unique_owners_sum=0, however the previous implementation would return nothing if there were no workspaces.


-- name: InsertWorkspace :one
INSERT INTO
Expand Down
15 changes: 9 additions & 6 deletions coderd/metricscache/metricscache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/quartz"
"github.com/coder/retry"
)

Expand All @@ -26,6 +27,7 @@ import (
type Cache struct {
database database.Store
log slog.Logger
clock quartz.Clock
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quartz.Clock has been added to make the tests more deterministic.

intervals Intervals

templateWorkspaceOwners atomic.Pointer[map[uuid.UUID]int]
Expand All @@ -45,7 +47,7 @@ type Intervals struct {
DeploymentStats time.Duration
}

func New(db database.Store, log slog.Logger, intervals Intervals, usage bool) *Cache {
func New(db database.Store, log slog.Logger, clock quartz.Clock, intervals Intervals, usage bool) *Cache {
if intervals.TemplateBuildTimes <= 0 {
intervals.TemplateBuildTimes = time.Hour
}
Expand All @@ -55,6 +57,7 @@ func New(db database.Store, log slog.Logger, intervals Intervals, usage bool) *C
ctx, cancel := context.WithCancel(context.Background())

c := &Cache{
clock: clock,
database: db,
intervals: intervals,
log: log,
Expand Down Expand Up @@ -84,7 +87,7 @@ func (c *Cache) refreshTemplateBuildTimes(ctx context.Context) error {
//nolint:gocritic // This is a system service.
ctx = dbauthz.AsSystemRestricted(ctx)

templates, err := c.database.GetTemplates(ctx)
templates, err := c.database.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{})
if err != nil {
return err
}
Expand All @@ -104,7 +107,7 @@ func (c *Cache) refreshTemplateBuildTimes(ctx context.Context) error {
Valid: true,
},
StartTime: sql.NullTime{
Time: dbtime.Time(time.Now().AddDate(0, 0, -30)),
Time: dbtime.Time(c.clock.Now().AddDate(0, 0, -30)),
Valid: true,
},
})
Expand All @@ -131,7 +134,7 @@ func (c *Cache) refreshTemplateBuildTimes(ctx context.Context) error {

func (c *Cache) refreshDeploymentStats(ctx context.Context) error {
var (
from = dbtime.Now().Add(-15 * time.Minute)
from = c.clock.Now().Add(-15 * time.Minute)
agentStats database.GetDeploymentWorkspaceAgentStatsRow
err error
)
Expand All @@ -155,8 +158,8 @@ func (c *Cache) refreshDeploymentStats(ctx context.Context) error {
}
c.deploymentStatsResponse.Store(&codersdk.DeploymentStats{
AggregatedFrom: from,
CollectedAt: dbtime.Now(),
NextUpdateAt: dbtime.Now().Add(c.intervals.DeploymentStats),
CollectedAt: c.clock.Now(),
NextUpdateAt: c.clock.Now().Add(c.intervals.DeploymentStats),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be wrapped in a dbtime.Time like above for the microsecond rounding?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, forgot to add that

Workspaces: codersdk.WorkspaceDeploymentStats{
Pending: workspaceStats.PendingWorkspaces,
Building: workspaceStats.BuildingWorkspaces,
Expand Down
Loading
Loading