Clean up successful and failed jobs #343
Conversation
WalkthroughThis update introduces new environment variables to control job retention in Redis queues, modifies backend job scheduling to respect these settings, and updates related documentation. Additional optional logging environment variables are added on the web side. A redundant log message is removed from the entitlements server, and the changelog is updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Backend
participant RedisQueue
User->>Backend: Trigger job scheduling (e.g., sync, index, GC)
Backend->>RedisQueue: Add job with options (removeOnComplete, removeOnFail) from env
RedisQueue-->>Backend: Job processed, cleanup according to settings
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
CHANGELOG.md (1)
16-16: Consider re-wording the bullet for clarityThe phrase “Clean up successful and failed jobs in Redis queues” is clear, but readers might more easily associate the fix with the original issue if you explicitly mention the memory-leak symptom, e.g. “Prevent Redis memory leak by cleaning up completed/failed jobs”.
Optional, purely editorial.packages/web/src/env.mjs (1)
70-72: Missing docs entry for new LOGTAIL variables
LOGTAIL_TOKENandLOGTAIL_HOSTare now part of the runtime schema, but the environment-variables reference page wasn’t updated in this PR. Please add them so users know how to configure centralised logging.docs/docs/configuration/environment-variables.mdx (1)
26-27: Good addition – consider clarifying semanticsYou might add “
0removes jobs immediately” and “n > 0retains the most-recent n jobs” so users understand the numeric meaning without referring to BullMQ docs.packages/backend/src/repoManager.ts (1)
402-405: Apply custom job retention settings to garbage collection queue
This mirrors the index-queue changes by automatically removing completed/failed GC jobs per configuration. To reduce duplication, consider extracting theremoveOnComplete/removeOnFailoptions into a shared constant or helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)docs/docs/configuration/environment-variables.mdx(1 hunks)packages/backend/src/connectionManager.ts(1 hunks)packages/backend/src/env.ts(1 hunks)packages/backend/src/repoManager.ts(3 hunks)packages/web/src/env.mjs(1 hunks)packages/web/src/features/entitlements/server.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/features/entitlements/server.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/backend/src/connectionManager.ts (1)
packages/backend/src/env.ts (1)
env(22-55)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
packages/backend/src/connectionManager.ts (1)
68-70: Verify runtime values are numeric after the env fix
removeOnComplete/removeOnFailexpectboolean | number | {count|age}.
After changing the schema tonumberSchema,env.REDIS_REMOVE_ON_COMPLETEwill be a number, but if the variables are unset they’ll default to0&100, which is fine.Once the schema fix lands, this block is good.
No action needed here other than re-running locally to ensure startup succeeds.packages/backend/src/repoManager.ts (2)
13-13: Add import forenvconfiguration
This import enables use of the new Redis cleanup environment variables (REDIS_REMOVE_ON_COMPLETEandREDIS_REMOVE_ON_FAIL) in job scheduling.
110-113:❓ Verification inconclusive
Apply custom job retention settings to index queue
Ensure thatenv.REDIS_REMOVE_ON_COMPLETEandenv.REDIS_REMOVE_ON_FAILare parsed as the correct types (number | boolean) required by BullMQ, and verify that their default values (0 and 100) fulfill your memory-leak mitigation and debugging needs.
🏁 Script executed:
#!/bin/bash # Verify type and default values for retention variables in env.ts rg -n "REDIS_REMOVE_ON_COMPLETE" packages/backend/src/env.ts rg -n "REDIS_REMOVE_ON_FAIL" packages/backend/src/env.tsLength of output: 235
Apply custom job retention settings to index queue – variables verified
- In packages/backend/src/env.ts (lines 38–39),
•REDIS_REMOVE_ON_COMPLETEis defined asz.number().default(0)
•REDIS_REMOVE_ON_FAILis defined asz.number().default(100)- These values satisfy BullMQ’s
numberrequirement.- If you also need to support boolean flags (e.g. keep all jobs when
true), consider using a preprocess/coerce schema (e.g.z.preprocessorz.coerce.boolean()) or adjust the types accordingly.- Confirm that retaining 0 completed jobs and up to 100 failed jobs aligns with your memory-leak mitigation and debugging strategy.
Fixes memory leak in redis
Summary by CodeRabbit