Skip to content

Frontend Asset Optimization #3

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

Closed
ShashankFC wants to merge 1 commit into from
Closed

Frontend Asset Optimization #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 13 additions & 3 deletions pkg/api/webassets/webassets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"os"
"path/filepath"
"sync"

"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/services/licensing"
Expand All @@ -31,12 +32,21 @@ type EntryPointInfo struct {
} `json:"assets,omitempty"`
}

var entryPointAssetsCache *dtos.EntryPointAssets = nil
var (
entryPointAssetsCacheMu sync.RWMutex // guard entryPointAssetsCache
entryPointAssetsCache *dtos.EntryPointAssets // TODO: get rid of global state
)

func GetWebAssets(ctx context.Context, cfg *setting.Cfg, license licensing.Licensing) (*dtos.EntryPointAssets, error) {
if cfg.Env != setting.Dev && entryPointAssetsCache != nil {
return entryPointAssetsCache, nil
entryPointAssetsCacheMu.RLock()
ret := entryPointAssetsCache
entryPointAssetsCacheMu.RUnlock()

if cfg.Env != setting.Dev && ret != nil {
return ret, nil
}
entryPointAssetsCacheMu.Lock()
defer entryPointAssetsCacheMu.Unlock()
Comment on lines +48 to +49

@codeant-ai codeant-ai Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: In concurrent scenarios, multiple goroutines can race through the initial read-lock check when the cache is still empty, all then contend for the write lock and re-compute the assets even if one goroutine has already populated the cache, causing redundant manifest reads and undermining the intent of caching under load; re-checking the cache after acquiring the write lock avoids this double initialization. [possible bug]

Severity Level: Major ⚠️ 
- ⚠️ Cold-start concurrent index requests do repeated asset manifest reads.
- ⚠️ Increases latency for first wave of UI/API requests.
- ⚠️ Extra filesystem I/O under load; cache underutilized initially.
Suggested change
entryPointAssetsCacheMu.Lock()
defer entryPointAssetsCacheMu.Unlock()
entryPointAssetsCacheMu.Lock()
defer entryPointAssetsCacheMu.Unlock()
// Re-check under write lock in case another goroutine populated the cache
if cfg.Env != setting.Dev && entryPointAssetsCache != nil {
return entryPointAssetsCache, nil
}
Steps of Reproduction ✅ 
1. Start Grafana with a non-dev environment (cfg.Env != setting.Dev) so the cache path in
`GetWebAssets` is active, and note that the global `entryPointAssetsCache` is nil at
process start (`pkg/api/webassets/webassets.go:35-40`).

2. Immediately after startup, send many concurrent HTTP requests to the main index route
handled by `HTTPServer.Index` (`pkg/api/index.go:217-224`), which calls `setIndexViewData`
(`index.go:25`) and then `webassets.GetWebAssets(c.Req.Context(), hs.Cfg, hs.License)`
(`index.go:82`).

3. At the beginning of `GetWebAssets` (`pkg/api/webassets/webassets.go:40-47`), each
goroutine acquires the read lock, reads `entryPointAssetsCache` into `ret` (currently
nil), releases the read lock, and because `ret == nil`, all goroutines skip the early
return and proceed towards the write lock.

4. The goroutines then serialize on the write lock (`webassets.go:48-49`); each one, in
turn, executes the manifest loading logic (`webassets.go:54-66`), repeatedly calling
`readWebAssetsFromCDN`/`readWebAssetsFromFile`, and finally assigning
`entryPointAssetsCache = result` (`webassets.go:69`) even though a previous goroutine has
already populated it—this can be observed by instrumenting `readWebAssetsFromFile`
(`webassets.go:73-83`) to log or count file opens and seeing multiple sequential reads of
`assets-manifest.json` during the first concurrent request burst.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** pkg/api/webassets/webassets.go
**Line:** 48:49
**Comment:**
	*Possible Bug: In concurrent scenarios, multiple goroutines can race through the initial read-lock check when the cache is still empty, all then contend for the write lock and re-compute the assets even if one goroutine has already populated the cache, causing redundant manifest reads and undermining the intent of caching under load; re-checking the cache after acquiring the write lock avoids this double initialization.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎


var err error
var result *dtos.EntryPointAssets
Expand Down
Loading