Frontend Asset Optimization

[ShashankFC]

User description

Test 8nnn## Summary by CodeRabbitnn* Refactorn * Improved internal performance and stability of web asset handling through enhanced thread-safety mechanisms.nn_{✏️ Tip: You can customize this high-level summary in your review settings.}nnn---nReplicated from ai-code-review-evaluation/grafana-coderabbit#8

---

CodeAnt-AI Description

Make web asset loading safe under concurrent requests

What Changed

• Web asset information cache is now read and written with proper synchronization so concurrent requests no longer race when loading frontend assets
• Cache still bypasses in development mode; in production the cached assets are returned to callers as before
• Observable behavior: fewer intermittent crashes or corrupted asset responses under high concurrency

Impact

✅ Fewer concurrency crashes when serving frontend assets
✅ More reliable asset responses under load
✅ Consistent asset caching behavior in production

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

[ShashankFC]

@CodeAnt-AI: review

[codeant-ai]

CodeAnt AI is running the review.

[codeant-ai]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review Caching on failure The code sets the global entryPointAssetsCache unconditionally to result even when result is nil or an error occurred. This can lead to caching a nil or incomplete state and prevent subsequent attempts to load assets. Only cache on success (non-nil result and no error). Double-checked locking The function performs an RLock -> read -> RUnlock and then acquires a write lock to load and set the cache, but it does not re-check the cache after acquiring the write lock. This can cause redundant concurrent loads and wasted work (or racey behavior around multiple goroutines loading simultaneously). Consider performing a second check under the write lock and returning cached data if another goroutine filled it.

[codeant-ai]

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.

👍 | 👎

[codeant-ai]

CodeAnt AI finished running the review.

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions!

[github-actions]

This pull request has been automatically closed because it has not had any further activity in the last 2 weeks. Thank you for your contributions!