fix(mascot): stop phrase-regen from swap-spiraling memory-pressured boxes#256
Open
yalexx wants to merge 1 commit into
Open
fix(mascot): stop phrase-regen from swap-spiraling memory-pressured boxes#256yalexx wants to merge 1 commit into
yalexx wants to merge 1 commit into
Conversation
…oxes Field incident (customer box, Jul 6): the background mascot phrase regen picked llama3.2:3b, timed out at 60s, and because a failed generation never updates the cache, the still-stale cache retried the full model load on every mascot fetch — 2.6GB RAM + 145% CPU every ~90s until the box hit 92% RAM + 100% swap. - 12h failure backoff persisted in KV: a timeout/bad-output/low-memory attempt blocks background retries; explicit Settings regen bypasses it - tiny models only (<=2B): drop 3B+ from the preferred list and stop falling back to 'first installed model' (could be a user-pulled 7B+) - RAM headroom guard: skip generation below 3GB MemAvailable - keep_alive: 0 so Ollama unloads the model right after the call instead of holding gigabytes for the default 5 minutes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe mascot phrase generation logic in src/lib/mascot-phrases-server.ts adds persistent failure backoff tracking via KV storage, a RAM headroom check before running Ollama models, restricts model selection to small (≤2B) models, and unloads models immediately after use via keep_alive: 0. ChangesGeneration Gating and Model Safety
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant RegenerateFn as maybeRegenerateInBackground/forceRegenerate
participant MemCheck as hasMemoryHeadroom
participant KV
participant Ollama
Caller->>RegenerateFn: trigger regeneration
RegenerateFn->>KV: read last failure timestamp
RegenerateFn->>MemCheck: check available RAM
MemCheck-->>RegenerateFn: headroom result
alt backoff not elapsed or low memory
RegenerateFn->>KV: record failure timestamp
RegenerateFn-->>Caller: skip generation
else proceed
RegenerateFn->>Ollama: generate phrases (keep_alive 0)
alt success
Ollama-->>RegenerateFn: phrases
RegenerateFn->>KV: clear failure timestamp
else failure
Ollama-->>RegenerateFn: error
RegenerateFn->>KV: record failure timestamp
end
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/mascot-phrases-server.ts (1)
345-356: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear the failure backoff only after the cache write succeeds.
Line 345 and Line 395 clear the persisted backoff before
writeCache()succeeds. If that write throws, the cache stays stale while the backoff is cleared, so future mascot fetches can retry generation repeatedly.As per path instructions:
src/lib/**are TypeScript server-side libraries and should be reviewed for proper error handling and type safety.Proposed fix
- clearFailure(); - const now = Date.now(); const existingPhrases = cached?.phrases ?? INSPIRATION_PHRASES; const merged = mergeBatch(existingPhrases, fresh, mode); - writeCache({ - phrases: merged, - language: ctx.language, - lastFullRegen: mode === "full" ? now : (cached?.lastFullRegen ?? now), - lastTopUp: now, - }); + try { + writeCache({ + phrases: merged, + language: ctx.language, + lastFullRegen: mode === "full" ? now : (cached?.lastFullRegen ?? now), + lastTopUp: now, + }); + clearFailure(); + } catch (err) { + recordFailure(); + throw err; + }- clearFailure(); const now = Date.now(); - writeCache({ - phrases: fresh, - language: ctx.language, - lastFullRegen: now, - lastTopUp: now, - }); + try { + writeCache({ + phrases: fresh, + language: ctx.language, + lastFullRegen: now, + lastTopUp: now, + }); + clearFailure(); + } catch (err) { + recordFailure(); + throw err; + } return fresh;Also applies to: 395-402
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/mascot-phrases-server.ts` around lines 345 - 356, The failure backoff is being cleared too early in the mascot phrase generation flow, before the cache persistence succeeds. Update the relevant logic in the mascot phrases server code around the `clearFailure()` and `writeCache()` calls so the backoff is only cleared after `writeCache()` completes successfully, and apply the same sequencing in the other affected block referenced by the review. Keep the existing symbols like `clearFailure`, `writeCache`, `mergeBatch`, and the regen path in `src/lib/mascot-phrases-server.ts` so the persisted state and backoff stay consistent if cache writes throw.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/mascot-phrases-server.ts`:
- Around line 345-356: The failure backoff is being cleared too early in the
mascot phrase generation flow, before the cache persistence succeeds. Update the
relevant logic in the mascot phrases server code around the `clearFailure()` and
`writeCache()` calls so the backoff is only cleared after `writeCache()`
completes successfully, and apply the same sequencing in the other affected
block referenced by the review. Keep the existing symbols like `clearFailure`,
`writeCache`, `mergeBatch`, and the regen path in
`src/lib/mascot-phrases-server.ts` so the persisted state and backoff stay
consistent if cache writes throw.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 014132e5-0de3-4890-a8c9-9ca3f19f2954
📒 Files selected for processing (1)
src/lib/mascot-phrases-server.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Field incident
Customer box (Jul 6):
maybeRegenerateInBackground()picked llama3.2:3b, inference timed out at 60s — and because a failed generation never writes the cache, the stale cache retried the full 2GB model load on every mascot fetch (~every 90s), driving the box to 92% RAM + 100% swap until everything else fell over.Fixes
PREFERRED_MODELSand removed the "first installed model" fallback (could select a user-pulled 7B+)MemAvailable(fails open on non-Linux dev machines)keep_alive: 0— Ollama unloads the model immediately after the call instead of holding ~2.6GB for the default 5minTesting
tsc --noEmitclean🤖 Generated with Claude Code
Summary by CodeRabbit