Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: increase fileCache hit rate in autobuilds lifecycle#18507

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.

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

Already on GitHub?Sign in to your account

Merged
Emyrk merged 1 commit intomainfromstevenmasley/dynamic_auto_build_optimization
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
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
18 changes: 18 additions & 0 deletionscoderd/autobuild/lifecycle_executor.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,8 @@ import (
"database/sql"
"fmt"
"net/http"
"slices"
"strings"
"sync"
"sync/atomic"
"time"
Expand DownExpand Up@@ -155,6 +157,22 @@ func (e *Executor) runOnce(t time.Time) Stats {
returnstats
}

// Sort the workspaces by build template version ID so that we can group
// identical template versions together. This is a slight (and imperfect)
// optimization.
//
// `wsbuilder` needs to load the terraform files for a given template version
// into memory. If 2 workspaces are using the same template version, they will
// share the same files in the FileCache. This only happens if the builds happen
// in parallel.
// TODO: Actually make sure the cache has the files in the cache for the full
// set of identical template versions. Then unload the files when the builds
// are done. Right now, this relies on luck for the 10 goroutine workers to
// overlap and keep the file reference in the cache alive.
Comment on lines +164 to +171
Copy link
Member

Choose a reason for hiding this comment

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

yeah, it could be nice if we did more of a "mark and sweep" style release strategy. or maybe generational...

oh no, am I about to build a general purpose garbage collector for this? Oh No

Copy link
MemberAuthor

@EmyrkEmyrkJun 23, 2025
edited
Loading

Choose a reason for hiding this comment

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

Yea, I had a similar set of thoughts and decided this was a good enough idea for this release.

There is a better way to do this for sure, and I wonder where else in the codebase we will have similar things going on (prebuilds?). That being a large set of workspaces that have some work to be done in bulk.

slices.SortFunc(workspaces,func(a,b database.GetWorkspacesEligibleForTransitionRow)int {
returnstrings.Compare(a.BuildTemplateVersionID.UUID.String(),b.BuildTemplateVersionID.UUID.String())
})

// We only use errgroup here for convenience of API, not for early
// cancellation. This means we only return nil errors in th eg.Go.
eg:= errgroup.Group{}
Expand Down
10 changes: 6 additions & 4 deletionscoderd/database/queries.sql.go
View file
Open in desktop

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

3 changes: 2 additions & 1 deletioncoderd/database/queries/workspaces.sql
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -642,7 +642,8 @@ FROM pending_workspaces, building_workspaces, running_workspaces, failed_workspa
-- name: GetWorkspacesEligibleForTransition :many
SELECT
workspaces.id,
workspaces.name
workspaces.name,
workspace_builds.template_version_id as build_template_version_id
FROM
workspaces
LEFT JOIN
Expand Down
12 changes: 7 additions & 5 deletionscoderd/files/cache.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -71,12 +71,12 @@ func (c *Cache) registerMetrics(registerer prometheus.Registerer) *Cache {
Help: "The count of file references currently open in the file cache. Multiple references can be held for the same file.",
})

c.totalOpenFileReferences = f.NewCounter(prometheus.CounterOpts{
c.totalOpenFileReferences = f.NewCounterVec(prometheus.CounterOpts{
Namespace: "coderd",
Subsystem: subsystem,
Name: "open_file_refs_total",
Help: "The total number of file references ever opened in the file cache.",
})
Help: "The total number of file references ever opened in the file cache. The 'hit' label indicates if the file was loaded from the cache.",
}, []string{"hit"})

return c
}
Expand All@@ -97,7 +97,7 @@ type Cache struct {

type cacheMetrics struct {
currentOpenFileReferences prometheus.Gauge
totalOpenFileReferences prometheus.Counter
totalOpenFileReferences*prometheus.CounterVec

currentOpenFiles prometheus.Gauge
totalOpenedFiles prometheus.Counter
Expand DownExpand Up@@ -173,6 +173,7 @@ func (c *Cache) prepare(ctx context.Context, db database.Store, fileID uuid.UUID
c.lock.Lock()
defer c.lock.Unlock()

hitLabel := "true"
entry, ok := c.data[fileID]
if !ok {
value := lazy.NewWithError(func() (CacheEntryValue, error) {
Expand All@@ -194,10 +195,11 @@ func (c *Cache) prepare(ctx context.Context, db database.Store, fileID uuid.UUID
c.data[fileID] = entry
c.currentOpenFiles.Inc()
c.totalOpenedFiles.Inc()
hitLabel = "false"
}

c.currentOpenFileReferences.Inc()
c.totalOpenFileReferences.Inc()
c.totalOpenFileReferences.WithLabelValues(hitLabel).Inc()
entry.refCount++
return entry.value
}
Expand Down
5 changes: 3 additions & 2 deletionscoderd/files/cache_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -161,7 +161,9 @@ func TestConcurrency(t *testing.T) {
require.Equal(t, batches, promhelp.GaugeValue(t, reg, cachePromMetricName("open_files_current"), nil))
require.Equal(t, batches, promhelp.CounterValue(t, reg, cachePromMetricName("open_files_total"), nil))
require.Equal(t, batches*batchSize, promhelp.GaugeValue(t, reg, cachePromMetricName("open_file_refs_current"), nil))
require.Equal(t, batches*batchSize, promhelp.CounterValue(t, reg, cachePromMetricName("open_file_refs_total"), nil))
hit, miss := promhelp.CounterValue(t, reg, cachePromMetricName("open_file_refs_total"), prometheus.Labels{"hit": "false"}),
promhelp.CounterValue(t, reg, cachePromMetricName("open_file_refs_total"), prometheus.Labels{"hit": "true"})
require.Equal(t, batches*batchSize, hit+miss)
}

func TestRelease(t *testing.T) {
Expand DownExpand Up@@ -245,7 +247,6 @@ func TestRelease(t *testing.T) {
// Total counts remain
require.Equal(t, batches*fileSize, promhelp.CounterValue(t, reg, cachePromMetricName("open_files_size_bytes_total"), nil))
require.Equal(t, batches, promhelp.CounterValue(t, reg, cachePromMetricName("open_files_total"), nil))
require.Equal(t, batches*batchSize, promhelp.CounterValue(t, reg, cachePromMetricName("open_file_refs_total"), nil))
}

func cacheAuthzSetup(t *testing.T) (database.Store, *files.Cache, *coderdtest.RecordingAuthorizer) {
Expand Down
Loading

[8]ページ先頭

©2009-2025 Movatter.jp